mengw15 commented on code in PR #8516:
URL: https://github.com/apache/texera/pull/8516#discussion_r3994403540
##########
frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts:
##########
@@ -510,11 +545,41 @@ 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 by the execute service once it
ends.
+ */
+ private clampEditability(): void {
+ this.workflowActionService
+ .getWorkflowModificationEnabledStream()
+ .pipe(untilDestroyed(this))
+ .subscribe(enabled => {
+ if (enabled && !this.mayUnlock()) {
Review Comment:
This clamp re-locks an author in edit mode after every run. The unlock and
the state event arrive in a fixed order: `updateWorkflowActionLock` runs before
`executionStateStream.next` (execute-workflow.service.ts:367-380), so when the
unlock emission reaches this subscription, `this.executionState` is still the
running state — `mayUnlock()` returns false and the clamp disables again. The
state event then lands and nothing re-enables, so the live panel and the
view-result command stay locked until the author leaves and re-enters edit
mode. (Reading the run state from ExecuteWorkflowService instead would not
help: its own `currentState` is also updated after the lock call.)
The three new specs pass because the harness emits the state before the
unlock — the reverse of the service's real order.
A small fix that keeps the clamp for the other unlockers: also call
`applyEditability()` in the execution-state subscription, after
`this.executionState = current.state`, so the reducer re-asserts once the state
has settled; and flip the specs' emission order to match the service.
--
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]
