🇧🇪Belgium @dieterholvoet

dieterholvoet → created an issue.

dieterholvoet → created an issue.

dieterholvoet → created an issue.

dieterholvoet → created an issue.

dieterholvoet → created an issue.

I checked and you're right, symfony/polyfill-php80 including the str_starts_with function was added in Drupal 9.0.0 → . That would mean we need to drop support for Drupal 8, but I'm okay with that. Feel free to add back str_starts_with and drop support for D8 if you like.

dieterholvoet → created an issue.

Just had this happen again randomly on a production website running Drupal 11.1.6. Clearing caches through the UI did not work. Restarting the server and running drush cr fixed the problem.

I was still encountering an issue when the Crop module is being used, adding flex-wrap: wrap; seems to fix that.

dieterholvoet → created an issue.

That's a good point. It looks like the 5xx cache maximum age setting doesn't actually do anything. Even if you explicitly create a cacheable 500 response like this:

return new CacheableResponse(status: 500);

there's Drupal\Core\PageCache\ResponsePolicy\NoServerError who marks the request as non-cacheable. I'll create another issue to remove that setting.

Why did you revert the change? str_starts_with is still there.

While we're at it, let's add the Expert mode checkbox from the Caching fieldset to this fieldset as well.

While this does seem like an oversight in the original implementation, this would be a backwards incompatible change for existing sites. To avoid having to create a new major version of the module, I propose we add two new settings: Surrogate 404 cache maximum age and Surrogate 5xx cache maximum age. If we keep these empty for both existing sites and new sites by default, there's no backwards compatibility break.

Thanks for your contribution! I left some feedback in the MR, please commit any future changes there.

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

The expiration time can be configured through the Confirmation URL expiration setting in the Webform handler.

dieterholvoet → created an issue.

Thanks!

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

I just tested but I can't reproduce the issue. I'm using required_api 3.0.0-beta1 and required_by_role 2.0.0-beta3.

You should probably open a new issue, this one is closed with no way to reopen except for the maintainer.

The custom version of time_diff which is being added in ✨ Add back missing Twig filters Active returns translatable markup. In case anyone wants to test: please do it within two weeks, I'm merging and releasing after that.

I listed the module as alternative in the project description.

I'm adding back all removed Twig filters in ✨ Add back missing Twig filters Active , restoring backwards compatibility. In case anyone wants to test: please do it within two weeks, I'm merging and releasing after that.

I'm adding back all removed Twig filters in ✨ Add back missing Twig filters Active , restoring backwards compatibility. In case anyone wants to test: please do it within two weeks, I'm merging and releasing after that.

This is being fixed in ✨ Add back missing Twig filters Active . I'll credit the people who worked on this in that issue.

The time_diff filter has a dependency on the symfony/translation package, so we can't use the original code. I'll add a custom one based on the suggestion in ✨ SyntaxError: Unknown "time_diff" filter Active .

Backported ArrayExtension and IntlExtension.

Seems like that module has a conflict 🐛 Custom Core Twig Filter "format_date" conflicts with twig/intl-extra Active with Drupal core. Let's just copy-paste the original extension instead.

As a start, I propose bringing the Twig IntlExtension → module back into this one.

dieterholvoet → created an issue.

I also pushed a fix for content translation and revision tabs not being hidden on taxonomy term forms when using Claro.

dieterholvoet → changed the visibility of the branch 2.0.x to hidden.

I created a new 3488386-hide-vertical-tabs branch that is based on the upstream 2.x branch. No idea where the 2.0.x in this issue queue comes from, but it shouldn't be used for a MR.

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

Left some comments in the MR.

Both the 7.x release of this module and Drupal 7 are not supported anymore, so I'm closing this as won't fix.

Both the 7.x release of this module and Drupal 7 are not supported anymore, so I'm closing this as won't fix.

Both the 7.x release of this module and Drupal 7 are not supported anymore, so I'm closing this as won't fix.

Both the 7.x release of this module and Drupal 7 are not supported anymore, so I'm closing this as won't fix.

Both the 7.x release of this module and Drupal 7 are not supported anymore, so I'm closing this as won't fix.

Both the 7.x release of this module and Drupal 7 are not supported anymore, so I'm closing this as won't fix.

Alright, I'll close any 7.x issues then.

If a user has permission administer users but not administer permissions, they cannot edit user roles on user/UID.

Seems like they can. The core AccountForm does contain the following code:

$form['account']['roles'] = [
  '#type' => 'checkboxes',
  '#title' => $this->t('Roles'),
  '#default_value' => (!$register ? $account->getRoles() : []),
  '#options' => $roles,
  '#access' => $roles && $user->hasPermission('administer permissions'),
];

but this module adds its own role_change field to the user form and the core access check doesn't affect that. Or am I missing something here?

Seems like I don't have permission to do this, assigning @acbramley.

Looks good! Only the dependency injection left to be done.

Still needs to be made into an option.

@hikkypo please don't just post random patches without context. Work on this is being done in the MR.

I added the changes from 3475414-role_delegation-5.patch to the MR. This new config option is going to need a settings form, can't expect people to manually edit yml files.

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

Never mind, I was too fast. I'm going to revert. Refusing access to the separate page was intentionally done in 📌 Roles tab is redundant for users who can administer users Fixed . Adding the separate page for users with the administer users permission as an option is being discussed in ✨ Allow users with administer users to see Roles tab via a setting Active .

I tested this change, seems right. The comment indicates that the check should grant access and it also makes sense that users with the administer users role can manage roles on the edit page.

Exactly. If anyone could share their id-less config that's causing this warning, that would be useful.

Thanks! I was wondering, do you want to keep supporting the Drupal 7 version? Or can I close those issues as won't fix?

dieterholvoet → created an issue.

I'm aware of that, that's not the problem here. I described those steps in 'Steps to reproduce'.

I can confirm MR !82 gets rid of all warnings on version 1.x. I created a new MR for version 2.x.

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

dieterholvoet → created an issue.

I sent acbramley, benjy and jeroent the following message through their Contact tabs:

Hi,

Could you post a comment on my offer to co-maintain Role Delegation ( https://www.drupal.org/project/role_delegation/issues/3526587 💬 Offering to co-maintain Role Delegation Active )? I would like to become a co-maintainer of the module to help resolve any pending and future issues.

Thanks in advance,
Dieter Holvoet

dieterholvoet → created an issue.

Looks good, thanks!

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

@davis zhou thanks for your contribution! It fixes the issue for me. I just have one question, I left it in the MR.

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

MR !146 seems to be the same as MR !147 now, which means that there aren't any changes since you last asked me to test.

I can confirm that the deploy hook in MR !86 does the job, but like I said before only for people using drush deploy.

The alternative is to make sure the fields aren't recreated, so no data is lost.

Yeah, there doesn't seem to be a way to do that. updatedb creates and fills the fields and config:import overwrites them. The only way I can think about is to add a rabbit_hole_field_config_insert() hook and migrate the field there, after it's created. Not sure if we can use the Batch API at that point though.

For my own purposes, I'm going to create a separate MR that makes this a deploy hook. It's a relatively easy and sufficient fix for people using Drush.

Update hooks updating data should still be post update hooks though, so I'll leave that in the MR.

Never mind, I was wrong. Post update hooks also run before config import. Only deploy hooks run after config import, but those are a Drush specific concept so probably not a good idea to introduce here. The alternative is to make sure the fields aren't recreated, so no data is lost. I'll try to come up with a solution.

dieterholvoet → created an issue.