Closed (fixed)
Project:
Page Manager
Version:
8.x-1.x-dev
Component:
Code
Priority:
Critical
Category:
Bug report
Assigned:
Unassigned
Reporter:
Created:
9 Sep 2015 at 18:39 UTC
Updated:
24 Sep 2015 at 16:04 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #2
tim.plunkettThis fixes it, but seems *extremely* heavy-handed.
Comment #3
dsnopekSo, I don't know smart cache at all, but could you do something like this:
... except with (a) the right API and what-not (since you're not being passed the entity object) and (b) making sure we actually have that cache tag set somewhere (which I don't think we do right now).
Comment #4
tim.plunkettAs far as I can see, this should fix it. But it doesn't...
Comment #5
tim.plunkettF*&%ing priorities... This should pass.
Comment #6
tim.plunkett#2566019: [PP-1] HtmlResponseSubscriber and DynamicPageCacheSubscriber should have negative priorities so that vanilla response subscribers can have priority zero
Comment #7
wim leersThis is a very generic cache tag. I'd say it should be
page_manager_route_name:$route_nameor something like that, to prevent a conflict in case core ever does something like that.Otherwise looks great :)
Comment #8
dsnopek@Wim Leers: Tim and I discussed on IRC last night that this is something Views might need as well (Tim's going to do some research/testing today) so it very well might make sense for core to do this.
Although, would this really be a "conflict"? Isn't adding a cache tag that's already there just a no-op? If core later starts adding this same cache tag, then the subscriber could be removed and no other code would need to be change, which might actually be a good thing. :-)
Comment #9
tim.plunkettI think @dsnopek has a good point, in that ideally this would be harmless to squat on the namespace. But it's Not The Right Way™, so I'll commit this with a namespaced tag to get tests passing, and then follow up on a core issue.
Comment #15
wim leersHah! Genius! :)
Comment #16
tim.plunkett:)
There are actually other bugs still, just missing test coverage. Will open a new issue shortly.