- 🇬🇧United Kingdom scott_euser
Correct if you are using latest merge request. The automated test coverage has to do the same thing in order to test the filter, so you can see that code here applying to all entity reference fields that are currently set to use string or numeric filters: https://git.drupalcode.org/project/drupal/-/merge_requests/4053/diffs#2f...
With such code applied (replacing with your own module name instead of that module which is named "views_test_entity_reference"), after clearing cache, adding a new filter with entity reference should show you the entity reference options rather than numeric options for example.
- 🇨🇦Canada jodavidson
Thanks @scott_euser.
If I understand the summary notes. I create a basically empty custom module that really only has the hook_views_data_alter() snippet in its MODULE.module file. And the patch should take effect once the custom module is activated?
- First commit to issue fork.
- 🇺🇸United States en-cc-org
Our site is also frozen at 10.2.7, but (not realizing patch #453 would work), I applied this patch https://www.drupal.org/files/issues/2024-04-17/3347343-comment156-10.2.x... → As mentioned, this patch required me to rebuild my views to use the new "field_name as a reference filter". All is stable for now. Thankful to those continuing to find solutions as we are very dependent on this feature.
- 🇬🇧United Kingdom scott_euser
Hi @jodavidson,
Essentially the problem with making a separate reference field (the 'as a Reference' that existed in some versions of the patch) would be very hard to merge back later.
Then the problem of replacing all existing Entity Reference filters on place is that the current Numeric filter offers more options (like all node IDs > 100) that would be lost and therefore it'd be a breaking change.
So the current status is to provide the Entity Reference filter but not actually make use of it (some follow up tasks to allow a content editor to use it via the UI). Splitting into follow-ups means we can actually get it to land (hopefully) but making this scope significantly less risky.
There is a code snippet in the issue summary now that shows how to make use of the latest merge request, but assuming we get framework manager approval I'll create a module in contrib that effectively does that for now until the follow-ups land (on the framework manager approval @catch noted he wants other eyes to grant that since he's the one who came up with the idea of the above).
So tiny bit of limbo state as I don't want to go make that contrib module unless we are quite confident this will be the agreed direction.
Hope that helps clarify at least (I know it doesn't solve sorry!)
- 🇺🇸United States mmenavas
My site is also frozen at 10.2.7. Let us know how the community can help out.
- 🇨🇦Canada jodavidson
Hello all,
With all the twists and turns this issue has taken I wonder if there is a current solution to getting the Settings option back in place for exposed entity reference filters?
For 10.2.7 the patch here: https://www.drupal.org/files/issues/2021-12-02/2429699-453-9.3.x.patch → worked well. In 10.3.1 I've made a local patch from MR !4053 mergeable (above) and applied it. Unfortunately it does not return the Settings option. Looking at the past few comments maybe it is not supposed to by itself?
Reading through these issue chains is pretty confusing for me, (and many others I imagine,) so I thought it best to ask for some clarification. Personally my site is frozen at 10.2.7 until there is a solution for this issue.
Thanks for your time and work on this issue, it sure looks like a big can of worms that has existed since D8.
- 🇬🇧United Kingdom catch
Reviewed the MR and it all looks good to me.
Retitling because the latest approach doesn't make it available for all entity reference fields yet, that bit will happen in ✨ Configurable views filters to allow for different widgets Active .
Leaving the framework manager review and subsystem review tags because the latest approach was my idea and this issue has a lot of twists and turns.
- 🇳🇿New Zealand quietone New Zealand
Only did a rebase and some docs updates because of request in Slack. I did notice there are more methods that need documentation, that declare strict types hasn't been added and that not all method arguments have type hints. I can't help with that now.
- First commit to issue fork.
- 🇬🇧United Kingdom scott_euser
Thanks! Updated issue summary post tasks as per #218
- 🇬🇧United Kingdom catch
This feels like it might unblock things. Short version of how we get to the original end result assuming everything currently being worked on comes together:
1. Add the new filter here, but not automatically to views data yet.
2. Do 🐛 Views handler loading should respect configuration Active so configuration is respected in views.
3. After #2, do ✨ Configurable views filters to allow for different widgets Active to allow the filter plugin to be selected in the UI. This would be an extra UI element in views, plus some kind of API for plugins or fields to declare compatibility with each other tbd.
The for entity reference fields, there'd be a choice of numeric and entity reference, and site builders can choose which one, and we don't have an upgrade path or bc concern specific to the entity reference change - although there is still quite a lot of that in #2 and #3 overall.
- 🇬🇧United Kingdom scott_euser
As discussed in Slack here as suggested by @catch, the change to views data & views data alter has been reverted and the EntityReference filter is made available without being used, allowing it to be used in place, rather than with the 'as a Reference' duplicate filter.
The issue summary has been updated with a code snippet of how to use and a contrib module automatically opting into this will be provided assuming the direction is accepted and ready to merge. That contrib module could also be used to provide any upgrade path away from the 'as a Reference' in case any sites are making use of the patch.
Tests have been updated to match without any functional change to the tests other then to stop referring to the _reference/'as a Reference' aspect of the filter.
The issue summary has also been updated to match this change in general.
Finally, the change record has been updated, though could use someone to update the screenshots when this is ready to merge.
- 🇳🇿New Zealand quietone New Zealand
The release managers discussed this at the end of May and I am only now commenting. We noted that this is part of the Workflow Initiative but that initiative has ended and there isn't anything here for us to review. We also are aware that there is the Trash module and a core issue to move that to core, 📌 Add experimental Trash module Active .
This issue was also closed; as won't fix in 2017 and then re-opened in 2019. But since then there has been no activity here despite a prompt for more information 2 years ago. After 5 years without comment here from someone from the workflow initiative it seems sensible to restore the won't fix from 2017.
And, if that there is a desire to add this to core, then it would be better to start fresh with a new initiative or plan. Therefore, I am going to close this. I trust someone will correct that if it is wrong.
- 🇬🇧United Kingdom catch
Also if we were to add this, I think it should be its own filter doing this generally for all site-relative links, not within the file reference filter.
For example https://www.drupal.org/project/rel_to_abs → provides this in contrib.
- 🇬🇧United Kingdom catch
The issue summary doesn't explain the problem with relative URLs, why is this a problem?
Additionally, Drupal sites can be run from multiple domains, using domain module and similar. Relative URLs allow this to work, I don't think we're able to cache text formats by arbitrary cache contexts so it would probably break that.
- 🇬🇧United Kingdom catch
Moving this to a plan.
We discussed the general problem (on-the-fly table creation and transactions) at Drupal dev days but I don't think that discussion made it back into an issue, this is probably the right one.
Generally, I still think it's a good idea to make database-implementations-of-services (cache, lock, flood etc.) responsible for creating their database schema, so that if you have a redis backend installed, you don't have 10 empty cache tables in your database for no reason.
However, we could use an interface + tagged services to allow those services to create their schema during module install at an explicit point, so that the fallback is only necessary if you change services.yml to switch an implementation (e.g. from redis to sql) on runtime. And we could maybe do the same tagged services check on a cache clear all so that sites can trigger table creation without having to wait for an entity save or something to do it for them.
This way we'd still have on-demand table creation but we'd be able to... demand it.
- 🇳🇿New Zealand John Pitcairn
Note
hook_module_implements_alter()
will not work if you need yourhook_preprocess_HOOK()
to run after some other module's preprocess hook. Preprocess hooks are not invoked by the module handler and are not present inhook_module_implements_alter()
.So removing this API will be problematic if you have a soft dependency on another module's preprocess hook, ie the module is not a requirement, but if it is present you need to preprocess something it also preprocesses, after it has done so. You don't want to declare a hard dependency in module .info. Altering module weight is the only way this will work, right?
- 🇬🇧United Kingdom longwave UK
There are a few merge conflicts and some missing docblocks but otherwise to me this is ready to go - still needs framework manager signoff though.
- 🇷🇸Serbia finnsky
This hits performance mostly because I've added lost drupal/message dependency
- 🇺🇸United States smustgrave
The fact this seems to hit performance is it still a task to do?
- 🇷🇸Serbia finnsky
I also added an experimental branch with web components. I believe that since Umami is an experimental topic, we can easily try this technology here. Browser support suits us.
The benefits we are achieving now:
- Both message methods (Drupal render and javascript theme function) use the same template.- Drupal has an example of using web components in its core and keeps up with the times
Please review!
- @finnsky opened merge request.
- 🇷🇸Serbia finnsky
One more bug.
Seems messages_list wrapper missed. So new messages added directly inside existing message
https://gyazo.com/dff6c11b71c3fbe18661379e45e01b8f - @finnsky opened merge request.
- 🇷🇸Serbia finnsky
Found one more bug. When Login with wrong password.
https://gyazo.com/6e5b6146b61a8d1d65249614dcb67f78
It not happends on
1. Olivero because it attaches library in template
https://git.drupalcode.org/project/drupal/-/blob/11.x/core/themes/oliver...2. Claro because it has direct dependency
https://git.drupalcode.org/project/drupal/-/blob/11.x/core/themes/claro/...Gonna add dependency aswell.