Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem/Motivation
It should be possible to initialize an entity browser with an existing selection. There are a few issues because the selection plugin is not necessarily bound to a specific url structure and its up to the selection plugin how the existing selection is presented to the user. Some selection widgets could handle multiple items (e.g. views) but some plugins are restricted so a single entity (e.g. entity_form)
Proposed resolution
Remaining tasks
User interface changes
API changes
Data model changes
Comment | File | Size | Author |
---|---|---|---|
#27 | interdiff.txt | 639 bytes | slashrsm |
#27 | 2656196_27.patch | 21.14 KB | slashrsm |
#26 | interdiff.txt | 3.12 KB | slashrsm |
#26 | 2656196_25.patch | 20.93 KB | slashrsm |
#24 | interdiff.txt | 18.35 KB | slashrsm |
Comments
Comment #2
webflo CreditAttribution: webflo at UEBERBIT GmbH commentedComment #3
webflo CreditAttribution: webflo at UEBERBIT GmbH commentedRelated entity embed issue #2656206: [entity_browser] Pass selected entity back to Entity Browser (when editing an existing embed)
Comment #4
webflo CreditAttribution: webflo at UEBERBIT GmbH commentedComment #5
webflo CreditAttribution: webflo at UEBERBIT GmbH commentedComment #6
slashrsm CreditAttribution: slashrsm at Examiner.com commentedFew comments:
If there are more entities in the current selection we always edit just the first one. Would be nice to figure out how to support editing any of them.
Recently we added "multi step" selection display, which allows reordering and removing of entities in the current selection. Maybe we could incorporated edit in there too.
Didn't manage to really test this to find out by myself? How does edit functionality related to ordering of current selection. Selected entities are generally appended to the end of the list? Will the same happen in the edit case? Will this cause duplicated entities appearing in the list?
Doc blocks missing.
Would it make sense to incorporate some kind of hash into this (for security reasons)?
This might break some other stuff (field widgets that @berdir created for file entity). Not a deal-breaker, but more of a note.
Also, https://github.com/drupal-media/entity_browser/pull/140 might be related.
We will need to add this to the Modal too.
Would be nice to inject uuid if continue going forward with this approach.
Comment #7
killua99 CreditAttribution: killua99 commentedThis patch would work on the current dev code? I'll like to try it out.
Comment #8
webflo CreditAttribution: webflo at UEBERBIT GmbH commentedComment #9
slashrsm CreditAttribution: slashrsm at Examiner.com commentedThis is definitely a feature that we want to have (and was planned since the beginning). Main problem that I have with the current approach is the fact that it is coupled to one specific widget type. I think we should architect this in a more general way. I also think we should split this into subtasks. This way it will be easier to wrap our heads around the problem and solve each individual problem separately. Divide and conquer! :)
Step 1: Allow passing existing selection on init
This is mostly done in #8. While there are other way to do it temp storage seems to be good approach. I was thinking about adding a configuration variable which would allow is to enable/disable this functionality.
We will also need to update selection propagation at the end of EB flow. Currently we append selected entities to the entities that might already exist on a parent form. That won't make sense any more when we implement this. I thought we could keep the current approach when there are no existing entities and replace selection entirely when there are.
Step 2: Allow passing existing selection from field widget
Depends on: step 1
We will need to update entity reference field widget to be able to propagate existing selection to the EB. We would make this configurable.
Step 3: Selection displays
Depends on: step 1
Selection displays need to handle existing selection. Multi step should work out of the box as it will read form state and find entities there. With "No display" there are few things we have to think about.
No display won't display existing entities by design. User won't have any idea which entities are already selected and will also have no way to do anything with them. That made me think if we should allow selection display plugins to declare whether they can accept initial selection or not (we could do this on annotation level).
If we go down that route we need to decide what to do if a selection display that can't handle this is used and entities are propagated anyway. Ignore them? Ignore them and display a message? Throw exception? I'd prefer loud and clear exception. This way we'd let people know that their configuration isn't right and they should fix it. This will also solve situation where entities are propagated while this feature is disable in configuration (Step 1).
Step 4: Editing entities
Depends on: steps 1 and 3
At this point we're able to initialize EB with existing entities. We're also able to reorder/add/remove them using multi step display. We should also able to edit them (second part of patch #8). This should be architected in a way that will allow any widget to be used as edit form. Of course, in some cases it does not make sense to allow that (how can upload tool edit file entities?). We should allow widget plugins to declare if they are able to edit for that reason (again, annotation).
Selection displays would be responsible to decide if we're editing and which entity should be edited. Multi step plugin could have "Edit" button for every displayed entity and clicking on it would trigger edit for it.
We will also need to decide which widget needs to be used for edit. We could take different approaches here (widget that is currently displayed, globally configured widget for edit, some kind of negotiation mechanism, ...) This should be up to the widget selector plugin and it's implementation.
Entity embed
I see two possible approaches for entity embed.
1. When embed is edited and entity browser is re-opened we select new entity and completely ignore existing one.
2. When entity browser is re-opened we automatically display edit of the embedded entity.
Case 1. is already possible and will be possible after this lands. Case 2. will need specific entity embed selection display plugin to handle specific business logic. While this might should cumbersome I think it is OK. We created this plugin types to allow this kind of fine tuning of the experience in the first place.
How does this sound? I am happy to start with step 1 (based on relevant hunks from #8).
Comment #10
webflo CreditAttribution: webflo at UEBERBIT GmbH commentedSorry no progress on proposal from #9. Just a re-roll
Comment #11
webflo CreditAttribution: webflo at UEBERBIT GmbH commentedComment #12
webflo CreditAttribution: webflo at UEBERBIT GmbH commentedComment #13
webflo CreditAttribution: webflo at UEBERBIT GmbH commentedComment #18
chr.fritschA little bit more WIP
Comment #21
slashrsm CreditAttribution: slashrsm at MD Systems GmbH for Acquia commentedReroll.
Comment #24
slashrsm CreditAttribution: slashrsm at MD Systems GmbH for Acquia commentedTakes step by step approach that I proposed in #9. It implements first step from that list, slightly changes approach in few areas, fixes test fails, adds test coverage.
Comment #25
phenaproximaUm...I don't think the class key in a service definition can be an interface, because interfaces cannot be instantiated :) Yet the test gets this service, and apparently doesn't break. What is this devilry?
Should be marked (optional).
Nit: There's an extra line of whitespace.
Comment #26
slashrsm CreditAttribution: slashrsm at MD Systems GmbH for Acquia commentedFixed #25.2, #25.3 and made sure key-value entry will expire after reasonable time.
WRT #25.1... If I remove it Symfony complains:
So something needs to be set. I tried to change it to a complete nonsense (non-existing class) and it still worked. Go figure.... It seems that the interface is actually the best option.
Comment #27
slashrsm CreditAttribution: slashrsm at MD Systems GmbH for Acquia commentedExplaining the class value in the service definition.
Comment #28
phenaproximaWorks for me.
Comment #30
slashrsm CreditAttribution: slashrsm at MD Systems GmbH for Acquia commentedCommitted. This covers #9.1. Let's continue with the steps that follow.
Comment #31
slashrsm CreditAttribution: slashrsm at MD Systems GmbH commentedCreated follow-ups:
#2767741: Allow editing of entities
#2767737: Allow passing existing selection from field widget
#2767739: Define which selection displays support existing selection and which not
This can be closed in my opinion.
Comment #33
Gábor Hojtsy