374 add groups to datasets via gui #398
Conversation
* Added dataset name as parameter * Added Share button to menu * Added new custom component that has the share pane UI
| setGroup(event.target.value); | ||
| }} | ||
| > | ||
| </Select> |
There was a problem hiding this comment.
This dropdown is always blank even though options shows all the groups added. Not sure what is wrong with it.
There was a problem hiding this comment.
This is now fixed. The dropdown shows the groups.
|
based on #391 |
|
I'm not sure what is going on with the pytest here. I ran locally and all the tests passed. |
| let group_option = {value:group.id, label:group.name} | ||
| options.push(group_option); | ||
| }); | ||
| console.log('group optioins are', options); |
There was a problem hiding this comment.
I've reviewed this PR and tested it, looks good to me! Two minor comments:
- Can we remove these console log statements?
We need to enforce that only the dataset "owner" can share rolesEDIT: I checked the code and looks like the "OtherMenu" component is only accessible to the owner of the dataset, so I think we are good on this front.
I'll add the remaining changes to this branch directly so we can merge both features together. Thanks!
There was a problem hiding this comment.
We might need to think about whether an editor should be able to share a dataset. But that can be a separate issue.
* Tested, verified that mongoDB adds entry to collection when hit submit button
| } | ||
| sx={{ mb: 2 }} | ||
| > | ||
| Successfully added role! |
There was a problem hiding this comment.
This is what the success dialog looks like
|
I tested this and the sharing feature works, two questions:
|
|
For sharing, I think it might be better to keep the box open in case a user wants to share a dataset with multiple users and groups. Though this might be something to get feedback on later. With the autocomplete, we might want to make any changes related to that other issues. |
Just adding as draft pull request to make it easier to review and get feedback.