Account created on 18 April 2010, about 15 years ago
#

Merge Requests

More

Recent comments

🇦🇺Australia elc

Tests pass is this pipeline
https://git.drupalcode.org/issue/views_advanced_cache-3203460/-/pipeline...

Removing commits to be merged in separate issues, prior to this one:

🇦🇺Australia elc

elc made their first commit to this issue’s fork.

🇦🇺Australia elc

@todo Move the following into their own issues and do not merge them as part of this issue:

  • GitlabCI setup
  • phpstan/cs fixes
🇦🇺Australia elc

MR made.

Many phpstan and phpcs issues still present in the module.

🇦🇺Australia elc

Maintainer malcolm_p contacted via d.o contact form.

🇦🇺Australia elc

The contrib module Fraction is also affected by this, with aggregation values disappearing.

In Core 10.4.5 the SUM output used Drupal\views\Plugin\views\field\NumericField, but in 10.4.6 it turned up in a Drupal\fraction\Plugin\views\field\FractionField using Drupal\fraction\Fraction\FractionItem::setValue(['numerator' => NN.NN]). setValue wasn't expecting such a value (normally it would be presented with FractionItem::setValue(['decimal' => NN.NN]) or ['numerator' => X, 'denominator' => Y]) resulting in the value always being empty as per the FractionItem::isEmpty().

Using both AUM & AVG in the same view ends up with one affecting the other, where in 10.4.5 that did not happen. I haven't figured out where that is happening.

🇦🇺Australia elc

There was a now reverted core change which has caused the change in behaviour. The change was introduced in 10.4.6 when 2735997 hit.

🐛 Decimal separator and decimals settings ignored when aggregating decimal fields Needs work
🐛 [regression] COUNT aggregation no longer outputs count value for some fields Active
🐛 Upgrading from 10.4.5 to 10.4.6 removes views format plural on Content ID field Active

It removed the numeric field handler inside core/modules/views/src/Plugin/views/query/Sql.php
https://git.drupalcode.org/project/drupal/-/commit/59d79456a1a135b3d02bb...

End result is that after aggregation, instead of a numeric field being used for the result of the operation, it uses a Fraction field which is where the setValue(['numerator' => NN.NN]) happens.

Add a "me too" to the issue about fixing it up, and see where the core issue goes.

🇦🇺Australia elc

Installed Drupal 10.4.1 + Fraction 2.3.1 which resulted in a working SUM display.

Some items of note:
- $field->field_alias is still 'unknown'
- Resulting SUM field is the following during hook_views_pre_execute
- Class: Drupal\views\Plugin\views\field\NumericField
- type: numeric

In Drupal 10.4.6 + Fraction 3.0.1:
- Resulting SUM field is:
- Class Drupal\fraction\Plugin\views\field\FractionField
- type: fraction_field

Also getting validation error in both:

display.default.display_options.fields.field_fraction	Default field	views.field.*	Yes	<array> 
'click_sort_column' is an unknown key because display.default.display_options.fields.field_fraction.plugin_id is fraction_field (see config schema type views.field.*).
'type' is an unknown key because display.default.display_options.fields.field_fraction.plugin_id is fraction_field (see config schema type views.field.*).
'settings' is an unknown key because [snip, etc]

Haven't had a chance to look deeper.

🇦🇺Australia elc

Changing target branch because it will not create an issue fork. Tags and branches are really messed up for this module.

🇦🇺Australia elc

This is expected behaviour from Drupal Core. If you make a view dependent on a module, and then uninstall that module, it will remove any config that was dependent on the module which is the entire view.

The fix is to delete the block, delete the range filter from the view, double check the view config to ensure that every trace of it is gone, and then uninstall the module.

You can always keep a backup of your synchronised config too, and re-import the view if needed.

🇦🇺Australia elc

Fixed in 8.x-2.0 tagged release.

