Arvados

Updates at arvados-workbench2|a4c96d2 - branch 16115-sharing-links
Test run: developer-tests-workbench2: #688

• Adds sharing token management methods to the ApiClientAuthorization service, with tests.
• Adds a new "Sharing URLs" tab to the collection's Share dialog that allows the user to view, create and delete sharing URLs.
• Simplifies the permissions management section of the Share dialog removing the "Advanced" mode and the visibility form and just allow the user to create permissions as we do in wb1.
• Adds unit and cypress tests.

Updates at arvados-workbench2|709996fd
Test run: developer-tests-workbench2: #689

• Fixes cypress test.

Lucas Di Pentima wrote:

Updates at arvados-workbench2|709996fd
Test run: developer-tests-workbench2: #689

• Fixes cypress test.

It works, and it looks nice!

A few questions:

• how does one edit the lifetime of the sharing links? In testing on ce8i5 I get a link that works for a day; what if I want to change the duration? It would make sense to allow the user to specify the duration when creating a link.

• by removing the "Advanced" option, sharing things publicly is now a bit less obvious (you have to share with the anonymous users group). I assume this is intentional?

Otherwise, LGTM!

Updates at arvados-workbench2|6fbb4eca
Test run: developer-tests-workbench2: #708

Addressed both comments by:

• Bringing back the "visibility level" form, but this time is visible in the main sharing dialog from the start instead of being hidden inside an "Advanced" mode.
• Adding the possibility for the user to specify the sharing token's expiration date. We aren't exporting the API.MaxTokenLifetime config at the moment so I've added a note that the selected date could be limited by the cluster's configuration.

Lucas Di Pentima wrote:

Updates at arvados-workbench2|6fbb4eca
Test run: developer-tests-workbench2: #708

Addressed both comments by:

• Bringing back the "visibility level" form, but this time is visible in the main sharing dialog from the start instead of being hidden inside an "Advanced" mode.

That's a great improvement, thank you! One thing that still could be better I think: the "Specific people can access (Shared)" option can be selected in the dropdown, but if you then don't share with anyone and hit save changes, that just changes back to "Private". It would probably be better if "Specific people can access (Shared)" is not an option that can be selected by the user, right? Private automatically becomes "Sharing" when someone else is given access to the collection, so I don't think "Sharing" needs to be a chooseable option in the dropdown.

Also, if you are sharing with some people and then change the collection to "Private" it clears out all the sharing links without any confirmation. That's the correct behavior but I wonder if we need some sort of warning/confirmation for that step?

• Adding the possibility for the user to specify the sharing token's expiration date. We aren't exporting the API.MaxTokenLifetime config at the moment so I've added a note that the selected date could be limited by the cluster's configuration.

Aha, exporting API.MaxTokenLifetime should probably become a separate ticket. Until then, the note is great.

I noticed that the sharing URLs, when setting an expiry date, always end at the current minute, e.g. if I select July 22 at 13:00, I get a link that works until July 22 at 13:17 (because it is 08:17 here right now). Seems like we should probably always set the minutes to 0 ?

On the first tab of the sharing dialog for collections, should we add a note that "Private" collections could still be shared with a URL? If there is an active sharing URL for the collection, we could just add a note that says something like "This collection is accessible via a sharing URL". Maybe even in a different color to make it stand out. I'm worried that the first tab makes it look like everything is locked down tight, while sharing URLs are active.

I don't have any comments on the code, it looks really nice and tidy!

Updates at arvados-workbench2|2d230988
Test run: developer-tests-workbench2: #710

Ward Vandewege wrote:

That's a great improvement, thank you! One thing that still could be better I think: the "Specific people can access (Shared)" option can be selected in the dropdown, but if you then don't share with anyone and hit save changes, that just changes back to "Private". It would probably be better if "Specific people can access (Shared)" is not an option that can be selected by the user, right? Private automatically becomes "Sharing" when someone else is given access to the collection, so I don't think "Sharing" needs to be a chooseable option in the dropdown.

