Bug #5189

[Workbench] Manage account does not show repositories directly owned by user

Added by Peter Amstutz about 5 years ago. Updated about 5 years ago.

Status:
Resolved
Priority:
Normal
Assigned To:
Radhika Chippada
Category:
-
Target version:
Start date:
02/16/2015
Due date:
% Done:

100%

Estimated time:
(Total: 0.00 h)
Story points:
0.5

Description

The manage account page only shows repositories for which there are permission links. It should also show repositories which are directly owned by the current user.

See arvados/apps/workbench/app/controllers/users_controller.rb#manage_account


Subtasks

Task #5223: Review branch: 5189-manage-acct-shows-own-reposResolvedRadhika Chippada

Associated revisions

Revision 1ee6ce5b
Added by Radhika Chippada about 5 years ago

closes #5189
Merge branch '5189-manage-acct-shows-own-repos'

History

#1 Updated by Peter Amstutz about 5 years ago

  • Story points set to 0.5

#2 Updated by Peter Amstutz about 5 years ago

  • Subject changed from [Workbench] Manage acct does not show repositories directly owned by user to [Workbench] Manage account does not show repositories directly owned by user
  • Description updated (diff)

#3 Updated by Peter Amstutz about 5 years ago

  • Description updated (diff)

#4 Updated by Radhika Chippada about 5 years ago

  • Status changed from New to In Progress
  • Assigned To set to Radhika Chippada
  • Target version changed from Bug Triage to 2015-02-18 sprint

#5 Updated by Brett Smith about 5 years ago

Reviewing 45d8d01

This code calls Repository.where(owner_uuid: current_user.uuid) twice. This can make two API calls, which adds unnecessary overhead. Would it be possible to rearrange things so that we only do this call once? Maybe set owned_repositories = Repository.where(owner_uuid: current_user.uuid) first, use it to build @my_repositories, and then reference UUIDs explicitly in the loop where @repo_writable is extended?

Thanks.

#6 Updated by Radhika Chippada about 5 years ago

Brett, I need separate lists @my_repositories (which has the list of repositories for the links + owned repositories) and owned_repositories. Rather than getting repos for links and add owned_repositories, I am getting it together. I have to make two calls either way, so really not saving an API call doing one way or the other. Hence, I am going to leave this as it is. Thanks.

#7 Updated by Brett Smith about 5 years ago

Radhika Chippada wrote:

Brett, I need separate lists @my_repositories (which has the list of repositories for the links + owned repositories) and owned_repositories. Rather than getting repos for links and add owned_repositories, I am getting it together.

I understand that you need to be able to keep track of the lists separately. However, the current code makes three API calls total: once for the links, and twice for owned repositories. I'm suggesting making each call only once, for a total of two calls. Like this:

 def manage_account
    # repositories current user can read / write
    repo_links = Link.
      filter([['head_uuid', 'is_a', 'arvados#repository'],
              ['tail_uuid', '=', current_user.uuid],
              ['link_class', '=', 'permission'],
             ])
    owned_repositories = Repository.where(owner_uuid: current_user.uuid)

    @my_repositories = (Repository.where(uuid: repo_links.collect(&:head_uuid)) |
                        owned_repositories).
                       uniq { |repo| repo.uuid }

    @repo_writable = {}
    repo_links.each do |link|
      if link.name.in? ['can_write', 'can_manage']
        @repo_writable[link.head_uuid] = link.name
      end
    end

    owned_repositories.each do |repo|
      @repo_writable[repo.uuid] = 'can_manage'
    end

    # rest of the method
  end

This code passes all controller tests, including the new one in the branch. Is there some other reason it wouldn't work?

#8 Updated by Radhika Chippada about 5 years ago

Thanks Brett. Merged into master after making the update.

#9 Updated by Radhika Chippada about 5 years ago

  • Status changed from In Progress to Resolved

Applied in changeset arvados|commit:1ee6ce5be0c86c1d2e903252ba2a70694be5cf31.

Also available in: Atom PDF