Skip to content
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

New LockException and Lock Dialog #6835

Merged
merged 9 commits into from
Dec 7, 2024
Merged

Conversation

bastianallgeier
Copy link
Member

Description

Merge first

Summary of changes

  • New Kirby\Content\LockException
  • New k-lock-dialog
  • New panel.content.lockDialog method

Reasoning

When an editor starts editing, but someone else made changes at the same time, there can be a race condition where the editor can no longer save or publish the changes because the other one was faster. This is now handled with a new dialog to inform the editor in a good way and reload the view afterwards.

The new LockException is used to throw a custom error with all the lock details. This is used in the Panel to fill the dialog with the necessary information.

Screenshot 2024-12-05 at 16 10 36

Testing

The easiest way to test this is to make sure that you have two users. Let's assume, the second one has the id "editor"

  1. Go to any page and start editing
  2. Navigate to the changes text file in the filesystem and change the Lock: field from your user id to the "editor" user id.
  3. Reload the page in the panel. The page should now be locked
  4. Open the browser console and try to still save or publish changes with the following commands:
panel.content.save({subheading: 'Test'});
panel.content.publish({subheading: 'Test'});

The new dialog should appear and give you the right information about the lock. Closing the dialog should reload the view.

@bastianallgeier bastianallgeier added this to the 5.0.0-beta.1 milestone Dec 5, 2024
@bastianallgeier bastianallgeier requested a review from a team December 5, 2024 15:39
panel/src/panel/content.js Outdated Show resolved Hide resolved
src/Content/LockException.php Outdated Show resolved Hide resolved
src/Content/LockException.php Outdated Show resolved Hide resolved
src/Content/VersionRules.php Outdated Show resolved Hide resolved
panel/src/components/Dialogs/index.js Outdated Show resolved Hide resolved
@bastianallgeier
Copy link
Member Author

I guess it makes sense to merge this first: #6839 and then continue with this

@distantnative
Copy link
Member

@bastianallgeier I added the suggested changes, please have a look if it's fine for you.

@bastianallgeier
Copy link
Member Author

@distantnative looks great!

@bastianallgeier bastianallgeier merged commit 33d345f into v5/develop Dec 7, 2024
7 checks passed
@bastianallgeier bastianallgeier deleted the v5/lock-response branch December 7, 2024 11:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants