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

159 - Manage File User Permissions for the Files Tab #165

Merged
merged 17 commits into from
Sep 22, 2023

Conversation

MellyGray
Copy link
Contributor

@MellyGray MellyGray commented Aug 22, 2023

What this PR does / why we need it:

This PR adds the management of the file user permissions for the the Files Tab.

Which issue(s) this PR closes:

Special notes for your reviewer:

Changes

  1. Two new useCases added to manage the file permissions checkFileDownloadPermission and checkFileEditDatasetPermission. Note: The checkFileEditDatasetPermission at the file level may be replaced in the future by a checkDatasetEditPermission at the dataset level but since in the js-dataverse implementation the editDataset permission can only be retrieved using a file, that's what have been implemented in the UI.
  2. The File model has been refactored to remove the canDowload property since this property is actually a permission instead of a property of the resource. This affects most of the changed code
  3. A FilePermissionsProvider has been added to globally manage the file permissions in the application

Suggestions on how to test this:

To test this from a UI perspective, the Files Tab elements in the Storybook should continue being displayed as in the previous PRs, this is because in previous PRs the permissions were hardcoded and now they have been implemented with their own use cases. So basically the testing consists on checking that everything is being displayed in the Files Table as it was before, for example as it was working in the feature/139-files-table-file-action-buttons-ui branch

  1. npm i
  2. cd packages/design-system && npm run build
  3. cd ../../
  4. npm run storybook
  5. Check the Dataset Page and the stories in Sections/Dataset Page/DatasetFiles/

Is there a release notes update needed for this change?:

No

Additional documentation:

@MellyGray MellyGray changed the base branch from develop to feature/139-files-table-file-action-buttons-ui August 22, 2023 14:48
@coveralls
Copy link

coveralls commented Aug 22, 2023

Coverage Status

Changes unknown when pulling abbf383 on feature/159-manage-user-permissions-files into ** on feature/139-files-table-file-action-buttons-ui**.

@MellyGray MellyGray marked this pull request as ready for review August 22, 2023 15:48
@MellyGray MellyGray added Size: 3 A percentage of a sprint. 2.1 hours. Size: 10 A percentage of a sprint. 7 hours. and removed Size: 3 A percentage of a sprint. 2.1 hours. Size: 10 A percentage of a sprint. 7 hours. labels Aug 22, 2023
@MellyGray MellyGray self-assigned this Aug 23, 2023
@MellyGray MellyGray removed their assignment Aug 23, 2023
@pdurbin pdurbin self-assigned this Sep 6, 2023
Copy link
Member

@pdurbin pdurbin left a comment

Choose a reason for hiding this comment

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

I'm approving this. I didn't do any particular testing apart from getting the section to load in story book.

I looked through the diff of just this PR (which merges into non-develop) at feature/139-files-table-file-action-buttons-ui...feature/159-manage-user-permissions-files

I forget if I asked this already, but I still wonder why it's called DatasetMother. I wonder if it should be DatasetParent instead.

Also, it's weird to me that there are 200 total files in Storybook and somehow within that small number there are 100K embargoed files. 😄

Screen Shot 2023-09-11 at 3 06 38 PM

@pdurbin pdurbin removed their assignment Sep 11, 2023
@kcondon kcondon self-assigned this Sep 11, 2023
@kcondon
Copy link
Contributor

kcondon commented Sep 11, 2023

@MellyGray I actually had trouble loading this in Storybook. I saw two blank pages:

Screen Shot 2023-09-11 at 4 15 10 PM

@GPortas
Copy link
Contributor

GPortas commented Sep 12, 2023

@pdurbin For any class you see with the "Mother" suffix, it is a naming convention for classes used in testing that create objects that we use for tests.

https://martinfowler.com/bliki/ObjectMother.html

@MellyGray
Copy link
Contributor Author

@MellyGray I actually had trouble loading this in Storybook. I saw two blank pages:

Screen Shot 2023-09-11 at 4 15 10 PM

@kcondon mmm, I tried with the above steps and it's working for me.

Sometimes I fix the blank page in Storybook by deleting the node_modules cache, from the dataverse-frontend/ root run:

rm -rf node_modules/.cache && npm i

If that doesn't work then maybe try deleting the whole node_modules folder:

rm -rf node_modules && npm i

Also sometimes it takes a while for storybook to load the pages, so try waiting for sometime to see if the console prints something else such as another error or [vite] connecting

If none of the above works, we can try troubleshooting in a zoom call because I've never seen that error message in the console. I'd say it's something about the dependencies but no idea.

@kcondon
Copy link
Contributor

kcondon commented Sep 13, 2023

@MellyGray I keep seeing blank pages, even after trying the tricks:

Sometimes I fix the blank page in Storybook by deleting the node_modules cache, from the dataverse-frontend/ root run:

rm -rf node_modules/.cache && npm i

If that doesn't work then maybe try deleting the whole node_modules folder:

rm -rf node_modules && npm i

@MellyGray
Copy link
Contributor Author

@kcondon We can try troubleshooting in a zoom call after the standup

@kcondon
Copy link
Contributor

kcondon commented Sep 15, 2023

@MellyGray Yes, maybe try after standup, thanks!

@kcondon
Copy link
Contributor

kcondon commented Sep 19, 2023

@MellyGray Works well but the changed behavior of the checkbox selecting all on a page rather than all isn't in this -still selects all files.

@MellyGray
Copy link
Contributor Author

Works well but the changed behavior of the checkbox selecting all on a page rather than all isn't in this -still selects all files.

@kcondon I've merged the changes from 'develop' into this branch, so now the checkbox should work as expected here as well

Base automatically changed from feature/139-files-table-file-action-buttons-ui to develop September 22, 2023 07:29
@kcondon kcondon merged commit 8f49243 into develop Sep 22, 2023
6 checks passed
@kcondon kcondon deleted the feature/159-manage-user-permissions-files branch September 22, 2023 07:37
jayanthkomarraju pushed a commit to jayanthkomarraju/dataverse-frontend that referenced this pull request May 31, 2024
…ions-files

159 - Manage File User Permissions for the Files Tab
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Size: 3 A percentage of a sprint. 2.1 hours.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Spike - Frontend] Manage user permissions for the conditional rendering
5 participants