UI to manage users/permissions for the content libraries [FC-0062] #1362

pomegranited · 2024-10-04T09:21:39Z

Description

Adds a modal UI to add/remove users from the Library Team, with different roles: Admin, Author, or Reader

Also updates the Libraries UI to ensure read-only users can't see the Publish, Revert, or Add Tags buttons -- these would only throw errors as the user doesn't have permission on the backend.

This change impacts Content Authors using Libraries V2.

Screenshots

Admin view

Author view

Reader view

Supporting information

Part of: #1342
Resolves: #1376
Private-ref: FAL-3869

Testing instructions

You will need 1 or 2 extra users for these tests.

Create a library, and add one or more components.
From the library info sidebar, click "Manage Access"
You should see yourself as an Admin.
Click "Add team member", and add another user.
That user should be added to the Team as a Reader.
In a private window, login as the Reader user and visit the library page in the MFE.
Confirm that you can view the library and its components, but cannot edit anything.
Confirm that you cannot see any "Publish" , "Revert changes", or "Manage Access" buttons in the library sidebar.
As the original Admin user, click "Make Author" to promote the user to Author.
In the private window, close and re-open the library info tab.
Confirm that you can now edit/publish/revert the library and its components.
Click "Manage Access" in the library sidebar, and confirm that you can see the Library Team, but cannot make any changes.
As the original Admin user, click "Make Admin" to promote the user to Admin.
In the private window, close and re-open the library info tab.
Confirm that you can now edit/publish/revert the library and its components.
Click "Manage Access" in the library sidebar, and confirm that you can edit the Library Team.
As the original Admin user, delete the user you added.
This should remove access to the library for that user.
In the private window, refresh the library page.
You should see a spinner and 403 errors in the Network tab.
The "Libraries" list will not contain this library.
As the original Admin user, toggle the "Allow Public Read" switch ON.
Confirm that this setting saves as expected.
In the private window, refresh the Libraries list.
Confirm that you can view the library and its components, but cannot edit anything.
Confirm that you cannot see any "Publish" , "Revert changes", or "Manage Access" buttons in the library sidebar.
As the original Admin user, test the responsiveness of the Library Team modal in various browser sizes.

Author Concerns

Do we need help text shown in the Library Team modal to explain the different roles?
Once there's more than one Admin user in a Library Team, it's possible for the current user to demote or remove their own Admin member record. Unless they're also a Global Staff, they will lose access to the Library and/or the Library Team. Do we need to guard/warn against this?

openedx-webhooks · 2024-10-04T09:21:44Z

Thanks for the pull request, @pomegranited!

What's next?

Please work through the following steps to get your changes ready for engineering review:

🔘 Get product approval

If you haven't already, check this list to see if your contribution needs to go through the product review process.

