🇧🇪Belgium @andreasderijcke

True, but this module clearly has a history concerning that part, so you might want check why this was done.
Beside, if you clearly know what needs to be done, why don't you provide the solution?

FYI: the composer file was removed intentionally by maintainer: https://git.drupalcode.org/project/varnish_purge/-/merge_requests/12/dif...

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

"This is not a bug, it's a feature".

From the module page, second paragraph:

Project name and module name
Because of some bad naming in the start - the project name is varnish_purge, but the module itself is called varnish_purger. Sorry for that, but it's hard to move namespace after you have registred one.

You have to use

composer require drupal/varnish_purger

The proposed info file rename does not fix this, and also ignores config, schema and others files that have 'varnish_purger' in their name or namespace.

Just leave it, until there is a proper solution to fix the project name(space).

A quick workaround for the case in previous comment, would be to alter

/**
 * Implements hook_js_alter().
 *
 * Handles script files added from libraries.
 */
function klaro_js_alter(&$javascript, AttachedAssetsInterface $assets) {

  ...

  foreach ($javascript as $path => &$script) {
    if ($script['type'] !== 'setting') {
      $app = $helper->matchKlaroApp($path);
      if ($app) {
        $script['preprocess'] = FALSE;
        $script['klaro'] = $app->id();
      }
    }
  }
}

to

/**
 * Implements hook_js_alter().
 *
 * Handles script files added from libraries.
 */
function klaro_js_alter(&$javascript, AttachedAssetsInterface $assets) {

  ...

  foreach ($javascript as $path => &$script) {
    if ($script['type'] !== 'setting') {
      $app = $helper->matchKlaroApp($path);
      if ($app instanceof KlaroAppInterface && !$app->isRequired()) {
        $script['preprocess'] = FALSE;
        $script['klaro'] = $app->id();
      }
    }
  }
}

which will cause all required stuff to be loaded as normal, and might/will have some unwanted side-effects in regards to the 'Only once' option and perhaps others?

I think 💬 Possible to ignore external resources Active kind of handles the same issue, therefor added as related.

We're using the OIDC My Citizen Profile Flanders → module to add a widget that for Belgian citizens so they can login to the site using their official ID.

The issue we now have, is that this module, even though all scripts are added to service that is to be required, is still preprocessing the widget scripts to text/plain, replace them post page load.
This causes the scripts to be loaded 'too late' and the oidc_mcpf module javascript to error, because it counts on those dependencies to be loaded async, and therefor already be present.

Being able to completely ignore resources (so no script inclusion rewriting in KlaroJsCollectionRenderer either) would solve the problem, without having the other module(s) change their ways.
This is what issue 💬 Possible to ignore external resources Active comes down to as well, as I understand it.

Stumbled on this issue as well. Happy to see this already submitted with a solution.
Tested the patch on my case, works perfect.

Code-wise, I've inverted the check so we can use 'continue' and keep the code change minimal (also avoiding an additional nesting level for about 100 lines of code);

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

andreasderijcke → created an issue.

Third try is hopefully sound.

So, the problem occurred for non-float numerical fields that only have a min or max set on the field, but still have the DB restrictions applied.
In that case, changing the minMessage/maxMessage to notInRangeMessage should fix it.

Found the culprit.

