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

CARDS-2514: New "locking" module, with basic locking feature #1756

Draft
wants to merge 13 commits into
base: dev
Choose a base branch
from

Conversation

acrowthe
Copy link
Contributor

@acrowthe acrowthe commented Apr 18, 2024

Initial implementation including:

  • Locking restriction
  • Lock node on subjects
    • Includes author who locked the node and time
    • Only locked subjects with a child lock node can be unlocked and only if their parent is unlocked
      to prevent partially unlocked child trees
  • Basic UI for locking and unlocking
    • Single lock/unlock button based on current lock state

Testing:

  • Verify that locking is opt-in:
    • Launch using ./start_cards.sh --test --dev to make sure nothing has changed and that locking is disabled by default
    • Launch using ./start_cards.sh --test --dev --locking to test that locking is can be enabled and is working as expected
    • Prepare and run docker with locking:
      • python3 generate_compose_yaml.py --cards_project cards4prems --oak_filesystem --dev_docker_image --locking
      • docker-compose build && docker-compose up
      • Test that locking is working as expected
      • docker-compose down && docker-compose rm && docker volume prune -f && ./cleanup.sh
    • Prepare and run docker without locking: python3 generate_compose_yaml.py --cards_project cards4prems --oak_filesystem --dev_docker_image

Make sure that:

  • Existing action buttons on the subject page work as before (delete, print preview)
  • When a subject is locked, non-admin users cannot edit the subject or any of it's child forms
    • Currently any user can add or remove locks, this will be restricted by a future task
  • Subjects cannot be locked or unlocked if they have a locked parent
  • If you lock a child subject then lock a parent subject, unlocking the parent will not unlock the child
  • If you lock a parent subject that has unlocked children (forms and/or subject), the child nodes will be locked too
Locking2.mp4

@acrowthe acrowthe changed the base branch from dev to CARDS-2513 April 18, 2024 12:49
@acrowthe
Copy link
Contributor Author

Improved UI so it updates subject data whenever the locking dialog is completed

Locking3.mp4

@acrowthe
Copy link
Contributor Author

Updated lock and unlock dialogs:
image
image

Changed HTTP method from POST to LOCK and UNLOCK

final VersionManager versionManager = session.getWorkspace().getVersionManager();

final Node requestNode = request.getResource().adaptTo(Node.class);
// TODO: check if user has permission to lock and unlock the request node
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This should be implemented in a separate issue that adds rights to lock and unlock resources

@acrowthe acrowthe marked this pull request as ready for review April 24, 2024 20:58
@sashaandjic
Copy link
Contributor

sashaandjic commented Apr 25, 2024

Suggestion: Add LOCKED BY ...
image

// Hardcode the resource type
- sling:resourceType (STRING) = "cards/Lock" mandatory autocreated protected
// Hardcode the resource supertype: the login page is a resource
- sling:resourceSuperType (STRING) = "cards/Resource" mandatory autocreated protected
Copy link
Member

Choose a reason for hiding this comment

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

This isn't a resource. It should be a cards:Item, but that PR is still a draft.

@sdumitriu
Copy link
Member

I think this will need adjusting, but not right now: creating a new form from the dashboard should not allow selecting a locked subject. I think this would be better handled by checking rights themselves, instead of customizing even more the subject selector component.

Initial implementation including:
- Locking restriction
  - TODO: Once lock and unlock rights are implemented, apply locked restriction to everyone by default
- Lock node on subjects
  - Includes author who locked the node
  - Only locked subjects with a child lock node can be unlocked and only if their parent is unlocked
    to prevent partially unlocked child trees
  - TODO: add time to lock node
- Basic UI for locking and unlocking
  - Single lock/unlock button based on current lock state
  - TODO: list any incomplete forms
  - TODO: update UI when locking or unlocking successfull to reflect new lock state
  - TODO: list current user and timestamp

Other tasks TODO:
- Make locking opt-in
- Make subject actions extensible to move the locking button into the locking modules
Lock button will now update the subject view so changes to locked status will
be displayed to the user without needing to refresh the page
Switch to LOCK and UNLOCK HTTP methods rather than Post
Create new Subject Action extension
- Move lock button into locking module and access it through the extension
- Restrict form and child subject creation for locked subjects
- Improve locking UI
  - Simplify subject actions extension
  - Add list of incomplete forms to lock UI
  - Update child form display when a subject is locked or unlocked
Improve locking UI
- Split locking dialog into up to 2 dialogs:
  - List of incomplete forms + continue
  - Sign off
- Convert username and date into disabled inputs
- Convert list of incomplete forms into Material UI list
Refactor lock handling into LockManager rather than built into LockServlet
Remove extraneous lock action extension definition
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.

4 participants