🇦🇺Australia @thomwilhelm

thomwilhelm → created an issue.

Also +1 for adding this. Much easier to have code like this rather than having to set up a config split.

// Disable Fastly purging by default.
$config['fastly.settings']['purge'] = FALSE;
if (is_production()) {
    // Enable purging on production.
    $config['fastly.settings']['purge'] = TRUE;
}

Hey @generalredneck just gave PR 17 a test and all worked nicely for me, thanks for rerolling, apologies for including those lines from the other ticket.

Have added a MR with a proof of concept for you to review.

I also tested that calling:

Cache::invalidateTags(["erp_toolbar"]);

Correctly invalidates the toolbar cache.

thomwilhelm → created an issue.

Can you link to the duplicate issue? I'm having this same issue but couldn't immediately find the duplicate one.

Agree, report data should not be stored in configuration, as on each deployment it will be reset.

I've had a stab at moving this data to be stored in state data instead, including having an update hook to migrate this data.

I'll try running this on a few different sites shortly, if there are any issues I'll update the patch accordingly.

I ran into this same issue and had tracked it down to the same place, as the "Update Report Data" button wasn't displaying for me at all even as an administrator.

One nitpick in the patch from #6:

// Only show this button to users w/proper access or super user.
$uid = $this->user->id();
if ($uid == 1 || $this->user->hasPermission('update report data')) {
  $btn['run_button'] = [
    '#type' => 'markup',
    '#markup' => $this->t('<div style="float:right"><a class="button" href="/admin/reports/paragraphs-report/update" onclick="return confirm(\'Update the report data with current node info?\')">Update Report Data</a></div>'),
  ];
}

The hasPermission method in docroot/core/lib/Drupal/Core/Session/PermissionChecker.php already does a check to see if $uid == 1, besides, Drupal is moving away from using $uid == 1 logic as it's not very flexible:

https://www.drupal.org/project/coder/issues/2975233 ✨ Add a sniff for checking against hard-coding uid=1 permissions Active
https://www.drupal.org/project/drupal/issues/540008 📌 Add a container parameter that can remove the special behavior of UID#1 Fixed

With this in mind, I believe we can simplify the check to just be:

// Only show this button to users w/proper access.
if ($this->user->hasPermission('update report data')) {
  $btn['run_button'] = [
    '#type' => 'markup',
    '#markup' => $this->t('<div style="float:right"><a class="button" href="/admin/reports/paragraphs-report/update" onclick="return confirm(\'Update the report data with current node info?\')">Update Report Data</a></div>'),
  ];
}

Hi @Dobefu the module was already performing this cast so it wouldn't break anything, see the diff. It's just correcting a math error in the original design.

Other cases not handled can be tackled in follow up issues.

Having the same issue after going from 4.0.18 to 4.0.19 whilst on Drupal 10.2, seems like the change in this issue caused the regression for me:

https://www.drupal.org/project/environment_indicator/issues/3456599 🐛 Toolbar active tab background is always white on Drupal 10.3 Fixed

Backported to release 8.x-2.4

No worries I'll put out a release for the 8.x-2.x branch tomorrow.

Kristen Pol → credited ThomWilhelm → .

Implemented into 3.0.0 release.

ThomWilhelm → made their first commit to this issue’s fork.

Apologies this won't be merged as we don't support the Drupal 7 version of the module any more.

Appreciate the issue and patch, however we don't support the Drupal 7 version of this module anymore.

Hey Sime! I can see your use case and can understand why it could make sense to work that way. I'll admit I hadn't really thought of this argument before.

However this would mean that you couldn't clear all log messages from the UI at all (without uninstalling this module) so I don't think I'd be in favour of that.

That being said, if there was a new configuration option "Prevent manual clearing of database logs" for example, that defaults to FALSE, I'd be happy to look at merging that as it wouldn't change existing sites, but would allow people to have this restriction should they wish.

Thanks I agree that doesn't seem intentional. Using the database service seems more logical here.

Yes I'm aware you can still somehow achieve this result if you add two rules, one for the visibility, and one for the required state of the field. But in what case would a field be hidden, but still required?

The issue summary accurately describes how the module used to work, the release when this functionality changed, and steps to reproduce.

In fact in other issues there are commits from maintainers that confirm this is the way the module is intended to work. ie. once a field is not visible, it should not be required:

https://www.drupal.org/project/conditional_fields/issues/2983381 →

And in the 4.x version of js/conditional_fields.js you can clearly see when a field isn't visible, it should not be required.

https://git.drupalcode.org/project/conditional_fields/-/blob/4.x/js/cond...

So for me the issue summary is totally accurate. This is a clear bug not a feature request.

Hey @dqd thanks for looking at this ticket!

using or setting-up a required field which gets hidden by CF and should be ignored then on save, sounds a little bit like contradicting site building attempts and should not be considered to work out of the box

This is not true at all and from my understanding is a main use case for this module, hence why people have noticed it's broken their sites. In fact this functionality you are describing is exactly what is shown on the modules project image. The field Operating System triggers a conditional field Other which is only required if it shown, otherwise it is not required.

https://www.drupal.org/files/images/conditional_fields_image.png →