The code from marksmith ( https://www.drupal.org/project/custom_field/issues/3518147#comment-16060456 🐛 Cannot set both min and max values for numeric fields Active ) does not have the problem, but in the final refactored version, adding the constraint values is done if they have a value, not if the constraint is actually set. That causes the problem if you have only a min or max constraint set and not both.

MR coming up.

Ah, clear. Thanks!

Thanks for the fast response!

One follow-up question though: What would a 'revoked key' imply for this case. A person that lost its citizenship?

andreasderijcke → created an issue.

Perhaps the resource detection can be skipped (and thus no logging) for all paths that are configured in the 'Disable Klaro! element and dont block attributed resources on following url patterns' option?

The option description states

... On these paths no resources are blocked and Klaro! will be disabled.

Which might imply the unknown resource detection is also not active, but based on the logs it is active on those paths.

Module maintainers do not always have say in whether a third party library they need (as in 'no better alternative') that is loading additional stuff from a CDN.

For example, the Bynder module → includes a widget from Bynder itself, and that uses CDN for additional stuff.
There is no way the Bynder module maintainers can change this. They can ask Bynder not to use a CDN, but will they change it (and if so, how long will it take)

Also, the inclusion is for backend only, so it feels wrong to have Bynder included in the banner for the end user, who will only see images (or files for that matter) from Bynder without addition scripts.

I think this is just an overlooked part after forking from drimage, with either part of a solution or leftover from a test.

See the MR for my proposal.

andreasderijcke → created an issue.

MR adds small change to checks for regex match if there is no full match. This allows for matching entity password forms, which use the target node id or bundle.
In case of nodes, you can use entity_access_password_password_node_*.

andreasderijcke → changed the visibility of the branch 3489447-not-working-with to active.

andreasderijcke → changed the visibility of the branch 3489447-not-working-with to hidden.

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

+1 for this.

• Users that might need to look at the reports, do not necessarily be able to change this module config.
• The other suggestion do make sense to me too.

Have applied the change on the D10.4 with the desired affect.

andreasderijcke → changed the visibility of the branch 3533214-processor-rendereditem-add to hidden.

andreasderijcke → created an issue.

andreasderijcke → created an issue.

Assuming we're talking about the new Facets as exposed filters:

Using Better Exposed Filters → you can have the facet rendered as 'Checkboxes/Radio buttons' on the filter widget. Disableing the 'Allow multiple selections' option on the facet itself, will get radio buttons.

andreasderijcke → created an issue.

Using imagemagick module 4.0.2 and ImageMagick 6, the patch needed to be slightly different. See attached.

andreasderijcke → changed the visibility of the branch 3533143-optimize-error-responses to active.

andreasderijcke → changed the visibility of the branch 3533143-optimize-error-responses to hidden.

andreasderijcke → created an issue.

andreasderijcke → created an issue.

andreasderijcke → created an issue.

In our case, we also used a dedicated linkit profile, not only to exclude Media but also to limit to certain node types and vocabularies.

I understand the choice not to support this as a feature, but perhaps this can be mentioned in the README as a 'use at your own risk' as I think it is very usefull towards content editors?

andreasderijcke → created an issue.

apmsooner → credited andreasderijcke → .

Should have been fixed in 🐛 Cannot set both min and max values for numeric fields Active .
Perhaps it customisations that are interferring. Will double check later this week.

https://www.drupal.org/project/custom_field/issues/3524259# →

andreasderijcke → created an issue.

This patch works as expected.

In combination 🐛 TypeError: Drupal\content_lock\ContentLock\ContentLock::release(): Argument #1 ($entity) must be of type Drupal\Core\Entity\EntityInterface, null given Active you can fix a 'stuck' environment without additional manual cleanup.

Ran the change on a site with the issue, it cleaned it up nicely.

I'm also in favour of adding this in addition to 🐛 Deleted entity id's on content_lock table make cron to fail Active .

I confirm that with the MR

• The view block shows up as facet source on /admin/config/search/facets
• A facet as exposed filter can be configured (Facets 3.0)

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

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

After some fiddling, I settled on adding tests against all the available opt-in's except for the next major (doesn't exists at this time) + 'abuse' the 'previous minor' to test against the latest D10.3 version as there is no opt-in variable for the 'LTS' version.

This results in testing against all current D10, current stable D11.1 and D11.x-dev.

andreasderijcke → created an issue.

MR works nicely.

+1

andreasderijcke → created an issue.

andreasderijcke → changed the visibility of the branch 3520514-referencefieldextractor-not-checking to active.

andreasderijcke → changed the visibility of the branch 3520514-referencefieldextractor-not-checking to hidden.

andreasderijcke → created an issue.

We are going to support time field in the custom_field module itself, see ✨ Provide a time field Active .
Will be less hassle the other way around.

andreasderijcke → changed the visibility of the branch custom_field-3520227-3520227-provide-a-time to hidden.

andreasderijcke → created an issue.

The hardcoded field name mistake is fixed.

The nested parent detection and temp storage update/rework to make it all work in paragraphs, I need to let that rest for a bit to get get my head around it.

I've tested the current status in the project where the need orginated, with Media types for images, remote video,... it all worked as expected.

So,

• Removed the code dealing with multiple items within a field item
• Added the best solution/workaround I could piece together right now, for the add more/remove field items and the form state stored IDs issue
• Some refactoring for more strict typing
• Just the getEntityByTargetId() I'm still wondering about if we do need to keep it. It is used, but I do not yet have a clear picture on when
• Added some ignore patterns of errors we can't fix any time soon (as far as my experience goes) to phpstan.neon but commented out, as those errors only appear from level 6. Getting my code pass level 7 with only unfixable errors ignored, is one my current personal goals.

Feel free to add me as co-maintainer!
Happy to help.

About writing tests, I've made some in the past, but it's steep mountain to climb every time again.
But having specific cases, is a good way to start again.

At our company, we are having issues again after our hosting moved to more complicated setup of multi webserver per websites and a shared assets directory.

So far, the problem seemed always to be differences in JS dependency, 'claro/ajax' included by this module, to be specific (as we know so far), that should be included or not depending on the user's role (admin versus editor).
An implementation of the gist https://gist.github.com/webflo/67a380250486f8ab2ccacc73efe21406 seemed to have fixed it on a few projects.

However, a colleague reported the problem again on a layout builder route. Have not been able to pinpoint the mismatch in that case and which permission/context it is related to.

