Bug #11060
closed[Workbench] User-facing "all visible repositories" page
Description
"repositories" page does not show repositories shared with "All users"
This is because workbench is not computing transitive permissions for repositories, it is only listing repositories with direct ownership and direct permission links. It needs to be able to get the list of repositories along with the current user's access rights to each repo.
Previous description:
Currently, /repositories shows all visible repositories, but there is no link to that page unless you're an admin.
/users/{uuid}/repositories shows (and should continue to show) only the current user's own repositories.
Files
Updated by Tom Morris over 7 years ago
- Target version changed from 2017-07-05 sprint to 2017-06-21 sprint
Updated by Tom Clegg over 7 years ago
- Subject changed from [Workbench] "repositories" page does not show repositories shared with "All users" to [Workbench] User-facing "all visible repositories" page
- Description updated (diff)
Updated by Radhika Chippada over 7 years ago
- Assigned To set to Radhika Chippada
Updated by Radhika Chippada over 7 years ago
- File repositories.jpg repositories.jpg added
Also, discussed this with Peter yesterday. We have two options regarding this implementation.
Option 1: Enhance /<user_uuid>/repositories page (accessible from User dropdown in topnav) to include "shared repositories". Please see the attached "repositories.jpg". I also think we should add a "delete" icon to rows in the My Repositories section.
Option 2: Add a link in to /repositories page in /<user_uuid>/repositories page as Tom said in the description. However the UI for /repositories page is very basic and most importantly is missing the URL information. So, if we want to use this option, we should consider improving the /repositories page display and add URL column (similar to /<user_uuid>/repositories page)
Which option do we want to use?
Updated by Tom Morris over 7 years ago
Having two different page layouts seems unnecessary to me, but each is currently missing some important pieces.
- Individual repositories page is missing delete (as you point out) and "show" is non-obviously hidden behind the "Share" button
- All repositories page is missing URLs and "Share" is hidden behind "Show"
I think my preference would be to use the individual repositories page as a base, but make the columns consistent with ordering on other pages (e.g. collections), so that we have
Trashcan "Show" <username>/<reponame> <cloning URLs>
icon
I think that layout will work for both pages.
Updated by Radhika Chippada over 7 years ago
- Status changed from New to In Progress
Branch 11060-all-visible-repos @ 4b056c34fa04a3b611a0cf5f70e9b8d59b7ef632
- Display all repositories accessible to the user in the user's repositories page. Display owned repos first, then those that the user can can_write / can_manage, and then the remaining repos
- In each repository row, included Show and Delete icons. Similar to other index pages, (not project tabs), placed the Delete icon on the right end of the row.
Test run @ https://ci.curoverse.com/job/developer-run-tests/327/
Updated by Peter Amstutz over 7 years ago
Workbench is only listing repositories with direct ownership and direct permission links. It is doing this (instead of just getting the straight repository listing) because we want to show whether the user has read/write/manage permission on each repository.
It would be more consistent to be able to query the repository with a normal listing, and have that include virtual fields indicating whether the current user has write/manage access to each repo.
Updated by Peter Amstutz over 7 years ago
Groups include a meta-field called "writable_by". Perhaps repositories return this field as well?
Updated by Peter Amstutz over 7 years ago
Before I forget:
In terms of layout, instead of a "Show" button can we have the following two buttons:
- "Browse" which links to
/repositories/zzzzz-s0uqq-zzzzzzzzzzzzzz/tree/master
- "Share" which links to
/repositories/zzzzz-s0uqq-zzzzzzzzzzzzzz#Sharing
Updated by Tom Clegg over 7 years ago
Peter Amstutz wrote:
Groups include a meta-field called "writable_by". Perhaps repositories return this field as well?
I expect we merely have to add "t.add :writable_by" to source:services/api/app/models/repository.rb (as we do in source:services/api/app/models/group.rb). The method is in ArvadosModel.
Updated by Peter Amstutz over 7 years ago
On further inspection, currently it does not display whether you have read/write/manage access to each repository. So it should be correct to just get repository list readable by the current user and not apply any additional filtering.
Updated by Peter Amstutz over 7 years ago
Comments / recommendations:
all_repositories = Hash[Repository.where(authorized_user_uuid: current_user.uuid).collect {|repo| [repo.uuid, repo]}]
The field authorized_user_uuid
only exists for authorized_keys
, not repositories
. It seems to be entirely ignored here. Looks like a copy-and-paste error. Also we should sort by repository name:
all_repositories = Hash[Repository.order(:name).all.collect {|repo| [repo.uuid, repo]}]
Let's get rid of this the query for "repo_links". It only returns direct permission links and not transitive permissions, so the sorting will be inconsistent depending on whether the permission is direct or indirect. Prefer the simpler sort order of [my repositories, everything else].
Because we're just displaying "Repository.all" (and there's no way to filter on "readable by user in param[:uuid]") the entire page should move to "/repositories" and the page under /users/<uuid>/repositories" should go away.
For layout, instead of (or in addition to) the "Show" button we I would like to see the following two buttons:
- "Browse" which links to
/repositories/zzzzz-s0uqq-zzzzzzzzzzzzzz/tree/master
- "Share" which links to
/repositories/zzzzz-s0uqq-zzzzzzzzzzzzzz#Sharing
There needs to be a test that a repository shared with "all users" (but not directly with the user) shows up in the user's repository list.
Updated by Radhika Chippada over 7 years ago
@ 1883b07d5de707066bf996c1526073df323c3991
all_repositories = Hash[Repository.order(:name).all.collect {|repo| [repo.uuid, repo]}]
Done
Let's get rid of this the query for "repo_links" ...
Updated sorting accordingly
the entire page should move to "/repositories" and the page under /users/<uuid>/repositories" should go away
Since this involves tweaking / moving several files and logic from users controller and user views, and updating tests accordingly, I created a new ticket for this. #11870
"Browse" link
The <repo_uuid>/tree/master is not accessible for all repositories and I am getting 404 errors. I also think a new tab or direct link in the repository Show page itself, after determining it is available for that repo, is a better place for this functionality. Added #11871
"Share" link
As Tom Morris said in his note 7, the Share tab is available once the user clicks and transitions into the repo show page. Also per your note 14, we cannot accurately determine the access level for each repository. Hence, I am omitting this as well in favor of Show link.
Tests
Added the check to "arvados" repository, which is shared with all_users.
Test run @ https://ci.curoverse.com/job/developer-run-tests/337/
Thanks.
Updated by Peter Amstutz over 7 years ago
- This doesn't seem to work (it's not actually ordered by name):
Hash[Repository.all.order(:name).collect {|repo| [repo.uuid, repo]}]
But this works:
Hash[Repository.order('name asc').collect {|repo| [repo.uuid, repo]}]
- The link "For more information see Writing a pipeline." goes to "/user/tutorials/tutorial-firstscript.html" which at the top says "Arvados pipeline templates are deprecated."
I think we can safely remove that link from the repositories page.
- The delete dialog box says "Really delete repositories?" which is confusing (we're only deleting one repository). It should say "Really delete repository <name>?"
- There's no infinite scrolling / paging. On qr1hi, admins can see almost 1000 repositories. The page renders fast enough that I don't think it is a problem right now, but just wanted to mention it.
Updated by Radhika Chippada over 7 years ago
@ 9318b234cbfae138524e45080f549c277e15776b
This works: Hash[Repository.order('name asc').collect {|repo| [repo.uuid, repo]}]
Ah, yes, I noticed this but later forgot to correct it as I was working on all other items in the note. Thanks.
The link "For more information see Writing a pipeline." goes to "/user/tutorials/tutorial-firstscript.html" which at the top says "Arvados pipeline templates are deprecated." ...I think we can safely remove that link from the repositories page.
Removed this
The delete dialog box says "Really delete repositories?" which is confusing (we're only deleting one repository). It should say "Really delete repository <name>?"
Corrected this
There's no infinite scrolling / paging. On qr1hi, admins can see almost 1000 repositories. The page renders fast enough that I don't think it is a problem right now, but just wanted to mention it.
Since we want to move this functionality into /repositories page, I added a note to #11870 to this effect.
Thanks.
Updated by Radhika Chippada over 7 years ago
- Status changed from In Progress to Resolved
- % Done changed from 0 to 100
Applied in changeset arvados|commit:8c2b89cb6a34b2f1a4ed672e8a883c680ffca80a.