Copilot commented on code in PR #8516:
URL: https://github.com/apache/texera/pull/8516#discussion_r3994444272
##########
frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts:
##########
@@ -510,11 +551,43 @@ export class WorkflowFormComponent implements OnInit,
OnDestroy {
}
/**
- * Show the workflow rather than edit it: the graph shape and its properties
are read-only
- * on this page. A later PR's authoring mode makes properties editable with
write access.
+ * Whether this page may hold the graph unlocked: a writer in edit mode, and
no run in flight. The
+ * run rule is the canvas's own (a run locks the graph until it ends), kept
here too so that entering
+ * edit mode mid-run cannot undo it. Every other state is locked, so a
reader, or a writer merely
+ * viewing, cannot change the workflow through this page.
*/
+ private mayUnlock(): boolean {
+ return this.authoring && this.canEdit && !this.isRunning;
+ }
+
+ /** Lock or unlock editing from the current state (see mayUnlock); the one
reducer for both directions. */
private applyEditability(): void {
- this.workflowActionService.disableWorkflowModification();
+ if (this.mayUnlock()) {
+ this.workflowActionService.enableWorkflowModification();
+ } else {
+ this.workflowActionService.disableWorkflowModification();
+ }
+ }
+
+ /**
+ * The lock is a root-level flag with writers that know nothing of this
page: the execute service
+ * unlocks it whenever a run ends (completed, failed, killed, reset), the
computing-unit selector
+ * unlocks it when it finds no run on the chosen unit, and any future caller
may too. Rather than
+ * chase each one, clamp at the one place they all report to: whenever the
flag turns on while this
+ * page must stay locked, turn it off again. In edit mode the canvas rule
then stands unchanged:
+ * locked while a run is in flight, unlocked once it ends -- the execute
service flips the lock
+ * before it emits the new state, so that unlock is clamped here (this page
still sees "running")
+ * and given back by the execution-state handler, which re-applies the rule
with the new state.
+ */
+ private clampEditability(): void {
+ this.workflowActionService
+ .getWorkflowModificationEnabledStream()
+ .pipe(untilDestroyed(this))
+ .subscribe(enabled => {
+ if (enabled && !this.mayUnlock()) {
+ this.workflowActionService.disableWorkflowModification();
Review Comment:
This clamp runs synchronously inside `enableWorkflowModification()`'s
`BehaviorSubject.next(true)`. That method emits before its final
`undoRedoService.enableWorkFlowModification()` call, so the nested disable
emits `false`, then the outer enable resumes and leaves undo/redo enabled;
subscribers registered after this clamp can likewise finish processing the
stale `true` event. After a run or computing-unit change outside edit mode,
keyboard undo and the result toggle can therefore remain active even though the
root flag reads false. Reconcile after the enabling call has completed, or move
this arbitration into `WorkflowActionService`, so all consumers observe one
final locked state.
##########
frontend/src/app/common/type/workflow.ts:
##########
@@ -67,13 +67,16 @@ export interface FormBindingConfig {
};
/** Array order is display order; the author reorders by dragging. */
fields: FormFieldBinding[];
- /** View-result operators whose results are also shown under the workflow
after a run, on top of
- * the final step's result, which always shows. */
- resultOperatorIds: string[];
+ /** Which steps' results show under the workflow after a run, for everyone.
Absent until the author
+ * chooses: then every final (terminal) step shows, as on the canvas. Once
set it is exhaustive:
+ * exactly these steps show, and [] means none. One list, so nothing can
contradict it; the cost is
+ * that a step which becomes final after the author has chosen does not
appear by itself. When
+ * displayed it is kept to steps that still have a result on the canvas. */
+ shownResultIds?: string[];
Review Comment:
This replaces a persisted field without a compatibility path. Workflows
saved by the current main branch contain `formBinding.resultOperatorIds`;
`hydrateFormBinding` assigns that JSON unchanged, while all new reads ignore
the old property and fall back to terminal-only results. Existing curated
intermediate results therefore disappear when opened after this change. Add
legacy hydration/migration that preserves the old semantics (current terminals
plus `resultOperatorIds`) and cover loading an old serialized workflow.
--
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]
