Bug #18594

Collection Advanced Menu is trying to fetch User record with collection UUID

Added by Daniel Kutyła 5 months ago. Updated 3 months ago.

Status:
Resolved
Priority:
Normal
Assigned To:
Category:
Workbench2
Target version:
Start date:
02/03/2022
Due date:
% Done:

100%

Estimated time:
(Total: 0.00 h)
Story points:
-
Release relationship:
Auto

Description

This throws an error message on WB2.


Subtasks

Task #18706: Review 18594-Collection-Advanced-Menu-is-trying-to-fetch-User-record-with-collection-UUIDIn ProgressDaniel Kutyła

Associated revisions

Revision 5ec1472f
Added by Daniel Kutyła 3 months ago

Merge branch '18594-Collection-Advanced-Menu-is-trying-to-fetch-User-record-with-collection-UUID' into main
closes #18594

Arvados-DCO-1.1-Signed-off-by: Daniel Kutyła <>

Revision 4cfd1b38 (diff)
Added by Daniel Kutyła 3 months ago

Merge branch '18594-Collection-Advanced-Menu-is-trying-to-fetch-User-record-with-collection-UUID' into main
closes #18594

Arvados-DCO-1.1-Signed-off-by: Daniel Kutyła <>

Revision 943b2db5 (diff)
Added by Daniel Kutyła 3 months ago

Merge branch '18594-Collection-Advanced-Menu-is-trying-to-fetch-User-record-with-collection-UUID' into main
closes #18594

Arvados-DCO-1.1-Signed-off-by: Daniel Kutyła <>

History

#2 Updated by Daniel Kutyła 4 months ago

  • Target version set to 2022-02-02 sprint
  • Assigned To set to Daniel Kutyła
  • Status changed from New to In Progress

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

Added check if metadata tailKind is of the user resource kind

#3 Updated by Peter Amstutz 4 months ago

  • Target version changed from 2022-02-02 sprint to 2022-02-16 sprint

#4 Updated by Daniel Kutyła 4 months ago

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

Added cypress test

#5 Updated by Lucas Di Pentima 4 months ago

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.

#6 Updated by Lucas Di Pentima 4 months 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.

#7 Updated by Daniel Kutyła 3 months 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

#8 Updated by Lucas Di Pentima 3 months 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)

#9 Updated by Daniel Kutyła 3 months ago

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

Code and test cleanup

#10 Updated by Lucas Di Pentima 3 months ago

Just one observation:

  • There's still a failing test. Hint, hint: cypress/integration/project.spec.js file, line 205.

The rest LGTM.

#11 Updated by Daniel Kutyła 3 months ago

  • % Done changed from 0 to 100
  • Status changed from In Progress to Resolved

#12 Updated by Peter Amstutz 3 months ago

  • Release set to 49

Also available in: Atom PDF