The hosting setup seems to be relevant in some way, as locally, we cannot reproduce this.
In the few occurrences that I managed to debug a bit, what seems odd is that both admin and editor got the same JS inclusions in cached responses, which are correct for one user but not there other. I'd expect these responses to be different (dynamic page cache) for these users, containing the proper JS inclusions for their role.
No clue yet if this is related to this or just a consequence of something else interfering or wrong in the base setup we use.

So, fixed other issue cause by the '$row_id' from the original that is replace here with the delta.
After some playing around with removing, replacing of a selection within a field item, things seems to work as expected.

Where we still do have issues, is when you start removing and adding new field items, you will see the new field item already prefilled with the selection previously stored for that delta.

But, before trying to fix this, I think it might be best to simplify all the code related to determining and storing the selection, as we always will have either a single item (or none) per delta, so we can get rid of the all array_map usages, some loops etc (+ also apply more strict typing where possible etc).
Once done, it might be easier to figure out how to deal with the remaining issues.

What is your take on this?

I was using Gin. I see what you mean now after switching to Claro which btw is a much clunkier UI with the buttons stacking like that.

Yea, that is also an issue on the original widget in Claro.
To make sure I understood you correctly, you're going to work on patch for Gin support for entity_browser module, right?

I'll leave it to you to refine further but its in a pretty good functional state at least for now.

Thank you, that was of much help!

I made the 'remove' button work, found an issue with the 'add another item' on the field itself + cleanup some code.

It is very likely there is more code we can strip, and some specific situations we have to handle properly or different compared to the original.

I realized for some reason the replace and edit icon are the same. When you disable the edit but enable the replace, the pencil icon still shows up and the replace functionality works on click. I don't know if we have something wrong in our implementation or if thats just the way it works... which would be odd

That sounds like a (admin) theme compatibility issue. Which theme are you using?
How does it look on a default entity reference field with the original entity browser widget?

Using Claro, there are no icons and the labels are correct.

The isApplicable() should maybe check if there are actually any entity_browser entities and that select field should be required

At the moment, I favour to not do that, so you'll be able to see the widget and its options and be triggered to configure some entity browsers if you don't have any. Otherwise, you might be wondering why the widget isn't shown as an option.

I noticed also an entity_browser can have an upload widget but it would be irrelevant if the entity_reference target_type is not one of file||image

As far as I know/understand things, upload is handled by/in the entity browser modal and this widget is just to expect the entity ID in return.

We don't need to check for the field_type otherwise in isApplicable() as its already defined in the plugin attributes.

Ah, great!

Related, entity_browser defines an adjusted widget for file and image. We might want to follow that, and keep this one to just 'entity_reference' (as you added file and image types)?

... i'm sure theres some settings that are irrelevant since we can only ever have 1 selection.

Yes, very likely.

As soon as the basic usage flow falls into place, I expect it will be more clear what pieces of can be scrapped.
For now, I'm keeping things I don't understand/have not checked or adjusted yet, for easier comparison with the original.

Related (again): I've heard there is some discussion going on about dropping support for submodules in contrib modules. (A necessity for project browser, also something related to composer/packaging issues, ...).
So perhaps, since this quite a complicated widget, it might be opportune to not merge it in the main module but release it separate, which should also make it easier to develop the main module without being 'dragged down' by complicated extensions like this and manage their dependencies.

It seems I was using 1.x branch of entity_browser as reference, so synced with the 2.13 release.

Managed to get selection (working with fixed cardinality of 1) saved and loaded on reload of the node edit form. But the 'edit', 'replace', 'remove' buttons of the entity browser are not shown properly, must be conflict of their (wrapper) markup with the parent widget wrappers.
That is on the list for tomorrow, as once fixed, it will be easier to fix and test the remaining gaps.

I will continue to work on this the next days during the Drupal Dev Days in Belgium.

It's a still work in progress!

Currently struggling with getting the selection in the entity browser stored and the entities rendered in the widgets.

andreasderijcke → changed the visibility of the branch 3519258-support-entity-browser to hidden.

andreasderijcke → created an issue.

Added:

• PagedImportSyncClientBase, PagedImportSyncClientInterface
• Example client SyncClientSimplePagedImport and job type SyncClientSimplePagedImport to explain how to use it.

andreasderijcke → created an issue.

Since this is a formatter, we can't validate on input, which I forgot in the issue description.
Therefor, proposed in the MR is:

• Some regex to strip characters the wa.me URL can't handle and massage the value towards the expected format.
• Added composer.json to suggest installing telephone_validation → for actual input validation

These two combined still allow for a zero between the country code and actual number, but that does not seem to be a problem for Whatsapp.

After reviewing and comparing the pipeline jobs and results with other modules, it turns out that

• OPT_IN_TEST_PREVIOUS_MAJOR
• OPT_IN_TEST_NEXT_MAJOR

are only useful if the module has unit tests. Otherwise, these will just trigger a composer install with that major version, which will succeed anyway if there is no composer requirement restriction for core.

andreasderijcke → created an issue.