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

Add Access Levels to Sharing Permissions #1997

Conversation

deundrewilliams
Copy link
Member

This PR has a migration

(fixes #1983)

This adds different access levels (Full, Partial, and Minimal) to modules. When sharing, the user will have the option of giving them Full, Partial, or Minimal access.

Full: All options
Partial: View stats, copy, edit, preview
Minimal: View stats, copy, preview

Changes:
All existing modules will give every user that already has access Full level permissions.

Previously, anyone with access had the option to delete a module, but this would only work for the module creator, and in every other instance, this would fail silently. Now, anyone with Full level access can delete and restore the module. If a module was deleted but you didn't have Full level access to it, it won't appear in your Deleted Modules, but it will re-appear in your modules if it's restored by someone with Full access.

When sharing a module, the default access level will be Full, but this can be changed afterwards. Also, a user cannot change their own access level, this is to prevent the situation of a module not having anyone with Full access.
Screen Shot 2022-04-19 at 12 23 01 PM

When showing the module menus, only the actions that a user has access to for each module will appear.

  • Minimal:

Minimal module menu

Minimal module options

  • Partial:

Partial module menu

Partial module options

  • Full:

Full module menu

Screen Shot 2022-04-19 at 12 17 36 PM

In the editor, the 'Delete' button is disabled for users with a Partial access level.
Screen Shot 2022-04-19 at 12 26 32 PM

When selecting modules for bulk operations, if a certain operation isn't available for all selected items (because of the user's access level), that button will be disabled.

Screen Shot 2022-04-19 at 12 30 31 PM

Screen Shot 2022-04-19 at 12 30 38 PM

@FrenjaminBanklin FrenjaminBanklin changed the base branch from dev/28-jadeite to dev/29-sodalite August 26, 2022 20:13
@deundrewilliams deundrewilliams marked this pull request as ready for review September 1, 2022 12:50
@deundrewilliams deundrewilliams changed the title Draft: Add Access Levels to Sharing Permissions Add Access Levels to Sharing Permissions Sep 1, 2022
Copy link
Contributor

@FrenjaminBanklin FrenjaminBanklin left a comment

Choose a reason for hiding this comment

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

This is a great addition and everything seems to be functioning well. I can only see a few things that I might want to be changed.

  1. The spacing for individual user items in the Module Access dialog looks strange, but I'm not sure how best to address it. My first thought is to left-align everything and line up the avatar with the name:
    image
    I'm open to alternatives. It might be nice to tighten up the vertical padding in the access level drop-down so each item takes up less vertical space, but I'm not sure how well that'll work or if the few pixels it would save are worth it.

  2. Instead of storing the full text description of access levels in the database (i.e. 'Full', 'Partial', 'Minimal') it would be preferable to map those permission levels to numeric constants for storage purposes (i.e. 'Full' => 1, 'Partial' => 2', 'Minimal' => 3, etc.). Since the frontend and backend use the same language, those constants could be defined in a single place and shared as necessary.

@deundrewilliams
Copy link
Member Author

I updated the code to use constants stored in obojobo-express/server/constants.js, now everywhere uses 1, 2, and 3 for the Full, Partial, and Minimal access levels.

I also updated the styling and text to reduce the overall height of each item in the list (below)

Screen Shot 2022-09-19 at 11 12 53 AM

@deundrewilliams deundrewilliams linked an issue Oct 19, 2022 that may be closed by this pull request
Copy link
Contributor

@FrenjaminBanklin FrenjaminBanklin left a comment

Choose a reason for hiding this comment

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

Everything seems to be working well and looks good. I'll call this one ready to go.

Two things I've noticed that can be spun off into their own issue:

  • What changes based on the permission level is not immediately evident. We could maybe put a tooltip somewhere in the upper half of the interface explaining what each level allows.
  • If a user has full access to a module when they visit their dashboard and do not refresh their dashboard after their access to that module is changed, they will still be able to interact with that module as if they have full access. I don't see this as too big a deal, but it would be nice to have some additional checks in the backend to invalidate any actions outside of a user's current access level.

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.

Fix Share and Delete Permissions for Modules
2 participants