Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
Problem/Motivation
The Entity Usage module is a dependency for the submodule "Paragraphs Library", and there's code that interacts with its API.
Entity Usage is going through a major rewrite and the 2.x branch (which is non BC) will be the only supported version in the future. We should plan to update the code in paragraphs library to support the 2.x branch when it's ready.
Proposed resolution
Update the code to use the 2.x API, and specify the minimum requirement in the info file / composer.json.
Remaining tasks
User interface changes
API changes
Data model changes
Comment | File | Size | Author |
---|---|---|---|
#17 | interdiff-14-17.txt | 942 bytes | marcoscano |
#17 | 2956907-17.patch | 8.21 KB | marcoscano |
| |||
#15 | 2956907-14.patch | 8.25 KB | marcoscano |
| |||
#14 | interdiff-11-14.txt | 2.06 KB | marcoscano |
#11 | interdiff-8-11.txt | 1.42 KB | marcoscano |
Comments
Comment #2
miro_dietikerCan we somehow depend here on some milestone issue for Entity Usage and postpone on its completion?
Comment #3
marcoscanoSure, we can consider #2956501: Update module documentation for 2.x and release 2.0-alpha1 as the last milestone for a 2.x release in Entity Usage
Comment #4
marcoscanoThis is what the change would look like.
Comment #5
marcoscanoEntity Usage 2.0-alpha1 is out :) so this is no longer postponed.
I also created #2957843: Provide a BC layer around 1.x helper methods so things won't break if we directly switch to 2.x. With that, we can even consider if this issue makes sense or not.
In any case, we might update the test because the returned count now is different, so the current tests will fail. Also, the message we show when editing a library item like "Library item used 2 times" will show an accumulated count (from all revisions / translations), which may confuse some users.
Comment #6
miro_dietikerSo yeah due to the dependency we should decide carefully. However, the Library is pretty new in Paragraphs and Entity Usage has limited adoption, so i guess there is not too much conflict potential if we push the dependency to >=alpha1 like you did.
Now that there is more functionality provided by Entity Usage itself, we might be able to switch to a soft dependency more easily?
IMHO we should focus on making Entity Usage (more) stable instead of trying to be backwards compatible.
Comment #7
BerdirI'm actually unsure if this should return the sum of usages or the distinct count of actually different sources/entities. So far it didn't matter, but I think it for the message, it makes much more sense to report that 3 actual entities are using it (and clicking on the message will then list throw rows which maybe 10 usages each).
Unsure about the removed test coverage, why is that done?
Comment #8
marcoscanoTrue, I've adapted the code to account for that, which is certainly better UX.
In entity usage 2.x we "fixed" the usage controller to skip paragraph items with no parents (in their default revision). The
(previous revision)
suffix was a side-effect of us not tracking revisions in 1.x, so testing for that doesn't make sense anymore in 2.x. (It may make sense again when #2954039: Fix ::label() logic and add a ParagraphInterface::getOrphanStatus() helper is fixed however).In any case, I have kept a test that ensures that no "broken" usage is shown (i.e. showing a paragraph row with no parent label).
Comment #10
Berdirwhy is this not showing the language label? :)
Still don't quite understand this, might be easier to talk this through in chat?
We are explicitly saving a new revision, so there *should* still be a usage from the previous version? With revisions now explicitly supported, that should work? Isn't #2954039: Fix ::label() logic and add a ParagraphInterface::getOrphanStatus() helper just about what label we are displaying if it doesn't exist in the current revision?
Comment #11
marcoscanoYep, that's another improvement for the list usage page on entity usage :)
(I've added it to https://www.drupal.org/project/entity_usage/issues/2952210#comment-12604196 so this is included as part of the effort to make this page more user-friendly)
About the empty label workaround, as discussed on the chat, the usage is correctly registered, but entity usage just skips the row if the source entity's label is empty. I've opened #2971131: Improve label handling on usage page to fix it more generically in entity usage, in case the label-related part of #2954039: Fix ::label() logic and add a ParagraphInterface::getOrphanStatus() helper is not fixed first.
I've also included here the fix for the test failures due to having our dev dependencies updated.
Comment #13
marcoscanoOoh didn't realize we had a WebTest! :) I'll work on updating those next.
Comment #14
marcoscanoComment #15
marcoscanoComment #16
Berdircount($entities) will do the same, without a second loop :)
Comment #17
marcoscano🤦 sorry for missing that.
Thanks!
Comment #18
BerdirTested manually, seems to work fine.
Comment #21
miro_dietikerVery nice then. Time to look forward.