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

useField can incorrectly update document lock #8781

Closed
stofolus opened this issue Oct 18, 2024 · 5 comments · Fixed by #9026
Closed

useField can incorrectly update document lock #8781

stofolus opened this issue Oct 18, 2024 · 5 comments · Fixed by #9026
Assignees

Comments

@stofolus
Copy link
Contributor

Describe the Bug

When using a useForm inside a custom field component you can have a the form-state request update the "updatedAt" field on a document locked by another user.

Link to the code that reproduces this issue

https://github.com/stofolus/payload-lock-bug

Reproduction Steps

Clone the repo or use the latest website template.

The changes to the template are in the Posts collection.

  • The documentLoaded field
  • Disable autosave
  • Add document locking
versions: {
    drafts: true,
    maxPerDoc: 50,
  },
lockDocuments: {
    duration: 60,
  },

Create a couple of users.

Open and make a change to a page without saving. Then close the window. Now open the same page with another user and if you check the database you can se that with every page load the updatedAt field is updated. Even though it's tied to user1 and you are viewing the page as another user.

Now I know that you could fix this by checking if the field is readOnly before updating it. And it's absolutely resonable to do. But I still think that the form-state api should not update the locked-documents document if it's tied to another user. It's a good way to make it harder for people to accidentally create bugs

Which area(s) are affected? (Select all that apply)

Not sure

Environment Info

Binaries:
Node: 20.17.0
npm: 10.8.2
Yarn: N/A
pnpm: 9.0.2
Relevant Packages:
payload: 3.0.0-beta.116
next: 15.0.0-canary.173
@payloadcms/db-mongodb: 3.0.0-beta.116
@payloadcms/email-nodemailer: 3.0.0-beta.116
@payloadcms/graphql: 3.0.0-beta.116
@payloadcms/live-preview: 3.0.0-beta.116
@payloadcms/live-preview-react: 3.0.0-beta.116
@payloadcms/next/utilities: 3.0.0-beta.116
@payloadcms/plugin-cloud: 3.0.0-beta.116
@payloadcms/plugin-form-builder: 3.0.0-beta.116
@payloadcms/plugin-nested-docs: 3.0.0-beta.116
@payloadcms/plugin-redirects: 3.0.0-beta.116
@payloadcms/plugin-search: 3.0.0-beta.116
@payloadcms/plugin-seo: 3.0.0-beta.116
@payloadcms/richtext-lexical: 3.0.0-beta.116
@payloadcms/translations: 3.0.0-beta.116
@payloadcms/ui/shared: 3.0.0-beta.116
react: 19.0.0-rc-3edc000d-20240926
react-dom: 19.0.0-rc-3edc000d-20240926
Operating System:
Platform: linux
Arch: x64
Version: #1 SMP Fri Mar 29 23:14:13 UTC 2024
Available memory (MB): 15970
Available CPU cores: 12

@stofolus stofolus added status: needs-triage Possible bug which hasn't been reproduced yet v3 labels Oct 18, 2024
@github-actions github-actions bot removed the status: needs-triage Possible bug which hasn't been reproduced yet label Oct 28, 2024
@PatrikKozak
Copy link
Contributor

PatrikKozak commented Nov 1, 2024

Hey @stofolus - I added this to a test config and observed a few things, plus I have a couple of questions:

When you navigate away from a document as the initial user editing it, are you still seeing a lock on the document (i.e., does api/payload-locked-documents still contain a doc with your user data in it)?

Normally, the lock should be removed when navigating away, allowing other users full access to update the document. However, if you close your tab instead of returning to the collection list view, for instance, the lock may remain because a proper unmount didn’t occur.

This is part of why we included the duration prop on lockDocuments—to ensure that after a set interval without edits, the lock expires, letting other users access the document without a lock notification.

I bring this up because if a document is locked and not expired, a separate user would not be able to enter the document and make changes without having to manually take over. However, I do see the DocumentLoaded field update for example if a separate user enters the document in read-only mode.

This might visually seem that the field is updated but the field wouldn't actually update in the DB since this separate user doesn't have editing / saving rights if the document is locked by another user at this time. They would re-gain saving rights if the lock has expired though.

Even though you might see a request be made to form-state in the network tab - this is simply happening due to the onChange function in the edit view component firing due to the nature of your custom field and the way you are setting the value of the field.

I see you have set the duration to 60 seconds - so if no edits were performed within 60 seconds after the initial edit, then separate users would be able to update the document freely, since the document lock would be considered expired at that point.

Are you experiencing an instance where a document is locked and NOT expired and a separate user is able to update that locked document?

I see you're on version 3.0.0-beta.116 and we've since then added a few key fixes to Document Locking in general since then so I was testing on v3.0.0-beta.123—I mention this in case you are experiencing different behavior than what I described above.

@stofolus
Copy link
Contributor Author

stofolus commented Nov 4, 2024

Hi Patrik

I think i might have been a bit vague and that the naming of things are causing some confusion in what's actually updated. I'm actually not experience my document being updates.

The thing that is being updated is the locked-documents document. For example if user 1 creates a lock on a page and then user 2 visits that page and the onChange function is run which causes a form-state to be send. The locked-documents document will get a new updatedAt even though it's user 2 that is viewing the document.

@PatrikKozak
Copy link
Contributor

@stofolus I see what you're saying now. I'll take a look into this. Thanks for bringing this to our attention.

Copy link
Contributor

github-actions bot commented Nov 6, 2024

🚀 This is included in version v3.0.0-beta.125

Copy link
Contributor

github-actions bot commented Nov 8, 2024

This issue has been automatically locked.
Please open a new issue if this issue persists with any additional detail.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 8, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
2 participants