Really should have been tagged 2.0.0 on the 2.0.x branch (2.x).

🇦🇺Australia elc

No difference found when changing the category back to a translated string.

Have found why the value is showing as empty:
Drupal\fraction\Plugin\Field\FieldType\FractionItem::setValue() is being called with $values = [[numerator] => 114.6800];, which results in a parent::setValue($values, $notify) call with that array, resulting in a null denominator.

Made this change to cope with that input (pushed to issue branch):

-    // If the decimal property is specified, use it.
-    if (is_array($values) && isset($values['decimal'])) {
-      $decimal = $values['decimal'];
+    if (is_array($values)) {
+      // If the decimal property is specified, use it.
+      if (isset($values['decimal'])) {
+        $decimal = $values['decimal'];
+      }
+      // If values array has been passed with only numerator and null
+      // denominator, treat it as if it was passed in as the decimal key.
+      elseif (isset($values['numerator']) && !isset($values['denominator'])) {
+        $decimal = $values['numerator'];
+      }
     }

Still unknown:
- Why field_alias is 'unknown'
- Why value is being set as numerator array instead of decimal array, or num+den array.
- What happened to cause of all of this

Pondering installing an older versions of Drupal + Fraction and investigating old behaviour to narrow down which version has contributed to the change.

🇦🇺Australia elc

Site is running Drupal 10.4.6 and Fraction 3.0.1.

I've been searching Drupal Core for any related changes without any luck. I haven't figured out how field_alias is ending up either with no value set, or the value set being unknown. The field is present in the query while the alias is unknown, which is weird itself since there has already been an alias calculated as the array key, and set as a key in the array too.

I'll reverse that commit and see if it makes any difference. First look is that it's only the category name that has changed. Hard pressed to think that would make a difference.

🇦🇺Australia elc

I've been using this patch to fix up SUM(fraction) successfully for years now, but it has just stopped working and I'm having problems narrowing it down to the exact failure. I haven't been able to narrow down the timeframe for when the failure happened either, except that it was either last week, or all the way back in January. It doesn't seem to line up with releases of this module.

Key to it is that $field->field_alias == 'unknown' for all of the Fraction fields in a view. That was causing the complete failure of what this patch was doing.

I've reworked the patch from #11 into 3.x-1515840-fix-sum to at least give me the correct answers in $view->results at hook_views_pre_render() time, but all of the fields are still considered empty and rendered as such. It seems very much like the field_alias being considered the default "unknown" by the field handler is the bugbear.

I had to change ordering of the 'entity field' vs 'field_name' in field definition since every field now has 'field_name', even if it is an entity field. This was resulting in no fields being found for fraction base fields.

The attempt to compensate for the field_alias being missing is not the correct way to go. Determining why that is missing is where I am currently focussed.

I have not created a MR as this still needs work. Some additional eyeballs would be greatly appreciated. Dumping to 3.x branch.

🇦🇺Australia elc

elc made their first commit to this issue’s fork.

🇦🇺Australia elc

This has been fixed in the 3.0.0 release by correctly adding the MODULE_NAME.field_type_categories.yml file.

@see Drupal 11 compatibility fixes for fontawesome Active

🇦🇺Australia elc

No reviews. Making a RC for field testing.

🇦🇺Australia elc

Merged in ready for 3.0.0-rc2 release.

🇦🇺Australia elc

Merged in ready for 3.0.0-rc2 release.

🇦🇺Australia elc

Migration completed. Additional eyeballs appreciated.

🇦🇺Australia elc

Converted hook_help as a first step.

🇦🇺Australia elc

No, sending plaintext passwords is not the job of this module. RPT is the place to go. Back to removing it.

🇦🇺Australia elc

No response from PRT module maintainers, so change of plans to cope with issue entirely here.

Offload the token into a sub-module and an update to keep the current status quo for existing sites, and the option to not install it on new ones.

  • ➖ Move functionality and tests the new sub-module
  • ➖ Add hook_update_N to install the new sub-module
  • ➖ Move [user:password] handling to sub-module.
  • ➖ Add hook_requirements detailing a conflict and steps to mediate
  • ➖ Add option and coding to bypass the conflict with PRT since the sub-module will do exactly the same thing
🇦🇺Australia elc

Tests good supporting both versions of Drupal core. Tests as they stand don't install Commerce 2.

🇦🇺Australia elc

Leaving at needs review so that another set of eyeballs can have a look.

Plan would then be to make a 3.0.0-rc1 release so it will be tested out in the wild.

🇦🇺Australia elc

Thank you! Soon to be released as 8.x-1.5.

🇦🇺Australia elc

VWO now shows the correct version when connecting a website.

Confirmation that the A/B testing is working as expected with this change would be appreciated before a release should be made.

🇦🇺Australia elc

elc made their first commit to this issue’s fork.

🇦🇺Australia elc

Closing and waiting for the Drupal 12 bot.

🇦🇺Australia elc

Drupal 7 is no longer a supported release.

There is a Backdrop port: https://backdropcms.org/project/site_verify

🇦🇺Australia elc

elc changed the visibility of the branch 3495763-remove-user-password to hidden.

🇦🇺Australia elc

Drupal 7 is no longer a supported release.

🇦🇺Australia elc

Created an issue: 📌 Add missing words to dictionary eg varchar Active

Task/Other issue on Drupal Core seems to be the place to discuss and do these things.

🇦🇺Australia elc

Fixed up the one test that was held up just by an admin ui change that needed to click differently named buttons.

Will leave fixing up the phpcs/cspell issues for 📌 Fix the issues reported by phpcs Needs work once this is merged.

I can see that you committed OPT_IN_TEST_MAX_PHP: 1 but it has disappeared without a corresponding commit or change.

🇦🇺Australia elc

elc made their first commit to this issue’s fork.

🇦🇺Australia elc

Needs to be ported back to 2.1.x and 2.0.x as this is a long lived bug.

🇦🇺Australia elc

Located duplicate project:

https://www.drupal.org/project/nouislider

Posted issue to reference as "Replaced by" as the newer one is up to date (older is 15.5.1, new is 15.8.1 and I assume has mechanism for staying up to date?) and includes the correct composer.json file, and has all of the library files instead of just the min ones.
📌 Set "Replaced by" to noUiSlider JS Active

🇦🇺Australia elc

@todo Add a change notice/record regarding the removal.

🇦🇺Australia elc

Interop with Registration Password Token module Compatibility with Generate Password module Active .

🇦🇺Australia elc

This module sets the "password" property to the $account object itself during form submission, bypassing the need for core to continue to set the value.

https://git.drupalcode.org/project/rpt/-/blob/2.x/rpt.module?ref_type=he...

🇦🇺Australia elc

If memory serves, this was Automatic User Names module stepping in as it used the core password generator to provide a random string which did not validate as a username when Generate Password was installed.

It was fixed in Automatic User Names:
🐛 Make Automatic User Names compatible with Generate Password module. Fixed

🇦🇺Australia elc

Add reference to core issue.

🇦🇺Australia elc

Thanks for the quick work! Yes, this is all it needed.

Only other question is whether this should cause a point release to differentiate it from the previously supported drupal cores.

🇦🇺Australia elc

Looks like a release came out, but it needs a new one already.

Module uses the password_generator service, introduced in Drupal Core 9.1.0. This makes this the minimum supported version.

@see https://www.drupal.org/node/3153113

@todo
Update the info.yml file to set minimum version of ^9.1 || ^10 and make a new release.

🇦🇺Australia elc

Despite the requirements stated in the info file, the minimum Drupal core version is 9.1.0. Please upgrade to this version or use the previous version of this module.

🇦🇺Australia elc

Triggering 3.0.0-beta2 release as this is a usability issue.

🇦🇺Australia elc

Project page updated for now. Will need an update in the new year, once Drupal 7 Core support is turned off in Jan 2025.

Still 15k users of this module on the 7.x-1.x branch. Lots of sites stuck on Drupal 7. Going to be interesting.

🇦🇺Australia elc

Released as 3.0.0-beta1 or 3.0.0-beta2

Follow up task 📌 Update project page Active

🇦🇺Australia elc

Drupal 11.1.0 and 10.4.0 have been released, fixing the Legacy/Hook false positive.

This will be merged into 3.0.x when I get the chance to update/write the release notes for dev/beta. Reviews always welcome, but not blocking.

I feel the review will effectively be on the release which will be a new branch and a beta, and will not remove support from any of the existing branches, so this makes it safe enough for it to be released as is.

🇦🇺Australia elc

Confirmed that importing did not work with validation being a requirement but it not being automatic. Made validation automatic and required.

@todo

  • ➖ Add test to import SiteVerification entities through config.
  • ➖ Update project home page

Still waiting on release of 11.1 and 10.4 to get rid of the phpstan error about Hook/LegacyHook not existing yet.

🇦🇺Australia elc

Still @todo

- Update README with current usage instructions (file/meta entry has changed to just name/content entry with parsing html meta line and file upload field processing no longer included)
- Finish adding help hooks (code completed, not pushed)
- Confirm if the forced validation currently coded breaks sync import of config, which was the entire point of this change

🇦🇺Australia elc

Tests need to be expanded to install redis and configure the tests to selectively use it in redis only tests. The code currently only tests the database storage of flood data.

The drupa/redis module has a modified .gitlab.yml file which would provide insight into getting redis installed and adding additional builds to test with a without it.

🇦🇺Australia elc

Marking as RTBC on behalf of @interlated

🇦🇺Australia elc

Let's fix the issue at hand, instead of adding nullable type hints to things that aren't ever going to be null. Having them nullable was a mistake, so just running an automated coding standards fix against them really isn't the way to handle it. It's important to understand the proposed fix and what is being applied to, and whether it is the correct result.

And fix up the only two nullable type hints that really are needed, plus match up their docblock hint.

🇦🇺Australia elc

elc made their first commit to this issue’s fork.

🇦🇺Australia elc

Appears tests are running against Commerce 3 and that is not currently passing tests.

This patch is working in a production site to reproduce the behaviour from the patches before the final result was merged in.

🇦🇺Australia elc

Fixed as part of Provide exportable configuration Active .

The SiteVerification content entity allows the admin to set the label, id, and description which are displayed in the entity list.

🇦🇺Australia elc

Fixed as part of Provide exportable configuration Active .

The SiteVerification content entity has validation constraints on duplicating the filename, but does still store both types in the same ConfigEntityType for simplicity of storage and handling.

Duplicate meta tag is not restricted as it is normal operation for their to be multiple of the same meta tag name.

🇦🇺Australia elc

Fixed as part of Provide exportable configuration Active .

The SiteVerification content entity allows the admin to set the label, id, and description which are displayed in the entity list.

🇦🇺Australia elc

The "phpstan (next minor)" error is unavoidable until 📌 Backport Hook and LegacyHook Attribute Active lands. At that point both of the errors in the phpstan.neon file can be removed.

Very much open on feedback and patches on this! Will be making a 3.0.0-beta1 release in about a week if I don't get feedback, and that'll be out in the world for testing from then on.

One item of note is the simplification of the admin form for creating and editing the entities - it now only includes the name and the content fields, and does not have a full metatag entry, or a file upload field. There was already confusion about the file uploading part, fixed in 2.1.0-rc1, but it was added complexity that I didn't feel was needed in what was already a full re-write of the module.

🇦🇺Australia elc

Updating version to 3.0.x as this has become a full re-write of the module.

Production build 0.71.5 2024