I have an issue filed against Panels IPE that introduces a new hook, hook_panels_ipe_tempstore_id_alter(), which is intended to be a permanent solution to a problem that is hurriedly papered over by a dirty hack of a patch that will never be committed to Panels, and rightfully so.
The basic gist of it is that in order to prevent very bizarre UX -- the kind of head-explodey WTF that will make users think Panels is broken -- Panelizer needs to be able to influence the tempstore ID used by Panels IPE for storing changes that have not been saved or discarded yet. #2793801: Allow modules to influence the IPE tempstore ID opens the door to that by providing a new hook; this patch takes advantage of that hook to nicely integrate Panelizer with Panels IPE.
| Comment | File | Size | Author |
|---|---|---|---|
| panelizer-panels-ipe-tempstore-id.patch | 747 bytes | phenaproxima |
Comments
Comment #2
dsnopekShould the tempstore ID include the entity revision as well? While probably not advisable, Panelizer/IPE can be used on revisions other than the most recent one. Although, including the revision might create more problems if multiple people are trying to edit the display for the same entity... Anyway, just something to think about - I don't really know what the right choice is.
Comment #3
samuel.mortensonThis patch breaks IPE, if you "Toggle preview" then "Add" on any Block Plugin form, saving the page will not work.
Comment #4
samuel.mortensonLooking at what this code is trying to do, it seems like you're wanting the TempStore ID to always be based on the Entity's UUID. Panelizer already determines what TempStore ID to use on load, so implementing a Panels IPE hook might not be necessary.
Would changing the logic at http://cgit.drupalcode.org/panelizer/tree/panelizer.module?h=8.x-3.x#n63 to always use Entity-specific Storage ID accomplish what this patch is doing?
An example scenario of how this would work:
@dsnopek The issue this is trying to address is that when a user starts editing a Panelized Entity using a default display using Panels IPE, we don't know what they intend to save their edits as. So we assume that if they loaded the default, they're going to save as default, and as a result multiple users cannot collaborate on editing multiple Panelized Entities using default displays at the same time (they're all using the same TempStore ID). If we make every TempStore ID unique to each Entity, we prevent overlap between users. Does this sound like an OK solution?
Comment #5
samuel.mortensonI researched this more as I still don't feel comfortable with the implementation, and filed multiple related issues:
#2856475: Allow Panels Display UUID to change to enable Panelizer use cases
#2856477: When saving as custom and changing the Panels Display UUID, clear existing tempstore for previous UUID
#2856471: IPE on Panelizer nodes should only allow for custom overrides
#2856724: Allow new Panelized Entities to come pre-customized by default
The tl;dr here is that the Panels Display ID should always be unique enough to work as a tempstore ID for IPE. The first two issues linked above illustrate that the way Panelizer currently changes UUIDs when users save as custom is broken, which is part of why this implementation exists. We're putting a bandaid over the issue that customized Panels Displays in Panelizer have the same ID as their original default display, which in my opinion is not acceptable. Panelizer has complete control over what the Panels IPE tempstore ID is, without implementing a new alter hook. Panelizer needs to make sure that Panels Display have unique UUIDs, instead of working around its internal bugs.
Comment #6
samuel.mortensonDoom-and-gloom aside, I'd be fine with this implementation if it took the revision ID and langcode into account, in addition to the source problems being looked at as well. I think there are core problems in Panelizer (like #2856475: Allow Panels Display UUID to change to enable Panelizer use cases) that could be fixed in addition to this issue, but fear that if we just commit this patch as-is there won't be a need to work on related problems.