Closed (fixed)
Project:
Content locking (anti-concurrent editing)
Version:
8.x-2.x-dev
Component:
Code
Priority:
Normal
Category:
Bug report
Assigned:
Unassigned
Reporter:
Created:
18 Aug 2023 at 08:30 UTC
Updated:
29 May 2024 at 14:19 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #2
no sssweat commentedComment #3
no sssweat commentedComment #4
no sssweat commentedComment #5
no sssweat commentedComment #7
no sssweat commentedComment #8
nikhil_110 commentedI reproduced this issue on my local and added MR or checked it, but when I add a node, only when I edit the node from admin, the lock status changes, but when I edit the node from another user. the locked status of the node does not change and is also not shown to other users.
Comment #9
smustgrave commentedTried replicating following the steps in the issue summary and the view is updating fine for me with the latest 8.x-2.x changes.
Comment #11
smustgrave commentedStill not able to replicate but now that we have a views test file I expanded on that to verify this.
Comment #12
smustgrave commentedHiding patches.
Comment #14
alexpottFixing issue title.
Comment #15
alexpottI followed the steps as defined in the issue summary and this is definitely still an issue. Which is good because nothing has been committed recently that would fix this.
The reason the tests pass is that view provided by the module has cache set to none - while the views.view.content has the cache type of tag.
Comment #18
alexpottI've fixed the view so that it works - but I'm scared... this means views an edit screen and locking a node will impact the cache. This does not feel like the correct solution.
I think what we should do is introduce a content_lock cache tag and cause views which use it's fields to add that.
Comment #20
alexpottThe changes to config/optional/views.view.locked_content.yml and src/ContentLock/ContentLock.php can be removed. They’re not necessary.
We also should add an upgrade path that finds views and resaves them if they use the content_lock table.
Comment #21
penyaskitoCompleted the upgrade and tested it manually.
Do we expect an upgrade test too? If not this is ready for reviews.
Comment #22
smustgrave commentedApplied MR locally
Hook ran fine without issue
Tests appear all green
Believe this is good, based also on the slack thread I was on.
Comment #23
alexpottAdded some comments to the MR - I think we need to add test coverage of the new code.
Comment #24
alexpottAdding a test and changed a few things.
Comment #25
alexpottComment #26
alexpottOkay - we've got test coverage of the view and saving the view while trying to change the cache plugin. This is looking good. I manually tested the post update and it is was working.
Comment #28
alexpott