Arvados

12881-fix-dnd partial fix, works for adding files in the same repository. Needs to handle drag-and-drop when the file is from a different repository from the target.

• File src/app/core/panels/my-apps-panel/arvados-apps-panel.service.ts:L155 - missing colon
• Tested adding a cwl file into the visual editor of another cwl file, both on the same, and on different repos (using a couple of repos of my own @ 4xphq) and it seems to have worked ok on both cases.
• Adding a cwl file into itself seems to be allowed (it displays an error message, but maybe it would be better to not allow it entirely?)

Lucas Di Pentima wrote:

• File src/app/core/panels/my-apps-panel/arvados-apps-panel.service.ts:L155 - missing colon

Fixed.

• Tested adding a cwl file into the visual editor of another cwl file, both on the same, and on different repos (using a couple of repos of my own @ 4xphq) and it seems to have worked ok on both cases.

This should now report an error.

• Adding a cwl file into itself seems to be allowed (it displays an error message, but maybe it would be better to not allow it entirely?)

This should now report an error.

12881-fix-dnd @ commit:dccb9a63eea9196524a9cb1a53306a69f4b8197f

Oops, try 12881-fix-dnd @ commit:e40623aafba27928a7f7c1624da0c801ed617a24 this updates the commit hash for the js-git dependency.

Drag & drop behavior works as intended on my end, so I just have some questions about readability/code formatting:

• File src/app/workflow-editor/graph-editor/graph-editor/workflow-graph-editor.component.ts
  • Lines 336-349’s indentation is a little off making the code less readable
  • else block at lines 342-346 seems to be superfluous.
  • Line 336: if isLocal is always true, there are at least 3 places where it's being checked for truthfulness that may get simpler.

Other than the above, this LGTM.

Lucas Di Pentima wrote:

Drag & drop behavior works as intended on my end, so I just have some questions about readability/code formatting:

• File src/app/workflow-editor/graph-editor/graph-editor/workflow-graph-editor.component.ts
  • Lines 336-349’s indentation is a little off making the code less readable

Ah yea, there's a tabs vs spaces thing going on there.

• else block at lines 342-346 seems to be superfluous.
• Line 336: if isLocal is always true, there are at least 3 places where it's being checked for truthfulness that may get simpler.

The isLocal flag is meaningful in the original code, which we eventually want to merge with, so removing that code would mainly have the effect of creating future merge conflicts.

Other than the above, this LGTM.

Thanks.