464 datasets visible to logged in users with AUTHENTICATED status #466
Conversation
|
This one is ready for review. |
# Conflicts: # backend/app/routers/authorization.py
|
Based on yesterday's discussion, there seems to be more refactoring needed for this? @tcnichol |
|
Yes. I'm going to try add new dependencies so that the public datasets will be visible when you are not logged in. So I'll keep working on this one. I will also change this to draft. |
# Conflicts: # backend/app/models/datasets.py # backend/app/routers/files.py # backend/app/routers/metadata_files.py # frontend/src/components/datasets/Dataset.tsx # frontend/src/components/datasets/OtherMenu.tsx
we can change status, new status reflected on other users explore pages
There was a problem hiding this comment.
Overall looks good to me. A couple of proposed changes:
- When a dataset is public and a user has access to it, they can execute an extractor. I am not sure we want to let them do this. We could disable/hide the button or maybe remove the extract tab all together?
- I don't think the "Published" option does anything right now. We should deleted from the list of options.
|
@tcnichol can fix conflicts? Have you been able to address to the two small changes above? Thanks! |
# Conflicts: # frontend/src/components/metadata/widgets/MetadataSelect.tsx
I'll be taking care of that this afternoon. |
|
Pushed new commits, issues should be resolved and merge conflict fixed. |
tcnichol
changed the title
tcnichol
changed the title
|
The status is now changed to AUTHENTICATED. The difference will be |
with other popups and renamed to Update.
lmarini
left a comment
lmarini
left a comment
There was a problem hiding this comment.
@tcnichol still works well after changes to label. I made one small change. I moved the submission button to bottom dialog actions in line with other popups and renamed to Update. Please take a look so we are consistent on all popups. Thank you.
For this pull request, the following has been implemented.
If a dataset is public, all users can view the dataset, the files, and the metadata.
A few things I might want to change:
In the route authorization.get_dataset_role, the response type is AuthorizationDB instead of just RoleType (which is the return type for similar methods for file, metadata, etc have.) So for the .get_dataset_role method I create an AuthorizationDB with RoleType 'Viewer' and return that if the dataset is public and the user has no role.
If anybody thinks it would be a better approach, I could change the response type of the .get_dataset_role method to RoleType. That seems like it might make more sense, but I wasn't sure if having the actual authorizationDB object was important anywhere else.
I have also not figured out a solution for allowing users to view public datasets when not logged in. Thought it would be good to handle this, get some feedback, and then move forward.