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

Obtain editing lock on submissions #137

Merged
merged 14 commits into from
Jul 26, 2024
Merged

Conversation

pkalita-lbl
Copy link
Collaborator

Fixes #90

Summary

SubmissionMetadata records have two fields (locked_by and lock_updated) that indicate who is currently editing a submission. Previously we were not consulting this field at all which could lead to edit conflicts if more than one person were to edit a submission at the same time. So the changes here introduce three things:

  1. Obtaining a lock prior to making submission modifications (this includes creating a new sample, editing an existing study or sample, or deleting an existing study or sample)
  2. If obtaining the lock fails (typically because someone else has it), display an informative message to the user
  3. If obtaining the lock succeeds, release the lock once the editing is complete. This is generally done when the user navigates away from an editing interface. There's no guarantee this navigation will actually happen (i.e. a user starts editing a sample and then drops their phone in a river). The desktop submission portal faces a similar issue which is why the backend already automatically expires these locks after 30 minutes.

Details

  • The nmdc-server backend provides endpoints to request and release a submission lock. These endpoints are called by new methods in src/api.ts along with some refinement to the type definitions.
  • In src/queries.ts two new mutations have been added to the call the API methods and to keep the server data state updated when the requests settle.
  • In the study edit page (src/pages/StudyEditPage/StudyEditPage.tsx), we attempt to obtain the submission lock when the page loads. If the lock cannot be obtained, the entire study form and the delete study button become disabled, and we present a message to the user indicating who holds the lock. If obtaining the lock is successful it is released when leaving the page.
  • In the sample page (src/components/SampleView/SampleView.tsx), we similarly attempt to obtain the submission lock when the page loads. If the lock cannot be obtained, the slot editing modal and the delete sample button become disabled, and we present a message to the user indicating who holds the lock. If obtaining the lock is successful it is released when leaving the page.
  • In the study view page (src/components/StudyView/StudyView.tsx), we do not obtain a lock when simply viewing the page. We do attempt to obtain the lock immediately before attempting to create a new sample in the study. If obtaining the lock fails, a new sample is not created and a message is displayed to the user.
  • One slightly tangential change here is in src/queries.ts. One feature of the submission portal is that when you create or update a submission and you provide an ORCID iD for the study's PI, that user should also have permission to view/edit the submission. For whatever reason that isn't handled automatically by the backend. Instead it's up to the client to use the permissions field in the request body to set that.

Testing

If you want to test out these changes you need to have a submission that is locked by someone other than yourself. And it also helps if you are an admin so that you can access everyone's submissions. I find it easiest to do that by direct SQL commands to my local nmdc-server Postgres database. For example, check to see if your are an admin:

select * from user_logins where name = '<your name>';

If is_admin is false:

update user_logins set is_admin = true where name = '<your name>';

Get the 10 most recently created submissions:

select * from submission_metadata order by created desc limit 10;

Pick one of those submissions and a random person's id from the user_logins table and set the lock:

update submission_metadata
set locked_by_id = '<other user id>', lock_updated = now()
where id = '<recent submission id>';

As mentioned previously a lock that is older than 30 minutes is ignored by the backend, so if you want to check which submissions still have an active lock:

select * from submission_metadata
inner join user_logins on submission_metadata.locked_by_id = user_logins.id
where lock_updated + interval '30 minutes' >= now()
order by lock_updated desc;

Once you have a submission with a fresh lock from someone else you can try various operations in the interface to make sure you're not allowed to modify the submission and that you're notified that someone else holds the lock.

Copy link
Collaborator

@eecavanna eecavanna left a comment

Choose a reason for hiding this comment

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

Thanks for including that PR description! 👏

I left some comments/suggestions on the diff. I didn't encounter anything in the diff that I considered to be a deal-breaker, so I am comfortable with this branch being merged in as is (I'll be OOO from Fri-Tue).

I have a question regarding the 30-minute lock timeout. Suppose Alice obtains the lock on a Study at 1 PM and she is still actively editing the Study at 1:25 PM. Does her lock still expire at 1:30 PM, or does that 30-minute expiration keep getting pushed back each time she makes a change to the Study?

pkalita-lbl and others added 3 commits July 26, 2024 09:32
@pkalita-lbl
Copy link
Collaborator Author

Suppose Alice obtains the lock on a Study at 1 PM and she is still actively editing the Study at 1:25 PM. Does her lock still expire at 1:30 PM, or does that 30-minute expiration keep getting pushed back each time she makes a change to the Study?

The lock gets extended with each saved update. The relevant part of the backend is here where while handling an update request the call to try_get_submission_lock simultaneously checks that the user has the lock and if they do it updates the lock_updated time on it.

@pkalita-lbl pkalita-lbl merged commit e5a8828 into main Jul 26, 2024
1 check passed
@pkalita-lbl pkalita-lbl deleted the issue-90-submission-locking branch July 26, 2024 16:49
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.

App should respect the edit lock set by the backend
2 participants