Copilot commented on code in PR #8456:
URL: https://github.com/apache/texera/pull/8456#discussion_r3998056929
##########
frontend/src/app/dashboard/service/user/download/download.service.ts:
##########
@@ -315,8 +315,13 @@ export class DownloadService {
*/
private retrieveWorkflowItem(id: number, name: string):
Observable<DownloadableItem> {
return this.workflowPersistService.retrieveWorkflow(id).pipe(
- map(({ content }) => {
- const workflowJson = JSON.stringify(content, null, 2);
+ map(({ content, defaultView }) => {
+ // Carry the landing view so a download-then-upload keeps a
form-default workflow opening
+ // as a form. It goes in as one extra top-level key next to the
workflow's own
+ // (operators/links/...); the importer destructures it back out (see
uploadWorkflow), and an
+ // older importer that reads the whole object as content simply
ignores the unknown key.
+ const exported = defaultView === undefined ? content : { ...content,
defaultView };
+ const workflowJson = JSON.stringify(exported, null, 2);
Review Comment:
This only updates dashboard downloads. The canvas's visible “export
workflow” action still serializes `getWorkflowContent()` alone
(`menu.component.ts:630-637`), so exporting there and uploading the file drops
a FORM default. The promised download/upload round-trip should cover that entry
point too, ideally through the same export-shaping helper.
##########
frontend/src/app/workspace/component/power-button/computing-unit-selection.component.ts:
##########
@@ -269,25 +269,66 @@ export class ComputingUnitSelectionComponent implements
OnInit {
if (wid !== this.workflowId) {
this.workflowId = wid;
if (isDefined(this.workflowId) && this.workflowId !==
DEFAULT_WORKFLOW.wid) {
- this.workflowExecutionsService
- .retrieveLatestWorkflowExecution(this.workflowId)
- .pipe(untilDestroyed(this))
- .subscribe({
- next: (latestWorkflowExecution: WorkflowExecutionsEntry) => {
- this.selectComputingUnit(this.workflowId,
latestWorkflowExecution.cuId);
- },
- error: (err: unknown) => {
- const runningUnit = this.allComputingUnits.find(unit =>
unit.status === "Running");
- if (runningUnit) {
- this.selectComputingUnit(this.workflowId,
runningUnit.computingUnit.cuid);
- }
- },
- });
+ this.selectInitialUnit(this.workflowId);
}
}
});
}
+ /**
+ * Pick the unit for a workflow that has just come into view. An explicit
choice remembered for
+ * it is newer than its last run, so it wins -- but only once the unit list
has arrived and still
+ * holds that unit. Deciding on an empty list would either chase a unit that
has since been
+ * terminated (the status service waits for it to appear, forever, and the
fallbacks below never
+ * run) or throw the choice away before the list has loaded. A remembered
unit that is gone is
+ * forgotten, and the fallbacks take over: the last execution's unit, else
any running unit.
+ */
+ private selectInitialUnit(wid: number): void {
+ const remembered = this.recallComputingUnit(wid);
+ if (!isDefined(remembered)) {
+ this.selectFromLastExecution(wid);
+ return;
+ }
+ this.computingUnitStatusService
+ .getAllComputingUnits()
+ .pipe(
+ filter(units => units.length > 0),
+ take(1),
+ untilDestroyed(this)
+ )
+ .subscribe(units => {
+ // The workflow can change while the list is still loading; that later
change made its own
+ // decision, so this one is stale.
+ if (wid !== this.workflowId) {
+ return;
+ }
+ if (units.some(unit => unit.computingUnit.cuid === remembered)) {
+ this.selectComputingUnit(wid, remembered);
+ } else {
+ this.forgetComputingUnit(wid);
+ this.selectFromLastExecution(wid);
+ }
+ });
+ }
+
+ /** The unit the workflow last ran on, else any unit that is running. */
+ private selectFromLastExecution(wid: number): void {
+ this.workflowExecutionsService
+ .retrieveLatestWorkflowExecution(wid)
+ .pipe(untilDestroyed(this))
+ .subscribe({
+ next: (latestWorkflowExecution: WorkflowExecutionsEntry) => {
+ this.selectComputingUnit(wid, latestWorkflowExecution.cuId);
+ },
+ error: () => {
+ const runningUnit = this.allComputingUnits.find(unit => unit.status
=== "Running");
+ if (runningUnit) {
+ this.selectComputingUnit(wid, runningUnit.computingUnit.cuid);
+ }
+ },
Review Comment:
The stale-workflow guard ends before this asynchronous fallback. If the unit
list rejects a remembered unit and workflow B opens while workflow A's
latest-execution request is pending, A's response calls `selectComputingUnit(A,
...)` and reconnects the shared status service to the workflow no longer
displayed. Recheck `wid === this.workflowId` in both callbacks before selecting.
##########
frontend/src/app/workspace/component/menu/menu.component.ts:
##########
@@ -626,6 +637,70 @@ export class MenuComponent implements OnInit, OnDestroy {
this.fileSaverService.saveAs(new Blob([workflowContentJson], { type:
"text/plain;charset=utf-8" }), fileName);
}
+ /**
+ * Open the Form View -- a full page load, not a route: the two views share
root-level
+ * singletons (graph, Yjs shared model), and routing left the old
collaboration client
+ * alive (you appeared as your own coeditor). A fresh document is the clean
handover.
+ */
+ public onClickOpenFormView(): void {
+ const wid = this.workflowActionService.getWorkflowMetadata().wid;
+ if (wid === undefined || this.handingOverToFormView) {
+ return;
+ }
+ // Save first, and hand over only once the save has completed. The
full-page load that
+ // follows unloads this document, and a request still in flight at that
moment is aborted, so
+ // navigating right after firing the save could lose the very edit the
switch is meant to carry
+ // across; the workspace's beforeunload save runs into the same unload and
is no safety net. A
+ // save that fails keeps the user here with the error shown, rather than
leaving with changes
+ // that were never stored. The form's own switch (openRegularCanvas) does
the same.
+ //
+ // Two more things the hand-over must not lose. An autosave already in
flight when the switch
+ // is clicked: WorkflowPersistService sends saves one at a time and in
order, so ours lands after
+ // it and completes after it. And an edit made while our save is out (the
page stays editable
+ // until the load): workflowChanged marks it, and the drain below saves
once more before handing
+ // over rather than letting the full-page load abort that edit's own
debounced autosave.
+ this.handingOverToFormView = true;
+ this.isSaving = true;
+ this.saveThenOpenFormView(wid);
Review Comment:
Read-only workflows still render this view switch, but this path always
attempts `persistWorkflow`; the backend rejects that write, so readers see an
error and can never reach Form View. Since `writeAccess` already reflects
`metadata.readonly`, skip the save and navigate directly when it is false,
matching the Form View's reader-side switch.
This issue also appears on line 668 of the same file.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
