Panels IPE should be locked if another user is making TempStore changes. Using Views UI as an example, we should store a User ID in our SharedTempStore and validate that User ID when making TempStore changes or initially loading Panels IPE.

CommentFileSizeAuthor
#8 2699085-tempstore-8.patch2.59 KBtim.plunkett
#8 2699085-tempstore-8-interdiff.txt1.26 KBtim.plunkett
#5 2699085-tempstore-5-PASS.patch2.36 KBtim.plunkett
#5 2699085-tempstore-5-FAIL.patch1.19 KBtim.plunkett
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

samuel.mortenson created an issue. See original summary.

samuel.mortenson’s picture

samuel.mortenson
he/him
Primary language English
CreditAttribution: samuel.mortenson at Acquia commented

We should consider moving to PrivateTempStore in this issue as well, with a SharedTempStore lock.

samuel.mortenson’s picture

samuel.mortenson
he/him
Primary language English
CreditAttribution: samuel.mortenson at Acquia commented

So I started work on this and realized that unlike \Drupal\views_ui\ViewsUI, we do not have an abstracted object that represents temporary state in Panels. Before I write more code - where do other maintainers think that this should live? I was planning on giving PanelsStorageInterface/PanelsStorageBase common methods for dealing with TempStore and locks, so that all UIs can use the same methods to load/clear/lock a PanelsDisplay (using what Panels IPE is already doing as a base).

Thoughts?

dsnopek’s picture

dsnopek
he/him
Primary language English
Location USA
CreditAttribution: dsnopek commented

I'd need to see the code in question to really say if it'd be appropriate to add to PanelsStorageInterface. If it's not something that we'll customize for individual Panels storage plugins, then I think it probably isn't appropriate there.

If it's something that will work the same always, maybe a better place would be on PanelsStorageManager/PanelsStorageManagerInterface?

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett at Acquia commented
Status: Active » Needs review
FileSize
2699085-tempstore-5-FAIL.patch1.19 KB
2699085-tempstore-5-PASS.patch2.36 KB

This prevents the tempstore from bleeding over to other users when they *view* the panelized entity.
This does not yet address two users concurrently editing.

The last submitted patch, 5: 2699085-tempstore-5-FAIL.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 5: 2699085-tempstore-5-PASS.patch, failed testing.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett at Acquia commented
Status: Needs work » Needs review
FileSize
2699085-tempstore-8-interdiff.txt1.26 KB
2699085-tempstore-8.patch2.59 KB

Uh, works on my machine...

Status: Needs review » Needs work

The last submitted patch, 8: 2699085-tempstore-8.patch, failed testing.

  • tim.plunkett committed ea2f6a4 on 8.x-4.x 
    Issue #2699085 by tim.plunkett: Lock Panels IPE when a user has made...
japerry’s picture

japerry
Location KVUO
CreditAttribution: japerry at Acquia commented
Version: 8.x-3.x-dev » 8.x-4.x-dev
tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett at Acquia commented
Version: 8.x-4.x-dev » 8.x-3.x-dev
Status: Needs work » Fixed

Made commits against CTools, Panels, and Panelizer to add a locking mechanism similar to Views UI.
If more work is needed, please open new issues and link them from here!

Status: Fixed » Closed (fixed)

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