Bug #18594: Collection Advanced Menu is trying to fetch User record with collection UUID - Arvados

I've been trying to reproduce the bug to make sure that the committed code is correctly addressing the issue. This ticket is lacking a good explanation on how to do this, so I based my attempts on the committed test and did the following in https://workbench2.ce8i5.arvadosapi.com:

Created a project with user A (admin)
Shared the project with user B (non-admin), with can_write permission.
Accessed to the "Shared with me" section with user B, right-clicked on the project recently created.
Clicked on "Advanced" context menu item.
Clicked on the "Metadata" tab.

As a result, I didn't got the error. I have also made the same tests between 2 non-admin users.

Then, I made my own testing branch 18594-only-test-commit with just the commit that added the Cypress test (arvados-workbench2|001f20f0) and launched a Jenkins job (developer-tests-workbench2: #577 ) to see if the test would fail without the fix, but it didn't.

If this fix really addressed the issue the reporting user was suffering, it might be an edge case, or special situation that needs to be investigated so that we address it appropriately, with a correct test that fails when run without the fix.

Actions

Copy link

Updated by Lucas Di Pentima almost 3 years ago

From chat:

I’ve just successfully replicated the issue following your comments from standup:

Created a “role” group (instead of a project).
Created a project group.
Shared the project with the role group.
Right-clicked on the project and selected "Advanced" => I got the error and wb2 tried to make a users GET call with the role group uuid.

I think the solution should be to correctly make a group GET call in those cases so that user groups permissions assigned to projects are listed too.

Actions

Copy link

Updated by Daniel Kutyła almost 3 years ago

New version first commit: https://dev.arvados.org/projects/arvados-workbench-2/repository/revisions/261201611113224f4a61b3979f6cd9992c4aa6c6
Test run: developer-tests-workbench2: #584
Branch: 18594-Collection-Advanced-Menu-is-trying-to-fetch-User-record-with-collection-UUID

Added multiple tails display, extended tests

Actions

Copy link

Updated by Lucas Di Pentima almost 3 years ago

There's a failed test
File src/store/advanced-tab/advanced-tab.tsx
- Lines 288 & 292 are abusing the link resource's properties field by stashing an unrelated piece of information that doesn't belong there. If the link happens to have a property with a "tail" key, it would be overwritten.
File src/views-components/advanced-tab-dialog/metadataTab.tsx
- Line 50 used to have a reference to props.user, and now it doesn't so effectively the user prop doesn't need to be passed around (because AFAICT this line was the only one that needed it).
What I'm trying to say is that it seems that all the user variable handling code from advanced-tab.tsx to metadataTab.tsx has always been unnecessary because the code already has the metadata links available to do some additional querying if needed.
In summary, you could remove all the user var related code from arvanced-tab.tsx and make getDataForAdvancedTab() not return it. Then, remove all references of that return value from the three places where they're expected, and then on the metadataTab.tsx, you could show UUIDs on the tail column (and not fiddle with the properties field)