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.
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.
andreasderijcke → created an issue.
andreasderijcke → created an issue.
Added a first proposal for views filter for TimeRange field, as extend on the core numeric filter:
- Scrapped the regex operations, as they make no sense with time input.
- The 'is empty' operation does not work if there is no field record at all, see comment in code.
- Definitely needs more work and testing.
In my case it's a view of nodes with paragraph having the timerange field.
While writing this down, I realise I've been confusing the 'from' and 'to' input of the between filters with the 'from' and 'to' from the TimeRange field, and the filter would just as well work on the regular Time field.
If that's true, then rename of the filter class is recommended.
andreasderijcke → changed the visibility of the branch 3029652-add-functionality-to to active.
andreasderijcke → changed the visibility of the branch 3029652-add-functionality-to to hidden.
andreasderijcke → changed the visibility of the branch 3029652-add-functionality-to to hidden.
I've added the file with following variables overwritten:
- SKIP_ESLINT: as the module does not have any JS.
- OPT_IN_TEST_PREVIOUS_MAJOR: to test against D10 too (at this moment)
- OPT_IN_TEST_NEXT_MAJOR: to test against D12 as soon as the branch is started.
Also added the .cspell-project-words.txt file using the artifacts from the CSpell run.
andreasderijcke → created an issue.
I cannot reproduce the case, but the additional check makes sense and should not break anything, in contrary.
andreasderijcke → made their first commit to this issue’s fork.
That patch makes indeed sense.
I'm just wondering if there are other situations where the current (content) language will not match entity (translation) being edited or created.
In my language (detection) setup, the $entity->getTranslation() is returning the default translation of the entity because of the fallback logic in \Drupal\Core\Entity\EntityRepository::getTranslationFromContext().
Any module affecting this fallback logic, could indeed lead to different result.
@swentel: What's the context of your case?
Inline entity form perhaps?
Ah, sure. I guess "if interface and content language differ" is indeed to vague.
Something like this:
(The "Selected language" is identical for both interface and content, as-in they share the value, set to default site language).
Point is to get to active language for interface and content to differ, one way or another.
That this can happen is often overlooked, both in core and contrib. Core does not always separate interface and content properly either, many issue about that.
Just imagine, you have editors with different native languages, and they each one wants to have the interface in their own preferred language (hence the "Account administration pages" enabled for interface) but they all managed content in all available languages.
So, in my specific case, I tried to edit an existing node with custom field with only NL translation (source) while having the interface forced to EN.
In that case, the code (pre) patch would use EN as default language to load the entity in for access checking, while it might not exist (yet) in that language. The content language does return NL in that case, since we're indeed editing the entity in that language.
PS: While trying to post the commit, the issue was already set to fixed. But submitting this anyway for future reference.
I've expanded the patch, as the previous version was did non suffice on D10.4 when the widget is hidden on entity translation forms due to the field being not translatable.
Core field constraints on the field are still fired, thus blocking creation of the entity translation. In the process, access check on the field returns 'allowed' causing the field to be taken into account for validation, even though it should not.
Have not found a solution to prevent this from happening, but discovered along the way that when the widget is hidden, the field submission values contain the options and current value, which cause the field constraint validation to fail.
By massaging the field submission values, or cleaning, this can be prevented and translation submission proceed.
This solution is added to the MR, including note that it is only a workaround until a better/real fix can be found.
andreasderijcke → made their first commit to this issue’s fork.
andreasderijcke → created an issue.
Different patch as MR, because I think the base check can be simplified.
In addition, the is_numeric check should not be necessary according to the return type of getDomainId(), so either the check is obsolete, or the function doc is incomplete.
As I don't have the correct answer, have flagged it as a todo.
andreasderijcke → made their first commit to this issue’s fork.
andreasderijcke → made their first commit to this issue’s fork.
Fix by releasing 1.1.0 → .
Thanks for reporting and the proposed solution.
I will start a new minor version for D11, as many modules do. At the time, I didn't realise this change in ConfigFormBase was this impactful.
andreasderijcke → changed the visibility of the branch 3501197-using-error-level to active.
andreasderijcke → changed the visibility of the branch 3501197-using-error-level to hidden.
andreasderijcke → changed the visibility of the branch 3501197-using-error-level to hidden.
Fix for both D11 too.
Tweak added. Didn't think of it, even having used it before.
andreasderijcke → changed the visibility of the branch 3508066-error-call-to to hidden.
andreasderijcke → created an issue.
I made the checks a bit more specific, as it didn't suffice for Layout Builder. The getValue() was still reached.
It would be even better if we could check the object class instead of presence of the getValue() method, but I don't know which one is expected.
andreasderijcke → made their first commit to this issue’s fork.
@stephencamilo The require-dev dependencies are only for development of the module and running tests that (might) test support and cooperation with those modules.
Those modules are not mandatory for this module to work, so it is correct that they are not listed in the info.yml file. As far as i can tell, both the info.yml and composer.json file are fine as they are.
Which features are you using that are causing a problem?
Still applies in combination with 🐛 MailjetConfigurationAccessCheck triggers on all routes Active .
I want to raise this to critical, because this problem can make a website completely inaccessible (access denied on all routes)
AccountInterface instance can have no ID in valid use cases, even though the AccountInterface does not say so (but should).
A good example is the Search API RenderedItem processor: a User(Session) is created on the fly to render and index entities for that user's role. This user is never saved, thus has no ID.
See https://git.drupalcode.org/project/search_api/-/blob/8.x-1.x/src/Plugin/...
Should the AccountInterface get updated to reflect the fact that the ID can be NULL, it still needs to be handled here.
Thanks!
No, was just a test to see if changing the issue credits assignment would stick, as a thank you for the work done.
Smart Date →
is also conflicting.
One way to 'fix' this, is by increasing the weight of the module in core.extensions.yml, not without risk however.