I'd actually go one step further and say once fixed we should ensure this feature has tests written to make sure this functionality doesn't get broken.

@yivanov Been a while since I looked at this, but I believe the issue is that dynamic_entity_reference 2.x and 4.x use triggers, which isn't supported by Acquia when copying a database.

This ticket was allowing the salesforce suite module to allow different versions of dynamic_entity_reference to be installed. I believe to fix your issue, you'll need to uninstall dynamic_entity_reference 4.x and install dynamic entity reference 3.x. This should mean your database doesn't have triggers anymore and can copy between environments without issue.

Proceed with caution but from memory this is what we did to fix.

ThomWilhelm → created an issue.

If 2.x is also the Recommended version, could we mark this as such on the project page?

+1 same issue for me, added "algolia/algoliasearch-client-php": "~3.3.0" to my composer.json for now.

Comment #5 you were correct!

I realised I had a broken config_split setup, so I had missed the schema update when upgrading at some stage.

After fixing my config_split setup, I uninstalled stage_file_proxy, then re-installed and configured the form again from scratch. After exporting dev config, I can see the origin_dir setting. So there is no need for this patch, I'll close the MR.

Oh that's interesting as I only stumbled across this as I got this warning on one of our sites. I'll do some more digging.

ThomWilhelm → created an issue.

ThomWilhelm → created an issue.

ThomWilhelm → made their first commit to this issue’s fork.

ThomWilhelm → made their first commit to this issue’s fork.

Thanks so much, having reviewed the module it was pretty much all good to be marked as Drupal 9 and 10 compatible. Was just a couple of Drupal rector and codesniffer fixes.

I tagged a new 2.0.0 release and made this the default.

Thanks for supplying this patch, I've fixed this up as part of the 2.0.0 release.

Created a 2.0.0 release that supports Drupal 9 and 10.

Contacted flocondetoile via his profile for a response.

Supplying a 9.5.x version of the patch for anyone upgrading to Claro before the Drupal 10 upgrade.

ThomWilhelm → created an issue.

ThomWilhelm → created an issue.

ThomWilhelm → created an issue.

+1 to this MR. Allowed me to upgrade to ckeditor5.

Would love to see this get committed as technically if you are using the video_embed_wysiwyg module, you can't migrate to ckeditor5 due to the dependency on ckeditor.

OK so I've tested the patch in comment #15 and it fixes the issue on our site. However, I'm marking this as Needs work for a couple of reasons:

• I don't think the correct fix is commenting out a return statement. If it's not needed, it should be removed.
• So we have chance to respond to comments in #16 and #17.

In regards to #16, the previous module behaviour was that if a required field was hidden, then it wouldn't be required. I know you could leave it optional in the settings and then make required based on a condition, but I think for the scope of this issue we just need the previous behaviour back.

With regards to #17, is this something that was working in alpha1 and broke during the alpha2 release?

+1, just adding a patch here for people using composer patches.

Hopefully this can get merged!

"The underlying problem is that support for PHP7 ends in November"

Hasn't PHP 7 been end of life since November 2022? I wouldn't expect contrib to actively fix modules based on a version that's EOL.

If this is to be re-opened, I feel we need steps to replicate to a vanilla Drupal install, as likely the issue is in custom code as previously mentioned.

I came across this same error while testing the PHP 8.1 upgrade, but it was actually caused by a custom module I wrote that reformatted the users display name by implementing hook_user_format_name_alter(). The problem was with the custom logic I wrote, the return value for the Anonymous user was returning NULL, triggering this error.

To fix I needed to add a case for the anonymous user to not reformat their username:

function my_module_user_format_name_alter(&$name, AccountInterface $account) {
  if ($account->isAnonymous()) {
    return;
  }
  // Existing custom logic.
}

I had the same issue, I fixed by using a new update hook that calls password_policy_update_8302() again.

/**
 * Re-run password_policy_update_8302.
 *
 * To fix https://www.drupal.org/project/password_policy/issues/3304188
 */
function MY_MODULE_update_8005(&$sandbox) {
  \Drupal::moduleHandler()->loadInclude("password_policy", "install", "password_policy");
  password_policy_update_8302($sandbox);
}

As comment #9 mentions, it would be interesting to get any further analysis on this. But this makes the error go away.

Just coming here to report similar issues, all my tablefields got broken after testing the upgrade to the latest tablefield version.

This change should have been at least mentioned in the 2.4 release:

https://www.drupal.org/project/tablefield/releases/8.x-2.4 →

When I upgraded, and then subsequently saved my webform settings, this got set to FALSE, which meant all my dedicated page webforms got broken. I think there should have been an update hook in this change updating existing sites configuration to default this setting to TRUE.

ThomWilhelm → created an issue.

Module is currently at 6.0.0-beta3, so 6.0.0-alpha1 would be a backwards version.

Or do you mean 7.0.0-alpha1?

ThomWilhelm → created an issue.

ThomWilhelm → created an issue.

This change starting throwing Javascript errors on my site which in turn broke the antibot module which relies on Javascript working.

I've uploaded the errors I'm getting incase others have the same issues. Needed to roll back to 3.25 for now.