If it does, you'll need to submit a product proposal for your contribution, and have it reviewed by the Product Working Group.
- This process (including the steps you'll need to take) is documented here.
If it doesn't, simply proceed with the next step.

🔘 Provide context

To help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:

Dependencies

This PR must be merged before / after / at the same time as ...
Blockers

This PR is waiting for OEP-1234 to be accepted.
Timeline information

This PR must be merged by XX date because ...
Partner information

This is for a course on edx.org.
Supporting documentation
Relevant Open edX discussion forum threads

🔘 Get a green build

If one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green.

🔘 Let us know that your PR is ready for review:

Who will review my changes?

This repository is currently maintained by @openedx/2u-tnl. Tag them in a comment and let them know that your changes are ready for review.

Where can I find more information?

If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources:

When can I expect my changes to be merged?

Our goal is to get community contributions seen and reviewed as efficiently as possible.

However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:

The size and impact of the changes that it introduces
The need for product review
Maintenance status of the parent repository

💡 As a result it may take up to several weeks or months to complete a review and merge your PR.

which allows Library Admins to add Readers, Authors, and other Admin users to the list of people who can access a Library. * Readers can only see the library, but not its Team. * Authors can see the Library Team, but cannot alter it. * Admins can update the Library Team. Modal is triggered from a button on the LibraryInfo sidebar which is only accessible to users who can edit the library.

src/library-authoring/data/apiHooks.ts

codecov · 2024-10-09T11:31:21Z

Codecov Report

Attention: Patch coverage is 93.69748% with 15 lines in your changes missing coverage. Please review.

Project coverage is 93.04%. Comparing base (8448760) to head (e90be35).
Report is 1 commits behind head on master.

Files with missing lines	Patch %	Lines
src/library-authoring/library-team/LibraryTeam.tsx	86.90%	11 Missing ⚠️
src/library-authoring/data/api.ts	85.71%	3 Missing ⚠️
src/content-tags-drawer/ContentTagsDrawer.tsx	94.44%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1362      +/-   ##
==========================================
- Coverage   93.04%   93.04%   -0.01%     
==========================================
  Files        1036     1043       +7     
  Lines       19743    19954     +211     
  Branches     4174     4233      +59     
==========================================
+ Hits        18370    18566     +196     
- Misses       1311     1323      +12     
- Partials       62       65       +3

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

pomegranited · 2024-10-09T12:43:02Z

src/library-authoring/library-team/AddLibraryTeamMember.tsx

@@ -0,0 +1,61 @@
+import { useIntl } from '@edx/frontend-platform/i18n';


The library-team UI borrows heavily from the course-team UI. I initially tried to factor out the common parts so we could re-use components. But there were so many subtle differences between them -- 3 roles instead of 2, different text, the "allow public" toggle switch -- that it made the code really hard to read and use. So I ended up just copying over what I needed, though I did decide to use some css classes to keep a consistent appearance.

rpenido

LGTM 👍
Thank you for your work, @pomegranited!

I tested this using the instructions from the PR
I read through the code
I checked for accessibility issues
Includes documentation

Some comments:

The user can "Manage Tags" (at least try to) in the readonly mode. We are addressing it in at least 2 other PRs, but I think we should also need it here.

In the private window, refresh the library page.
You should see a spinner and 403 errors in the Network tab.

I don't think this is a BIG issue, but because we use a token on the meilisearch queries it will still work. We can reduce the token expiry, as I think it is currently set to one week.

src/library-authoring/data/apiHooks.ts

src/library-authoring/library-team/AddLibraryTeamMember.tsx

src/library-authoring/library-team/LibraryTeamMember.tsx

* camel-case the updated library data before storing it in the cache * add validation for the email on the Add Team Member form * remove showToast from the useCallback context (console errors) * key field only needs to be locally unique * updates comments on "don't demote/delete the single Admin member"

pomegranited · 2024-10-10T05:03:25Z

@rpenido

The user can "Manage Tags" (at least try to) in the readonly mode. We are addressing it in at least 2 other PRs, but I think we should also need it here.

I looked into this and it's more complicated than it seems on the surface because of how complicated our tagging permissions are :/ Have raised #1376 to address this, because it's going to require both backend + frontend changes to fix.

and makes minor mods to the UI/data to fix a label and a data type.

bradenmacdonald

Looks great. Nice work!

bradenmacdonald · 2024-10-12T00:28:42Z

src/library-authoring/data/api.ts

+export interface LibraryTeamMember {
+  username: string;
+  email: string;
+  accessLevel: string;


Could we define this as a LibraryRole type instead? Or at least 'admin' | 'author' | 'read' instead of string.

src/library-authoring/library-team/messages.ts

pomegranited · 2024-10-14T22:48:32Z

Thanks for catching those type issues @bradenmacdonald ! Addressed and ready for your review/merge.

openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Oct 4, 2024

pomegranited force-pushed the jill/library-team branch from c5ab164 to bd21593 Compare October 8, 2024 07:11

fix: don't show Publish/Revert button unless user can edit the library

5ec863e

pomegranited force-pushed the jill/library-team branch 2 times, most recently from 56078bd to c9b5d2f Compare October 9, 2024 11:19

pomegranited force-pushed the jill/library-team branch from c9b5d2f to 06fff10 Compare October 9, 2024 11:20

pomegranited commented Oct 9, 2024

View reviewed changes

src/library-authoring/data/apiHooks.ts Outdated Show resolved Hide resolved

pomegranited commented Oct 9, 2024

View reviewed changes

rpenido approved these changes Oct 9, 2024

View reviewed changes

pomegranited mentioned this pull request Oct 10, 2024

"Manage Users/Permissions" for v2 Libraries #1342

Closed

2 tasks

mphilbrick211 added the FC Relates to an Axim Funded Contribution project label Oct 10, 2024

pomegranited added 4 commits October 11, 2024 09:59

Merge remote-tracking branch 'origin/master' into jill/library-team

c2660b1

fix: hide manage/add/edit tags buttons when not allowed to tag object

43783a0

fix: various misspellings in code and comments

8554cd5

test: adds Library Team tests

9041907

and makes minor mods to the UI/data to fix a label and a data type.

pomegranited marked this pull request as ready for review October 11, 2024 11:15

pomegranited requested a review from a team as a code owner October 11, 2024 11:15

bradenmacdonald approved these changes Oct 12, 2024

View reviewed changes

fix: use string literals for library accessLevel

a0e0d84

fix: defineMessage has types now

17ec568

pomegranited force-pushed the jill/library-team branch from 92aaa33 to 17ec568 Compare October 14, 2024 22:57

chore: update with latest master

e90be35

bradenmacdonald enabled auto-merge (squash) October 15, 2024 17:14

bradenmacdonald merged commit 1ee3229 into openedx:master Oct 15, 2024
6 checks passed

bradenmacdonald deleted the jill/library-team branch October 18, 2024 23:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

UI to manage users/permissions for the content libraries [FC-0062] #1362

UI to manage users/permissions for the content libraries [FC-0062] #1362

pomegranited commented Oct 4, 2024 •

edited

Loading

openedx-webhooks commented Oct 4, 2024 •

edited

Loading

codecov bot commented Oct 9, 2024 •

edited

Loading

pomegranited Oct 9, 2024

rpenido left a comment

pomegranited commented Oct 10, 2024

bradenmacdonald left a comment

bradenmacdonald Oct 12, 2024

pomegranited commented Oct 14, 2024

		@@ -0,0 +1,61 @@
		import { useIntl } from '@edx/frontend-platform/i18n';

UI to manage users/permissions for the content libraries [FC-0062] #1362

UI to manage users/permissions for the content libraries [FC-0062] #1362

Conversation

pomegranited commented Oct 4, 2024 • edited Loading

Description

Screenshots

Admin view

Author view

Reader view

Supporting information

Testing instructions

Author Concerns

openedx-webhooks commented Oct 4, 2024 • edited Loading

What's next?

🔘 Get product approval

🔘 Provide context

🔘 Get a green build

🔘 Let us know that your PR is ready for review:

Who will review my changes?

Where can I find more information?

When can I expect my changes to be merged?

codecov bot commented Oct 9, 2024 • edited Loading

Codecov Report

pomegranited Oct 9, 2024

Choose a reason for hiding this comment

rpenido left a comment

Choose a reason for hiding this comment

pomegranited commented Oct 10, 2024

bradenmacdonald left a comment

Choose a reason for hiding this comment

bradenmacdonald Oct 12, 2024

Choose a reason for hiding this comment

pomegranited commented Oct 14, 2024

pomegranited commented Oct 4, 2024 •

edited

Loading

openedx-webhooks commented Oct 4, 2024 •

edited

Loading

codecov bot commented Oct 9, 2024 •

edited

Loading