https://dev.arvados.org/https://dev.arvados.org/favicon.ico?15576888422018-01-02T14:45:39ZArvadosArvados - Bug #12881: [Composer] Drag and drop of tools into graph view doesn't workhttps://dev.arvados.org/issues/12881?journal_id=588422018-01-02T14:45:39ZPeter Amstutzpeter.amstutz@curii.com
<ul><li><strong>Status</strong> changed from <i>New</i> to <i>In Progress</i></li></ul> Arvados - Bug #12881: [Composer] Drag and drop of tools into graph view doesn't workhttps://dev.arvados.org/issues/12881?journal_id=588562018-01-02T15:33:23ZPeter Amstutzpeter.amstutz@curii.com
<ul><li><strong>Status</strong> changed from <i>In Progress</i> to <i>New</i></li></ul> Arvados - Bug #12881: [Composer] Drag and drop of tools into graph view doesn't workhttps://dev.arvados.org/issues/12881?journal_id=588702018-01-02T15:35:14ZPeter Amstutzpeter.amstutz@curii.com
<ul><li><strong>Assigned To</strong> set to <i>Peter Amstutz</i></li></ul> Arvados - Bug #12881: [Composer] Drag and drop of tools into graph view doesn't workhttps://dev.arvados.org/issues/12881?journal_id=590442018-01-04T22:14:21ZPeter Amstutzpeter.amstutz@curii.com
<ul><li><strong>Status</strong> changed from <i>New</i> to <i>In Progress</i></li></ul> Arvados - Bug #12881: [Composer] Drag and drop of tools into graph view doesn't workhttps://dev.arvados.org/issues/12881?journal_id=591372018-01-08T14:56:30ZPeter Amstutzpeter.amstutz@curii.com
<ul></ul><p>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.</p> Arvados - Bug #12881: [Composer] Drag and drop of tools into graph view doesn't workhttps://dev.arvados.org/issues/12881?journal_id=593142018-01-11T14:57:23ZLucas Di Pentimalucas.dipentima@curii.com
<ul></ul><ul>
<li>File <code>src/app/core/panels/my-apps-panel/arvados-apps-panel.service.ts:L155</code> - missing colon</li>
<li>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.</li>
<li>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?)</li>
</ul> Arvados - Bug #12881: [Composer] Drag and drop of tools into graph view doesn't workhttps://dev.arvados.org/issues/12881?journal_id=594702018-01-17T19:39:18ZPeter Amstutzpeter.amstutz@curii.com
<ul><li><strong>Target version</strong> changed from <i>2018-01-17 Sprint</i> to <i>2018-01-31 Sprint</i></li></ul> Arvados - Bug #12881: [Composer] Drag and drop of tools into graph view doesn't workhttps://dev.arvados.org/issues/12881?journal_id=595542018-01-22T17:51:59ZPeter Amstutzpeter.amstutz@curii.com
<ul></ul><p>Lucas Di Pentima wrote:</p>
<blockquote>
<ul>
<li>File <code>src/app/core/panels/my-apps-panel/arvados-apps-panel.service.ts:L155</code> - missing colon</li>
</ul>
</blockquote>
<p>Fixed.</p>
<blockquote>
<ul>
<li>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.</li>
</ul>
</blockquote>
<p>This should now report an error.</p>
<blockquote>
<ul>
<li>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?)</li>
</ul>
</blockquote>
<p>This should now report an error.</p>
<p>12881-fix-dnd @ commit:dccb9a63eea9196524a9cb1a53306a69f4b8197f</p> Arvados - Bug #12881: [Composer] Drag and drop of tools into graph view doesn't workhttps://dev.arvados.org/issues/12881?journal_id=595562018-01-22T17:58:12ZPeter Amstutzpeter.amstutz@curii.com
<ul></ul><p>Oops, try 12881-fix-dnd @ commit:e40623aafba27928a7f7c1624da0c801ed617a24 this updates the commit hash for the js-git dependency.</p> Arvados - Bug #12881: [Composer] Drag and drop of tools into graph view doesn't workhttps://dev.arvados.org/issues/12881?journal_id=595742018-01-22T19:12:51ZLucas Di Pentimalucas.dipentima@curii.com
<ul></ul><p>Drag & drop behavior works as intended on my end, so I just have some questions about readability/code formatting:</p>
<ul>
<li>File <code>src/app/workflow-editor/graph-editor/graph-editor/workflow-graph-editor.component.ts</code>
<ul>
<li>Lines 336-349’s indentation is a little off making the code less readable</li>
<li><code>else</code> block at lines 342-346 seems to be superfluous.</li>
<li>Line 336: if <code>isLocal</code> is always true, there are at least 3 places where it's being checked for truthfulness that may get simpler.</li>
</ul></li>
</ul>
<p>Other than the above, this LGTM.</p> Arvados - Bug #12881: [Composer] Drag and drop of tools into graph view doesn't workhttps://dev.arvados.org/issues/12881?journal_id=595822018-01-22T19:34:57ZPeter Amstutzpeter.amstutz@curii.com
<ul></ul><p>Lucas Di Pentima wrote:</p>
<blockquote>
<p>Drag & drop behavior works as intended on my end, so I just have some questions about readability/code formatting:</p>
<ul>
<li>File <code>src/app/workflow-editor/graph-editor/graph-editor/workflow-graph-editor.component.ts</code>
<ul>
<li>Lines 336-349’s indentation is a little off making the code less readable</li>
</ul></li>
</ul>
</blockquote>
<p>Ah yea, there's a tabs vs spaces thing going on there.</p>
<blockquote>
<ul>
<li><code>else</code> block at lines 342-346 seems to be superfluous.</li>
<li>Line 336: if <code>isLocal</code> is always true, there are at least 3 places where it's being checked for truthfulness that may get simpler.</li>
</ul>
</blockquote>
<p>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.</p>
<blockquote>
<p>Other than the above, this LGTM.</p>
</blockquote>
<p>Thanks.</p> Arvados - Bug #12881: [Composer] Drag and drop of tools into graph view doesn't workhttps://dev.arvados.org/issues/12881?journal_id=598002018-02-01T16:54:08ZPeter Amstutzpeter.amstutz@curii.com
<ul><li><strong>Status</strong> changed from <i>In Progress</i> to <i>Resolved</i></li></ul>