Copilot commented on code in PR #8456:
URL: https://github.com/apache/texera/pull/8456#discussion_r3999991367
##########
frontend/src/app/workspace/component/menu/menu.component.ts:
##########
@@ -219,6 +223,13 @@ export class MenuComponent implements OnInit, OnDestroy {
}
public ngOnInit(): void {
+ // Marks an edit for the Form View hand-over (see onClickOpenFormView):
set the moment an edit is
+ // reported, before the autosave debounce, cleared when the switch's save
snapshots the workflow.
+ this.workflowActionService
+ .workflowChanged()
+ .pipe(untilDestroyed(this))
+ .subscribe(() => (this.editedSinceSwitchSnapshot = true));
Review Comment:
`workflowChanged()` only covers graph/content events; workflow name and
description edits do not emit it (they update metadata and call
`MenuComponent.persistWorkflow()` directly). If the user edits metadata while
this hand-over save is in flight, that save is queued *after* the switch save,
but `editedSinceSwitchSnapshot` remains false. Completing the switch save then
navigates immediately, unloading the page before the later metadata save
starts/completes and potentially losing the edit. Track metadata-originated
save requests during hand-over or wait for an actual queue-drained signal
before navigating.
##########
frontend/src/app/common/service/workflow-persist/workflow-persist.service.ts:
##########
@@ -301,4 +339,24 @@ export class WorkflowPersistService {
public setDefaultView(wid: number, view: DefaultView): Observable<void> {
return
this.http.put<void>(`${AppSettings.getApiEndpoint()}/${WORKFLOW_SET_DEFAULT_VIEW_URL}/${wid}`,
{ view });
}
+
+ /**
+ * The landing view as the workflow row holds it now, for an export taken
from an open page (the
+ * canvas or the form). The page's own copy dates from its load, while the
default is set from the
+ * dashboard and may have changed since; whichever page the file is saved
from, it should carry the
+ * default of that moment. The page's copy is the fallback when the row
cannot be read, and the
+ * answer outright for a workflow not saved yet.
+ */
+ public currentDefaultView(
+ wid: number | undefined,
+ fallback: DefaultView | undefined
+ ): Observable<DefaultView | undefined> {
+ if (wid === undefined) {
+ return of(fallback);
+ }
Review Comment:
This unsaved-workflow check also misses `DEFAULT_WORKFLOW.wid === 0`.
Consequently exporting a new canvas workflow issues `GET /workflow/0` instead
of returning the fallback immediately; only the resulting error happens to
recover the value, and any global HTTP error handling may still surface the
spurious 404. Handle the default-workflow sentinel alongside `undefined`, as
other workspace services do.
##########
frontend/src/app/workspace/component/menu/menu.component.ts:
##########
@@ -617,14 +628,94 @@ export class MenuComponent implements OnInit, OnDestroy {
}
public onClickExportWorkflow(): void {
- const workflowContent: WorkflowContent =
this.workflowActionService.getWorkflowContent();
- const workflowContentJson = JSON.stringify(workflowContent, null, 2);
- const fileName = this.currentWorkflowName + ".json";
- // Through the injectable wrapper (as the dashboard downloads already do),
so a spec stubs it
- // with TestBed instead of module-mocking the CommonJS file-saver package,
which the unit-test
- // builder cannot hoist reliably.
- this.fileSaverService.saveAs(new Blob([workflowContentJson], { type:
"text/plain;charset=utf-8" }), fileName);
+ // The same shape the dashboard download produces (see exportedWorkflow):
the content plus the
+ // landing view as a sibling key, so a file exported here uploads as a
form-default workflow too.
+ // The content is what is on screen at the click; the landing view is read
from the row now,
+ // not from this page's copy, since it is set from the dashboard and may
have changed since load.
+ const content = this.workflowActionService.getWorkflowContent();
+ const { wid, defaultView } =
this.workflowActionService.getWorkflowMetadata();
+ this.workflowPersistService
+ .currentDefaultView(wid, defaultView)
+ .pipe(untilDestroyed(this))
+ .subscribe(current => {
+ const workflowContentJson = JSON.stringify(exportedWorkflow(content,
current), null, 2);
+ const fileName = this.currentWorkflowName + ".json";
+ // Through the injectable wrapper (as the dashboard downloads already
do), so a spec stubs it
+ // with TestBed instead of module-mocking the CommonJS file-saver
package, which the unit-test
+ // builder cannot hoist reliably.
+ 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;
Review Comment:
The unsaved-workflow guard misses Texera's actual sentinel:
`DEFAULT_WORKFLOW.wid` is `0`, not `undefined`
(`workflow-action.service.ts:56-63`). The canvas can hold that default workflow
(for example after `onClickCreateNewWorkflow()`), so this path persists it as a
newly assigned workflow but still calls `openFormViewPage(0)` using the
captured ID, navigating to a nonexistent `/user/workflow/0/form`. Treat
`DEFAULT_WORKFLOW.wid` as unsaved (and hide/disable the switch), or navigate
with the ID returned by the create-on-persist response.
--
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]
