Arvados

11652-recursive-contents @ fd4bdb760eb47013b8b60b40859ba7f004ee8383

LGTM

Please consider adding the following three tests in groups_controller_test.rb

  test 'get contents recursive=true from home project' do
    authorize_with :active
    get :contents, {
          recursive: true,
          format: :json,
        }
    owners = json_response['items'].map do |item|
      item['owner_uuid']
    end
    assert_includes(owners, users(:active).uuid)
    assert_includes(owners, groups(:aproject).uuid)
    assert_includes(owners, groups(:asubproject).uuid)
  end

  test 'get contents recursive=false' do
    authorize_with :active
    get :contents, {
          id: groups(:aproject).uuid,
          recursive: false,
          format: :json,
        }
    owners = json_response['items'].map do |item|
      item['owner_uuid']
    end
    assert_includes(owners, groups(:aproject).uuid)
    assert_not_includes(owners, groups(:asubproject).uuid)
  end

  test 'get contents with no recursive flag' do
    authorize_with :active
    get :contents, {
          id: groups(:aproject).uuid,
          format: :json,
        }
    owners = json_response['items'].map do |item|
      item['owner_uuid']
    end
    assert_includes(owners, groups(:aproject).uuid)
    assert_not_includes(owners, groups(:asubproject).uuid)
  end

Lucas: branch 11652-recursive-contents-wb is ready for review @ f1aed11fd3a5e204a2cf2e4fc5f099179e877eb6

This branch implements the workbench side of recursive project contents for search.

To review: please compare against the 11652-recursive-contents branch (which is being addressed by Tom to implement the API server support for recursive flag. I will merge this branch after Tom merges 11652-recursive-contents branch.)

git diff 11652-recursive-contents...11652-recursive-contents-wb

11652-recursive-contents @ 29665e2d9a543bffb237d148c3484c03b03e30aa

A couple of small comments:

• File apps/workbench/test/controllers/search_controller_test.rb
  • Line 51: Shouldn’t the message say something like “search results for non empty project should not be empty”?
• File services/api/lib/can_be_an_owner.rb
  • Line 36: I think the map call is not needed, shouldn’t exec_query(‘…’).rows be enough in this case?

The rest LGTM. Thanks!

search_controller_test.rb - Line 51: Shouldn’t the message say something like “search results for non empty project should not be empty”?

Thanks for catching that. Copy/paste lapse. Corrected

File services/api/lib/can_be_an_owner.rb - Line 36: I think the map call is not needed, shouldn’t exec_query(‘…’).rows be enough in this case?

With exec_query.rows, we get this for descendets: zzzzz-j7d0g-axqo7eu9pwvna1x%22%5D_%5B%22zzzzz-j7d0g-subprojsamenam2%22">"zzzzz-j7d0g-axqo7eu9pwvna1x"], ["zzzzz-j7d0g-subprojsamenam2" , where as with the other we get this ["zzzzz-j7d0g-axqo7eu9pwvna1x", "zzzzz-j7d0g-subprojsamenam2"]. It may work, but better to have the right format. (This is already merged by Tom)

Thanks.