community-platform icon indicating copy to clipboard operation
community-platform copied to clipboard

feat: lock research to multiple editor access

Open thisislawatts opened this issue 1 year ago • 6 comments

PR Checklist

PR Type

  • [ ] Bug fix (non-breaking change which fixes an issue)
  • [x] New feature (non-breaking change which adds functionality)
  • [ ] Breaking change (fix or feature that would cause existing functionality to change)
  • [ ] Developer experience (improves developer workflows for contributing to the project)

Description

Introduces the concept of locking for Research editors. It is now possible for multiple users to be able to access a Research article or Research Update. This is great but introduces the potential for multiple users to overwrite each others changes.

This PR attempts to minimise this risk by introducing the concept of locking a document. A document is marked as locked when the edit form is locked. The document is then marked as unlocked when the edit form is unloaded.

Git Issues

Closes #2144

Screenshots/Videos

Locked screen when attempting to access Research if another user is working on it.

Screenshot 2023-04-10 at 16 00 56

How to validate

  1. You will need two users (A + B) with edit access to the same Research Article.
  2. User A opens the Research article in edit mode
  3. User B attempts to edit the Research article
  4. Expectation: User B is unable to access the Research article and will be shown a useful error message.

What happens next?

Thanks for the contribution! We try to make sure all PRs are reviewed ahead of a monthly dev call (first Monday of the month, open to all!).

If the PR is working as intended it'll be merged and included in the next platform release, if not changes will be requested and re-reviewed once updated.

If you need more immediate feedback you can try reaching out on Discord in the Community Platform development channel.

thisislawatts avatar Apr 09 '23 21:04 thisislawatts

3 flaky tests on run #4236 ↗︎

0 192 0 0 Flakiness 3

Details:

feat: lock research editor access
Project: onearmy-community-platform Commit: bbfa892375
Status: Passed Duration: 04:51 💡
Started: Sep 23, 2023 5:11 PM Ended: Sep 23, 2023 5:16 PM
Flakiness  howto/read.spec.ts • 1 flaky test • ci-chrome

View Output Video

Test Artifacts
[How To] > [List how-tos] > [By Everyone] Output Screenshots
Flakiness  common.spec.ts • 1 flaky test • ci-firefox

View Output Video

Test Artifacts
[Common] > [User Menu] > [By Authenticated] Output Screenshots
Flakiness  SignUp.spec.ts • 1 flaky test • ci-firefox

View Output Video

Test Artifacts
[Sign-up - new user] > sign in as new user Output Screenshots

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings.

cypress[bot] avatar Apr 09 '23 21:04 cypress[bot]

The next piece of work here is to introduce a mechanism which locks the document. The initial idea would be to add the lock when the edit description/update form loads. Although the problem I see would be around signally that the document can be unlocked.

Events that would trigger the document being unlocked:

  • Navigating away from the edit form
  • Closing the tab/window

Risks:

  • Someone leaves the tab open which blocks the document from being edited.
  • Network error on unloading process leaves document in locked state. There probably needs to be a way to unlock a document.

thisislawatts avatar Apr 10 '23 12:04 thisislawatts

Thanks for adding this.

  • It worked the first time. Feels good! However when I tried it again after that. (So edit the editted step) I can access it with both usernames at the same time.
  • "Back to home" links a bit far away. (Back to the Academy). Could this redirect back to the original research page? Or Research overview?

davehakkens avatar Apr 12 '23 21:04 davehakkens

Thanks for taking it for a test run @davehakkens 🥳

I haven't been able to recreate the behaviour you've described yet but try coming at it a few different ways.

I've updated the BlockedRoute component to take a customisable URL and button text, so whilst it defaults to the home page you can change it to be more helpful. See screengrab below which shows it point back to the Research article.

Screenshot 2023-04-13 at 20 19 30

thisislawatts avatar Apr 13 '23 18:04 thisislawatts

Cant seem to replicate the bug anymore either. So looks good. Nice update on the button.

davehakkens avatar Apr 19 '23 06:04 davehakkens

😮‍💨 Having some issues recreating this test failure locally.

thisislawatts avatar May 13 '23 19:05 thisislawatts

What did we decide with this feat @thisislawatts are we going forwards since the bug can't be replicated or is this still a WIP?

AlfonsoGhislieri avatar Jul 13 '23 13:07 AlfonsoGhislieri

Just coming back to check on this one, it’s become pretty stale. What are we thinking @davehakkens @thisislawatts

AlfonsoGhislieri avatar Sep 10 '23 10:09 AlfonsoGhislieri

Not an urgent issue. But happy to merge in if @thisislawatts thinks its good

davehakkens avatar Sep 12 '23 22:09 davehakkens

:tada: This PR is included in version 1.99.0 :tada:

The release is available on GitHub release

Your semantic-release bot :package::rocket:

onearmy-bot avatar Sep 23 '23 20:09 onearmy-bot