-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
Add utility method for explicitly setting cache tags to cache tag class #12495
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
base: main
Are you sure you want to change the base?
Conversation
Documentation build overview
Show files changed (2 files in total): 📝 2 modified | ➕ 0 added | ➖ 0 deleted
|
I think this likely also might have downstream permissions issues on .com. We should double check that these aren't being cached on .com. |
This only affects cache tagging for purging, so shouldn't have an effect on caching. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is going in a good direction. I haven't done a deep review yet but I understand that the idea is to change the login from "leave the mixin to handle cache tags (after implementing 2 methods)" to "set the tags manually by the class itself", right?
I'm not opposed to that, I think it's more explicit and easier to digest as well.
I don't have all the implications of this change in mind. Do we need to make changes in .com as well? I think @stsewd is a good candidate to chime in here since I think he has implemented this code originally.
This was the first attempt to fix #12494, then was broken up into multiple stages of work. This is a very minor method addition to the cache tag class, used when project/version are not set on the view instance at the class level. This was an issue in multiple views.