I see what you mean, and I agree this can be improved, but I'm not sure if avoiding the 'Shared' option to be selectable is the way to go, as it would make a particular case in contrast with the other two options, UX-wise I think that would be more confusing.
One working case in choosing the 'Shared' option and then saving without specifying any permissions is when the user previously set the visibility level to 'Public', so I guess it kind of makes sense. This widget would probably benefit from a complete do-over.

Also, if you are sharing with some people and then change the collection to "Private" it clears out all the sharing links without any confirmation. That's the correct behavior but I wonder if we need some sort of warning/confirmation for that step?

This would require some sort of internal data sharing between different forms (the public access form & the permission management form) that I'm not sure how we could do that, at least in a clean manner. As this behavior was already present in the past and we haven't had any complains about it, do you think we could leave it as is for now?

I noticed that the sharing URLs, when setting an expiry date, always end at the current minute, e.g. if I select July 22 at 13:00, I get a link that works until July 22 at 13:17 (because it is 08:17 here right now). Seems like we should probably always set the minutes to 0 ?

Yeah, I did it that way to allow an exact 1h default expiration date. Now, it uses minutes & seconds set to zero and the default expiration date is set in a way that the user at least gets 1h of time.
Also, the "Create sharing URL" button gets disabled if the user selects a date in the past (only possible by selecting an hour in the past).

On the first tab of the sharing dialog for collections, should we add a note that "Private" collections could still be shared with a URL? If there is an active sharing URL for the collection, we could just add a note that says something like "This collection is accessible via a sharing URL". Maybe even in a different color to make it stand out. I'm worried that the first tab makes it look like everything is locked down tight, while sharing URLs are active.

Done. I also added the number of sharing urls to the corresponding tab, for even better awareness.

Lucas Di Pentima wrote:

Updates at arvados-workbench2|2d230988
Test run: developer-tests-workbench2: #710

Ward Vandewege wrote:

That's a great improvement, thank you! One thing that still could be better I think: the "Specific people can access (Shared)" option can be selected in the dropdown, but if you then don't share with anyone and hit save changes, that just changes back to "Private". It would probably be better if "Specific people can access (Shared)" is not an option that can be selected by the user, right? Private automatically becomes "Sharing" when someone else is given access to the collection, so I don't think "Sharing" needs to be a chooseable option in the dropdown.

I see what you mean, and I agree this can be improved, but I'm not sure if avoiding the 'Shared' option to be selectable is the way to go, as it would make a particular case in contrast with the other two options, UX-wise I think that would be more confusing.
One working case in choosing the 'Shared' option and then saving without specifying any permissions is when the user previously set the visibility level to 'Public', so I guess it kind of makes sense. This widget would probably benefit from a complete do-over.

Yeah let's punt on that for now. This is a pretty minor issue.

Also, if you are sharing with some people and then change the collection to "Private" it clears out all the sharing links without any confirmation. That's the correct behavior but I wonder if we need some sort of warning/confirmation for that step?

This would require some sort of internal data sharing between different forms (the public access form & the permission management form) that I'm not sure how we could do that, at least in a clean manner. As this behavior was already present in the past and we haven't had any complains about it, do you think we could leave it as is for now?

Sure, we can always improve this in the future. The functionality of clearing all the links in one go is handy!

I noticed that the sharing URLs, when setting an expiry date, always end at the current minute, e.g. if I select July 22 at 13:00, I get a link that works until July 22 at 13:17 (because it is 08:17 here right now). Seems like we should probably always set the minutes to 0 ?

Yeah, I did it that way to allow an exact 1h default expiration date. Now, it uses minutes & seconds set to zero and the default expiration date is set in a way that the user at least gets 1h of time.
Also, the "Create sharing URL" button gets disabled if the user selects a date in the past (only possible by selecting an hour in the past).

Great, thanks!

On the first tab of the sharing dialog for collections, should we add a note that "Private" collections could still be shared with a URL? If there is an active sharing URL for the collection, we could just add a note that says something like "This collection is accessible via a sharing URL". Maybe even in a different color to make it stand out. I'm worried that the first tab makes it look like everything is locked down tight, while sharing URLs are active.

Done. I also added the number of sharing urls to the corresponding tab, for even better awareness.

Nice! This is excellent.

LGTM thanks!