Arvados

New version first commit: https://dev.arvados.org/projects/arvados-workbench-2/repository/revisions/decb7304af4d86f0aff7a2d7c87f869166111499
Test run: developer-tests-workbench2: #265

Removed uuid user filter

New version first commit: https://dev.arvados.org/projects/arvados-workbench-2/repository/revisions/87d9f6d4a643e5a5ec381f221d376b66c1f3cb29
Test run: developer-tests-workbench2: #266

Removed uuid user filter

• Cannot reproduce the reported issue as it's described on the ticket.
• Please write some integration tests exposing the problem so that we're sure it's fixed.

New version first commit: https://dev.arvados.org/projects/arvados-workbench-2/repository/revisions/a45fc1ed0f5e0385e3741cca6c0b48284ae6f8bb
Test run: developer-tests-workbench2: #282

Added browser tests, excluded non writable by user

Some comments/suggestions:

• At file cypress/integration/favorites.spec.js:
  • Line 54: There's a only() call that avoids other test cases to be run.
  • Line 66 & 85: There's a wait() call that is usually a sign of brittle testing. Instead, could you instead add an assertion/guard for the test framework to wait on before continuing?
  • Lines 97-141: Could we write that part of the test without so many indentation? I remember hearing you said the new Cypress has features to do that. I think it would be good for readability/maintainability.
  • Line 125: There're trailing whitespaces that need removing (you could probably configure your editor to automatically do these kind of things).
• At file src/store/tree-picker/tree-picker-actions.ts
  • Lines 257-263: I believe the loadFavoritesProject() action is used by several different parts of the app, sometimes needing to show all favorites. I think the fact that only writable favorites be listed should be a parameter of the action, with the default being the previous behavior so we don't add bugs to pre-existing code.
  • Related to the above, please check there're tests making sure the favorite picker works correctly on both modes: "writable only" and "all favorites".

Regarding my comment about loadFavoritesProject(), I think the discovered underlying bug also avoids actions like allowing the user to select a workflow input collection that's inside a read-only favorited project from another user. That's why I think we should be adding the possibility to display "all projects" in addition to "writable projects" (and adjusting the different client code of that action to show the proper listing in each case).

I've skimmed the code a bit and couldn't find how the left side panel's "My Favorites" tree picker is populated. It's obviously not using the loadFavoritesProject() and I think it would be really great that we re-use that code for everything that needs a favorites tree picker, so that we get an unified behavior across de entire app.

Regarding my 'individual favorited collections' comment on chat, please dismiss it, as I've re-read the code (and tested manually) and those collections are being included correctly.

New version first commit: https://dev.arvados.org/projects/arvados-workbench-2/repository/revisions/f60bd51d56ccbb4d6954ae407dbf992c26319e8e
Test run: developer-tests-workbench2: #283

Fixed tests, added option to limit favorite tree results

Danny: This bug report is unveiling more related issues, here're my comments:

• At file cypress/integration/favorites.spec.js
  • Line 131: The test is relying on the fact that "Favorites" is listed last on the chooser dialog, could this be done in a more ordering-independent way? Like adding some data-cy attribute specifically named after every category?
  • Line 134: This also seems to be relying on ordering, could it be just checking that the form-dialog doesn't contain the read-only shared project name?
  • The test is about moving a collection into another project, but only gets up to the point of listing favorited (writable) shared projects on the picker. Could you extend it to do the moving and checking that it was successful?
  • Could you rename the mySharedProjectN vars with more meaningful names? One is read-only, the other writable, I think that fact should be on the names so that reading the test is easier.
• This ticket's description talks about the operation of selecting a file inside a collection and using the "copy selected into collection" option, could you add a complete test for that? I've tried it manually and I'm seeing two problems:
  • When having a favorited read-only collection, it's being listed on the picker and obviously returns an error when trying to copy something into it.
  • When trying to copy something into a writable collection I'm getting the error: "422 Unprocessable Entity: Manifest text Invalid manifest: does not end with newline" (haven't checked what's wb2 trying to do to get this message)
• The ProjectsTreePicker now has the same issue that loadFavoritesProject() had on my last review: that is hard-coding configuration and what I think we want is to be able to set it up differently depending on the place it's being used. A quick lookup at the code shows me that ProjectsTreePicker is being used on:
  • src/views-components/projects-tree-picker/tree-picker-field.tsx
  • src/views/run-process-panel/inputs/directory-array-input.tsx
  • src/views/run-process-panel/inputs/directory-input.tsx
  • src/views/run-process-panel/inputs/file-array-input.tsx
  • src/views/run-process-panel/inputs/file-input.tsx
• ...could you investigate the use cases on every one of the above files and see in which mode the ProjectsTreePicker should be used? The src/views/run-process-panel/inputs/* files probably need to be able to show all objects, because I suppose they're workflow inputs selectors.
• Also I saw that ProjectsTreePicker includes SharedTreePicker and PublicFavoritesTreePicker both of which I believe they should be configurable to list "writable" or "all" objects, do you agree?

New version first commit: https://dev.arvados.org/projects/arvados-workbench-2/repository/revisions/4860e74d347552476c77e313c85502787b6d7dfe
Test run: developer-tests-workbench2: #285

Fixed copying collection elements to another collection, this should work although it still contains some flaws
- collection with many elements that only few are copied to another collection can perform very slow (due to required removal of non selected files)
- copy of nested elements is not possible due to parent being not selected when at least one of the children is not selected

• Minor issue: typo on data-cy attribute at src/views-components/projects-tree-picker/projects-tree-picker.tsx Line 37 (and tests)
• Integration tests failed on Jenkins:
  • The collections panel test failure was already fixed on current master, so I think you can dismiss it
  • The favorites test (cypress/integration/favorites.spec.js line 175) failure also happens on my local environment. Also I believe you could remove the loginAs() call from line 163 as it's already logged in as active user on line 146.
• I manually tried to copy some file to a collection inside favorited shared (with write access) project from other user and it's not listed inside the "Favorites" picker. Can you also write an integration test for that case?
• The same happens when I try to copy a selected file to a favorited shared (writable) collection, the target collection isn't listed.
• Related to both above bulletpoints: the "Shared with me" tree picker does show both writable and read-only objects on a chooser that should just show writable objects because what the user is trying to do is write a file on some target.
  • I think this should be fixed as it's related to the "showOnlyWritable" flag that was added on this branch.
  • Please also add tests for this.

New version first commit: https://dev.arvados.org/projects/arvados-workbench-2/repository/revisions/6e807fccb8a400d77b7e39f343f6ce634c40a922
Test run: developer-tests-workbench2: #292

Fixed collection tests

This LGTM, thanks!