Arvados

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.

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.

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?

Thanks Brett. Merged into master after making the update.