The stored config does not have correct ID when provider != entityTypeId. Example:
Old media, provided by media_entity.
* form saves auto_entitylabel.settings.media_entity.image
* hook looks for auto_entitylabel.settings.media.image
The stored config does not have correct ID when provider != entityTypeId. Example:
Old media, provided by media_entity.
* form saves auto_entitylabel.settings.media_entity.image
* hook looks for auto_entitylabel.settings.media.image
Comments
Comment #2
geek-merlinTrivial patch flying in that should fix this.
Comment #3
geek-merlinThat's the one. Huh, variable naming is quite obfuscated here...
Comment #4
geek-merlinComment #5
guptahemant commentedHi @Axel.rutz,
I am adding all the related issues from which this patch has an effect on.
Comment #6
purushotam.rai commentedIn that case, do we need entityTypeProvider anymore @axel.rutz and @guptahemant?
Comment #7
geek-merlin> In that case, do we need entityTypeProvider anymore @axel.rutz and @guptahemant?
Good point! I looked into this and found an ugly can of worms and needs-cleanup.
You can find everything in attached patch.
Note: To keep this reviewabe, this patch contains 6 simple commits, that you can see by doing:
git am <auto_entitylabel-2923876-7-Broken-when-provider-entityId.patchComment #8
OnkelTem commentedJust fixed the same locally for ECK and only then found this issue. Because it's hard to find the issue by name until you don't know what to patch :)
Ok, this patch is more elaborated anyway so I'm going to check it out.
Comment #9
OnkelTem commentedWorks. Let me set RTBC to indicate it worked at least once! ;-)
Comment #10
kclarkson commentedpatched applied cleanly. Needed this patch to use eck auto title.
Comment #11
mparker17I can confirm the patch in #7 still applies cleanly to commit
b222273(HEAD of8.x-2.xas of 2018-02-16).Comment #12
daggerhart commentedThe patch in #7 fixes the problem, but I have some minor concerns.
1. Since this module is in beta do we need to provide an update hook to automatically correct any saved configurations that have the wrong keys?
2. Also the patch is very confusing to read because it's 6 separate patches all applied one after the other. Ideally this is rolled into a single patch, so I've done so and attached it.
Comment #13
daggerhart commentedComment #14
Anonymous (not verified) commentedHey this works with ECK when I have it patched with https://www.drupal.org/project/auto_entitylabel/issues/2829571!
Per the comment in #12 about update hook, I wouldn't think so in this case. Since this was technically broken before, configs that may have been saved would be orphaned and then when fixed they would change to the correct one, right? They probably have to get fixed manually regardless since if you're changing labels on custom entities you'd have #2829571 patched already, which isn't committed yet.
If I'm wrong an update should be provided, but otherwise I will RTBC this so we can get moving on the other issues.
Comment #15
scottrigbyRTBC as well, same as #14
Comment #16
geek-merlinRe #12: #7 was intentionally a multipatch:
> To keep this reviewable, this patch contains 6 simple commits
The rest is up to the maintainer.
Comment #17
joelpittetThis looks to need a re-roll
Comment #18
hanoiiReroll of #7 for latest dev, #7 didn't apply. I do think it's easier to review a single patch that a bunch fo separate commits, and also it makes it for a more compatible patch, one that can be referenced with composer-patches. #7 might do as well, but I'd think not.
Comment #19
seth.e.shaw commentedAny word on when this patch will get applied? It reportedly fixes https://www.drupal.org/project/auto_entitylabel/issues/2926779 which would be really useful.
Comment #20
kevinsiji commentedThe patch in #18 fixed the issue of taxonomy term auto title. Anyway, the token browser lost the tokens of fields in the taxonomy term. Before changing to dev version, the tokens where available in the token browser.
Tokens work while generating the title, only the browser is missing the tokens.
Comment #21
bwoods commentedIt appears that this patch no longer cleanly applies to 2.x-dev. In AutoEntityLabelForm.php, #theme, #token_types and #dialog are included in a variable called token_link. Using the patched value for #token_types still does the trick to correct. I also ran into the same problem from #20, so I pulled the tokens from pathauto patterns instead.
Comment #22
geek-merlinIt's a pity that maintainers didn't commit this when it was rtbc, as it is a quite complex patch. Note: A multipatch like #7 usually is easier to rebase too.
Comment #23
manuel.adanCurrent maintainers seems to be busy. If someone else could bring some help, just open an issue offering as (co)mainteiner.
Comment #24
panchoRerolled the three functional commits within patch #7 against latest dev.
Left out coding standards and further minor improvements (commits 4, 5, and 6) to keep the patch reviewable and avoid missing the target again and again. Those may go into a followup.
Comment #25
panchoSame patch as in #24, but with the other patch from #2945361-3: Clean up AutoEntityLabelForm being rolled in.
Note that from patch #7, commits 4 and 5 are obsolete now, while commit 6 (renaming some variable names) seems debatable.
The patch to be reviewed is still the one in #24. This one is just to have a cleaner starting point for further changes.
[edit: This one actually belongs in #2945361: Clean up AutoEntityLabelForm. Please disregard for the moment and stick with #24]
Comment #26
panchoComment #27
panchoFixed taxonomy tokens per #20/21.
Comment #28
panchoMinor indentation fix.
Comment #29
dakwamineTested #28.
Applies on dev branch, not on beta1.
Works great.
Comment #31
colanThanks!
Comment #32
geek-merlin