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

fix(next, ui): only show locked docs that are not expired #8899

Merged
merged 6 commits into from
Oct 29, 2024

Conversation

PatrikKozak
Copy link
Contributor

@PatrikKozak PatrikKozak commented Oct 28, 2024

Issue:

Previously, documents that were locked but expired would still show in the list view / render the DocumentLocked modal upon other users entering the document.

The expected outcome should be having expired locked documents seen as unlocked to other users.

I.e:

  • Removing the lock icon from expired locks in the list view.
  • Prevent the DocumentLocked modal from appearing for other users - requiring a take over.

Fix:

  • Only query for locked documents that are not expired, aka their updatedAt dates are greater than the the current time minus the lock duration.
  • Performs a deleteMany on expired documents when any user edits any other document in the same collection.

Fixes #8778

TODO: Add tests

@PatrikKozak PatrikKozak linked an issue Oct 28, 2024 that may be closed by this pull request
Comment on lines +200 to +203
const validLockedDocs = lockedDocs.filter((lock) => {
const lastEditedAt = new Date(lock?.updatedAt).getTime()
return lastEditedAt + lockDurationInMilliseconds > now
})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this needed? It seems that we already queried for lockedDocuments with the where filter updatedAt.

@DanRibbens
Copy link
Contributor

There might be some more code reuse we can add when you're doing tests. Good enough for now!

@DanRibbens DanRibbens merged commit 1e002ac into beta Oct 29, 2024
50 checks passed
@DanRibbens DanRibbens deleted the fix/beta/expired-document-locks branch October 29, 2024 00:05
const isLockingEnabled = lockDocumentsProp !== false

const lockDurationDefault = 300 // Default 5 minutes in seconds
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like it should be somewhere else. Maybe set in sanitization?

Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Document lock duration doesn't work
3 participants