🇧🇪Belgium @RandalV

@apmsooner Eureka! That fixed the issue.

Sorry for the duplicate.

randalv → created an issue.

Hi, thanks for the work so far.

Updating the view does work, but sadly the exposed filter blocks aren't refreshed.
For that, I was looking at this issue: https://www.drupal.org/project/drupal/issues/3032353 ✨ Exposed forms in a block are not currently updated when Ajax filtering is executed Needs work

Applying the MR 'works' in the sense that the blocks do get refreshed, but instead of showing the relevant configured blocks (meaning the exposed filter blocks with the hidden fields context), it just loads the standard views exposed filter block into them.

I'll try to make the time to look into this whenever I can.

Updating page status.

Documentation with a basic example has been added to https://www.drupal.org/docs/develop/drupal-apis/htmx/ajax-api-to-htmx/pr... → .

Added documentation for replacing the PrependCommand AJAX command.

Adding a note for HTMX 4.0 changes.

Removing arbitrary route name in the URL object.

Documentation was added, setting to 'Needs review'.
https://www.drupal.org/docs/develop/drupal-apis/htmx/ajax-api-to-htmx/appendcommand →

I've added the documentation with a very basic example to explain/show the necessary changes to the API.

Apologies, the last patch missed some changes to the entity's interface.

Another possible edge case would be if one entity's field is translatable and the other isn't, then the one to many relationship should also be reworked so it updates all translations of the translatable entity rather than just the current translation.

I updated the MR to allow one-to-many translation relationships.

I can reproduce the issue you're talking about, furthermore I also run into an issue where the values are added to the default translation of the corresponding entity.

I added a merge request that fixes these issues for me locally.

It:
- Retrieves the translated version of the original entity, if it exists
- Retrieves the translated version of the referenced (corresponding) entity, if it exists

This should also not affect untranslated fields, if the field is untranslated and you retrieve a translation of an entity, the values will just be saved to the default translation.

I also added a static patch of the MR in its current state.

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

Hmm, after further investigation, setting this to a non-translatable data type also removes its config translatability.
The question is, does this need to be translatable at all?

If not: I feel like this patch is a valid fix, if not: we should find a different solution to store the excluded entities, perhaps in an array (sequence/mapping) rather than a long unbroken string?

randalv → created an issue.

MR created

randalv → created an issue.

Alright, I've pushed some changes that should make it fully D11 compatible.

The biggest one was the constructor for the ConfigurationForm, in D11 an extra argument was required.
To keep backwards compatibility, I've removed the constructor entirely and set the custom properties in the static create() method.

This has the added advantage that any future changes to the constructor will be fully handled by the parent create() method, and we can simply set our own custom properties without having to worry about that.

Other than that, only a version requirement change was needed in composer & info.yml.

randalv → created an issue.

A small fix in this patch: when checking whether the alias is unique, two faults were present in the code.

1. The REQUEST domain was used, not the ENTITY domain, this meant that with entity saves via CLI (e.g. drush), the domain context was always the main domain... So aliases for other domains were saved with `-0` or `-1`
2. When checking if the alias is reserved, the source was not given, meaning if the alias previously existed in the database with the current source then it's marked as 'reserved' even though it's not.

This latest patch solves all of that.

Little use case where the patch did not help for me:

- layout_builder
- layout_builder_at (for asymmetric LB translations)
- added new boolean field to block
- saved pre-existing block with that new boolean field (ticket or unticked)

