-
Notifications
You must be signed in to change notification settings - Fork 16
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
Collection permissions check #463
Conversation
…collection and create dataset page
Ready for Review again, some e2e tests for the FileRepository were failing randomly, there is an open issue for that already. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Just some small suggested changes
tests/component/sections/shared/add-data-actions/AddDataActionsButton.spec.tsx
Outdated
Show resolved
Hide resolved
tests/component/sections/shared/add-data-actions/AddDataActionsButton.spec.tsx
Outdated
Show resolved
Hide resolved
tests/component/shared/hooks/useGetCollectionUserPermissions.spec.ts
Outdated
Show resolved
Hide resolved
tests/component/shared/hooks/useGetCollectionUserPermissions.spec.ts
Outdated
Show resolved
Hide resolved
tests/component/sections/shared/add-data-actions/AddDataActionsButton.spec.tsx
Outdated
Show resolved
Hide resolved
…sButton.spec.tsx Co-authored-by: Ellen Kraffmiller <ekraffmiller@hmdc.harvard.edu>
…pec.ts Co-authored-by: Ellen Kraffmiller <ekraffmiller@hmdc.harvard.edu>
…pec.ts Co-authored-by: Ellen Kraffmiller <ekraffmiller@hmdc.harvard.edu>
@ekraffmiller thanks! all changes done |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good, approved!
Merge conflicts solved |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have tested the permissions flow and works as expected.
Merging!
What this PR does / why we need it:
Which issue(s) this PR closes:
Closes #434
Special notes for your reviewer:
I have also changed the alias 'root' used in several components for an exported constant, to avoid mistakes and improve reusability, as it is done in js-dataverse.
Suggestions on how to test this:
Does this PR introduce a user interface change? If mockups are available, please link/include them here:
Is there a release notes update needed for this change?:
No
Additional documentation:
No