Comments

axel.rutz created an issue. See original summary.

geek-merlin’s picture

Status: Active » Needs review
StatusFileSize
new869 bytes

Trivial patch flying in that should fix this.

geek-merlin’s picture

StatusFileSize
new1.65 KB

That's the one. Huh, variable naming is quite obfuscated here...

geek-merlin’s picture

Title: Broken when provider !=entityId » Broken when provider !=entityTypeId
Issue summary: View changes
guptahemant’s picture

Hi @Axel.rutz,
I am adding all the related issues from which this patch has an effect on.

purushotam.rai’s picture

In that case, do we need entityTypeProvider anymore @axel.rutz and @guptahemant?

geek-merlin’s picture

> 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.patch

OnkelTem’s picture

Just 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.

OnkelTem’s picture

Status: Needs review » Reviewed & tested by the community

Works. Let me set RTBC to indicate it worked at least once! ;-)

kclarkson’s picture

patched applied cleanly. Needed this patch to use eck auto title.

mparker17’s picture

I can confirm the patch in #7 still applies cleanly to commit b222273 (HEAD of 8.x-2.x as of 2018-02-16).

daggerhart’s picture

StatusFileSize
new5.29 KB

The 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.

daggerhart’s picture

Status: Reviewed & tested by the community » Needs review
Anonymous’s picture

Status: Needs review » Reviewed & tested by the community

Hey 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.

scottrigby’s picture

RTBC as well, same as #14

geek-merlin’s picture

Re #12: #7 was intentionally a multipatch:
> To keep this reviewable, this patch contains 6 simple commits

The rest is up to the maintainer.

joelpittet’s picture

Status: Reviewed & tested by the community » Needs work

This looks to need a re-roll

hanoii’s picture

Status: Needs work » Needs review
StatusFileSize
new5.5 KB

Reroll 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.

seth.e.shaw’s picture

Any 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.

kevinsiji’s picture

The 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.

bwoods’s picture

It 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.

geek-merlin’s picture

It'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.

manuel.adan’s picture

Current maintainers seems to be busy. If someone else could bring some help, just open an issue offering as (co)mainteiner.

pancho’s picture

StatusFileSize
new2.18 KB

Rerolled 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.

pancho’s picture

Same 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]

pancho’s picture

pancho’s picture

Fixed taxonomy tokens per #20/21.

pancho’s picture

StatusFileSize
new2.31 KB
new995 bytes

Minor indentation fix.

dakwamine’s picture

Tested #28.

Applies on dev branch, not on beta1.

Works great.

  • Pancho authored c057e50 on 8.x-2.x
    Issue #2923876 by Pancho, axel.rutz, daggerhart, hanoii: Fixes for when...
colan’s picture

Status: Needs review » Fixed

Thanks!

geek-merlin’s picture

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.