This field was initially empty (not `['value' => 0]` but just non-existing), upon saving a block (even the default translation) I received the error mentioned in this issue because the value was "changed" (it changed from non-existing to `['value' => 0]`.

The solution for me was to simply turn on the "Hide untranslatable fields" checkbox on my block types, this did not impact the editor experience whatsoever in my case because with `layout_builder_at` the blocks are *cloned* into translations, they aren't translated, so even on node translations all the fields on the block are always visible because you're always editing the block's default translation.

Hope it helps someone.

This seems like a useful feature, just passing the mapped value along to the field setter should work fine.

Drupal's entity_reference field setters can handle all of the following input values just fine:
- 13
- [13, 14, 15]
- ['target_id' => 13]
- [ ['target_id' => 13], ['target_id' => 14], ['target_id' => 15] ]

So I don't see any issues with this patch.

I think, usually when removing the canonical route, it's not "actually removed" but instead redirects or links to the edit link template?
I feel like that should solve the initial issue along with this issue.
The canonical "route" remains, but it's not an actual route and just sends you to the edit page.

Here's a patch that applies the change that works for us.

Hi,

Thanks for the issue, it already got us closer to a solution as well.
However, I notice a small issue in the `commerce_checkout.schema.yml` file.

commerce_checkout_pane_configuration:
  type: mapping
  mapping:
    display_label:
      type: string
      label: 'Display label'
      translatable: true
    step:
      type: string
      label: 'Step'
    weight:
      type: integer
      label: 'Weight'
    wrapper_element:
      type: string
      label: 'Wrapper element'

I *think* the `display_label`'s type should be "label" rather than string.
Currently, the display label is editable in the UI but not translatable.
Not every plugin provides a display label from its attribute/annotation, and since it is editable in the UI it should also be translatable in the UI.

I'm not sure if the combination of `type: string` with `translatable: true` does anything, though? I checked the "config schema cheat sheet" PDF file and couldn't find anything.

Hi @a8w4,

I'm afraid I'm still not able to reproduce this ☹️
I followed these steps:
- `composer create-project drupal/recommended-project:10.4.6 sandbox-10`
- `ddev config && ddev start`
- `ddev composer require drush/drush drupal/msqrole && ddev drush pm:enable msqrole`
- `ddev drush pmu msqrole`

I also tried installing and uninstalling via the UI, it seems to always have worked.

Could it be possible there was just some corrupt caching or config in your project at the time?
If there's any more clues as to how this could be recreated, I'm happy to look further!

Hi @a8w4,

Thanks for your report!
I'm unable to reproduce this issue though, would it be possible to share the exact steps / paths where this issue occurs?
As well as let me know if it's reproduceable on a clean D11?

Thanks in advance!

Thanks for the fix, and sorry for the inconvenience :-(.

I'll release a new tag.

A small fix applied to new entities being saved, they shouldn't be processed in that presave hook.

Welp.. Sadly I have no idea how to fix that mess.

Instead, here's the patch as a file.

Hoo wee.. Something seems very wrong with this merge request, it targets the initial fork branch rather than the module's branch 🤦🏻‍♂️

I had to open a new branch because the existing one (1.x) created issues for me locally, due to it having the same name as the main module's git branch.

This attempts to make the functionality as 'global' as possible, by retrieving most stuff dynamically.
Please take a look and feel free to test it.

There is an edge case where this could "not work", which is when the route parameter in `entity.[entity_type].canonical` is not the same as the entity type machine name... But I can't think of a single instance where that would be the case.

I think this issue is definitely an important one, but the scope still seems too small.
I wonder why it should be limited to nodes and taxonomy terms at all, all content entities should be valid here.

I'll see if I can cough up a solution for that.

And another apparent oopsie, issues like this already exist 🤦🏻‍♂️

I'll link them and mark it duplicated, apologies.

Apologies, I forgot this has now been included in core and the issue queue has moved there.
Changing project + component.

randalv → created an issue.

Applied to 1.2.x, will come with when a new tag gets released.

randalv → created an issue.

Thank you for the fix! I've committed it and it'll be introduced in the next release.

Hi @rosie.catchpole,

Thanks for reporting the error, I'll take a look at the solution and will get back to you ASAP.

I had encountered a bug in a react application (embedded in Drupal on the same domain) that for anonymous users, no session id was present.
I found the functionality where this was set and extended it slightly, this should correctly set the session id if it's ever missing.

I also fixed the core version requirement I had mentioned before.

The tests are still needed though, leaving status as-is.

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

Hi Manuel,

You're right, I have not yet been involved with issues thus far :) I'll get into that ASAP.

I was hoping we could open a new branch for D10/11 compatibility (2.0.x), and immediately tackle all those issues.
That way, 1.x could remain 'supported' but not maintained for D9, and we can go forward in 2.0.x with modern code (OOP hooks and other newer technologies).

I'll keep you posted!

randalv → created an issue.

New tag has been released. Thanks again!

Hi @andybroomfield,

Good catch! The class name was altered at the last moment before merge, seems I forgot to alter the service name. Thank you.

I'll merge and release a new version.

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

Seems logical, I've added the hooks_converted parameter.

Since the other threads are resolved, I'll merge into 1.2.x

Processed the feedback.

I was also debating whether the Help and Cron hooks could be merged into a "Core.php" file since they're both small hooks, but I just kept them in their own file.
Figured it's better to keep things as split up as possible, which is the whole point of moving away from one gigantic .module file to OOP hooks :)

randalv → created an issue.

@svendecabooter the changes have been applied in the MR.
Feel free to check it out, everything seems to work fine.

Changed minimum Drupal Core version to 10.1, PHP remains at >= 8.1.

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

- Merged 3.x into the MR
- Changed the update hook so it lands at the end (I figure since this is a new functionality, logically we shouldn't let it run before newer update hooks)
- Added form states so the client secret is only visible and required if PKCE mode = off
- Added a deprecation warning explaining that not supplying the "$session"-parameter is deprecated, but doesn't break anything if that is the case

The MR applies again and seems to work functionally as it did before.

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

Hi julianriemersma,

Thanks for reporting the issue!
I've finally gotten around to provide a fix, I had to fix it a bit differently because removing 'authenticated' from disallowed roles was creating some other issues.

I added a new release.

Patch #2 works here as well.

As per this issue / comment: https://www.drupal.org/project/select2/issues/3474793#comment-15991481 🐛 Declaration of Select2::defaultConfiguration() must be compatible with FilterWidgetBase::defaultConfiguration() Active
Is this still active/relevant?

Merged latest changes from 9.0.x into MR branch, seems to apply and work fine on my end.
Setting back to 'Needs review' so someone else can have a test before it's merged!

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

Merge request added!

I also took the liberty to remove what I assume was an orphaned `var_dump` 😇

A little correction from my end, all the functions that call the "renderSiteSettings"-method in the extension class do seem to render with the correct language context, but the other two functions (`site_setting_field` && `site_setting_entity_by_name`) specifically check for a langcode and load that translation, so those should be altered I think.

I'll add a MR for it.

randalv → created an issue.

With the current MR changes, I run into a plethora of issues where the value is set to an empty array or null or something...
Not sure why this happens.

Causes issues on basically any entity form for me, for example when saving a new taxonomy term, then the 'weight' value is set to null and gives a WSOD.

Can confirm the issue persists and is solved by the patch, thanks!

Hi @julianriemersma,

Sorry for the delay, I'll try to take a look into this ASAP.
I think something else might be wrong still, it's normal the authenticated role can't be chosen (since you *need* that role to use Masquerade by Role), but it shouldn't deny access when trying to masquerade as authenticated.

I think I've found a solution but need to do more testing.
Will keep you updated.

Negate my proposed solution, I just saw it's a static function.
I tested it by adding a static property, but this doesn't seem to really help... The performance issue remains.

Perhaps a maintainer has a better proposal for this?

randalv → created an issue.

MR can be reviewed, it's a simple but logical change.

randalv → created an issue.

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

I was looking for this functionality as well.
We have a website that has pricelists 'linked' to a value on the user entity.

We import our pricelists from an ERP, and we've worked around this issue by hooking onto the user & pricelist insert/update events.

Pricelist on insert/update:
- Find all users with the custom value and attach them to the customers field
User on insert/update:
- Remove the user from pricelist(s) with the old value (if applicable) and add to pricelist(s) with the new value

The main problem here is that, if no users have the value of that pricelist, the pricelist applies to EVERY user. That's something that conditions would have easily solved.
In our use case, we've solved this by creating a 'dummy' user and if no other users apply for the pricelist, we attach the 'dummy' user to the pricelist.

Configuration/development wise this would have been much faster and cleaner if we had conditions, but I understand the argument of speed is a factor here too.
I'll set this as postponed, it's not and hasn't been really an active issue and if the need arises later on it can be reopened.

Ah.. my bad, you're absolutely right. I overlooked the default value of the class property.

I'm not sure if a backport to 1.x is still necessary, I suppose it depends on how long 8.x-1.x will remain supported? If it will, then maybe it would be good to have that fix there too 😇

Hi @mably, apologies, I hadn't seen that!

Although if I'm not mistaken that doesn't entirely fix the issue I was experiencing, because the encoder will still not strip tags or trim strings unless a setting is specifically provided (whereas before it would strip/trim by default).

With the fix currently in the 2.x branch, the warning message is gone, but if the encoder is used in Views Data Export it'll still let any and all HTML through (without the possibility of configuring it otherwise 😞).

It would be easily solved if encoders allowed configuration forms, but since (I think?) they don't, would it be preferred to keep the old functionality intact?

I've created an MR that adds default values. This preserves the behavior pre-#3108301.

randalv → created an issue.

Hi @mably and @dejan0,

Thanks for your solutions! I've tested the MR, and it seems to work fine for me.

randalv → created an issue.

I haven't got the time to fully debug this in facets itself, but in my case it was solvable by rendering it in a custom block like below.
I had to render in a custom block anyway because I wanted to add extra items before and after the exposed filters, so this was a quick fix for me.

/** @var \Drupal\views\Plugin\Block\ViewsExposedFilterBlock $plugin_block */
$plugin_block = $this->blockManager->createInstance('views_exposed_filter_block:events-overview');

// Make sure the facets are populated.
$view = $plugin_block->getViewExecutable();
$view->preExecute();
$view->execute();

return [
  // ...
  'filters' => $plugin_block->build(),
  // ...
];

I'm also experiencing this issue in a new project of ours using Facets 3.0.0-beta4, exactly as described above.

On the view page itself, everything is fine. Anywhere else, all the facets are empty :(

I'll see if I can find anything, haven't had the time yet to debug.

Drupal 11 compatibility is already implemented.

Hi @mitsuko,

I'm not entirely certain what the case was anymore but when I built this module, I ran into some strange behaviour when the authenticated role wasn't present.
I could test it again, but I figure it's not really important anymore.

In any case, the reason you didn't run into any such incidents is because with your patch, behind the scenes the module would still apply the authenticated role (even if you didn't select it) in the submit handler.

I think regarding the validation, we shouldn't worry about that anymore. Before it was deliberately made impossible to leave the role selectbox empty, so we had to double check in the validator to see if people didn't get around HTML validation and leave it empty anyway.
But since we now want it to be left empty when required, we don't need that extra validation anymore.

Any extra roles that are added through the inspector will also fail the form, since Drupal automatically handles that as invalid input as it wasn't in the form element's `#options` array.

It's been released in 1.0.18.
Thanks for your suggestion!

Hi @mitsuko,

I see your point, it would be useful to only masquerade as the authenticated role, however I think I'll fix it somewhat differently.

The authenticated role *should* always be selected, so removing it from the automatically selected roles might produce strange results.
I'll just remove the 'required' parameter from the roles selector, leaving the field empty will result to only the authenticated role being enabled.

That should cover every use case.

I'll reopen this since the fix hasn't actually been released yet, nearly 2 years later.

@Maintainer, could you please release a new tagged version with this fix present? Thanks!

I can confirm the MR applies to 1.25.

I now get the aria-label="Cookie privacy banner" parameter in the popup's HTML tag.

(PS. Let's keep working in the MR -if anything needs to change- rather than attaching patches)

Please, feel free to review.

This fixes the issue of terms always being rendered in their default language, rather than the current language.

randalv → created an issue.

Thanks for the patch/MR.
I've applied the MR diff to one of our sites, and the issue has been resolved.

Hopefully this finds its way into a release 🙏

That's a very interesting development... an array containing the serialized content with `50` as its key, thanks for sharing @mahde!
I'll debug some more too and see if I can reproduce it somehow.
I also notice it adds both roles twice, interesting!

Hi @mahde,

Thanks for taking the time to debug and updating the MR!
This brings us back to my comment in #5 🐛 TypeError_: unserialize(): Argument #1 ($data) must be of type string, array given in _unserialize() Postponed: needs info though, there's only one piece of code in the module that updates the user data for roles, and it always serializes it..
I don't understand how it could possibly contain an array unless some other module messes with it 😓