-
-
Notifications
You must be signed in to change notification settings - Fork 549
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[5.x] Fix term reference updates after slug change #11058
base: 5.x
Are you sure you want to change the base?
Conversation
I've just switched a production site from flat-file to eloquent — the suggested fix in this PR is only required for flat-file data. Terms saved in eloquent seem to trigger a reference update as intended. The good news is that it also doesn't hurt :) |
I wish I had a more consistent way to reproduce this. I saw it happen once, then couldn't figure out how to do it again. I think the issue is that there's a certain situation where a term doesn't have If you can provide a repo with this reproducible that'll be amazing. |
@jasonvarga Strange. I can reproduce this on every save on three production sites and on a vanilla install, locally and on a VPS. Not sure what I‘m doing wrong to reliably trigger this. I‘ll create a vanilla install and share the repo here :) |
@jasonvarga I've uploaded a reproduction that reliably shows the issue at daun/statamic-term-update-repro. It's a vanilla install with the default |
@jasonvarga Now we have an ability to reproduce the issue, can we please re-open this issue? As described here and #11264 |
@jonathan-bird Does this PR reliably solve the issue you're seeing in #11264? |
@daun Not sure. I can't reliability replicate the issue with my client's site and I had to manually fix theirs as per my Github issue. |
Fix an issue where updating a term's slug through the control panel would not update the references to that term in existing entries, even though
system.update_references
is enabled.The root of the issue appears to be that the auto-bound Term instance in the CP controller lacks its original data because there is no initial
syncOriginal()
call. The first call tosyncOriginal
happens only after saving, which is not helpful here. I’m not sure why this doesn't also happen with Entries, but it seems to work there.The existing tests haven't caught this as it only happens in the CP controller, not when creating and updating terms in a test environment. Would love to contribute tests here as well, but that's a bit over my head how to test this reliably across the CP routes/controllers.
Closes #10938
Related: #11021