Copilot commented on code in PR #8516:
URL: https://github.com/apache/texera/pull/8516#discussion_r3994144070
##########
frontend/src/app/common/type/workflow.ts:
##########
@@ -68,8 +68,13 @@ 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. */
+ * the final steps' results, which show by default. */
resultOperatorIds: string[];
+ /** Final (terminal) steps whose result the author turned off. A final step
shows by default, so a
+ * step that becomes final later shows without being listed anywhere; only
an explicit "off" is
+ * stored. Absent until the author turns one off (and on every config
written before the field
+ * existed), which means none turned off. */
+ hiddenResultOperatorIds?: string[];
Review Comment:
This new field is not included in
`WorkflowActionService.isFormBindingNonEmpty`
(`workflow-action.service.ts:773-774`). For a workflow that never had
`formBinding`, turning off a final result can leave `fields`,
`resultOperatorIds`, and `instruction` empty, so `getWorkflowContent()` omits
the entire config and the hidden result is lost on save/reload. Include a
non-empty `hiddenResultOperatorIds` in that persistence predicate and cover the
hidden-only round trip.
##########
frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts:
##########
@@ -539,39 +577,91 @@ export class WorkflowFormComponent implements OnInit,
OnDestroy {
return ["TEXTAREA", "SELECT"].includes(active.tagName) ||
active.isContentEditable;
}
+ /**
+ * The steps a result can come from. The engine compiles only the enabled
operators, so a disabled
+ * step produces nothing however it is flagged: not offered in the picker,
not shown as a result.
+ */
+ private enabledOperators(): OperatorPredicate[] {
+ return this.workflowActionService
+ .getTexeraGraph()
+ .getAllOperators()
+ .filter(op => !(op.isDisabled ?? false));
+ }
+
+ /**
+ * The picker's candidates: every final step (on by default, the engine
always materialises it) and
+ * every intermediate step with view-result ("the eye") on the canvas, plus
any already-chosen step so
+ * a pick never vanishes from its own picker. Reuse the one terminal rule
(terminalOperatorIds) rather
+ * than a second copy. Disabled steps are not offered at all, chosen or not:
they are left out of the
+ * run, so featuring one could never show anyone anything. The shown flag
mirrors shownResultIds, so
+ * it reflects the viewer's own choice when they have made one and the
author's default otherwise.
+ */
+ private rebuildResultChoices(): void {
+ const config = this.formBindingService.getConfig();
+ const viewed =
this.workflowActionService.getTexeraGraph().getOperatorsToViewResult();
+ const chosen = new Set(config.resultOperatorIds);
+ const terminals = new Set(this.terminalOperatorIds());
+ const shown = new Set(this.shownResultIds);
+ this.resultChoices = this.enabledOperators()
+ .filter(op => terminals.has(op.operatorID) || viewed.has(op.operatorID)
|| chosen.has(op.operatorID))
Review Comment:
A reader is also given saved `resultOperatorIds` whose canvas eye has since
been turned off. Such a choice is always `shown: false`, and clicking it cannot
turn it on: `onToggleResult` adds the ID, but `refreshShownResults` immediately
filters it out because no result is materialized. Keep stale saved choices only
in authoring mode, where the author can remove them; readers should see only
terminal or currently viewed operators.
##########
frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts:
##########
@@ -510,11 +541,18 @@ 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.
+ * Lock or unlock editing: the graph and its properties are read-only unless
a writer is in edit
+ * mode, which is the only state that unlocks them (see toggleAuthoring).
*/
private applyEditability(): void {
- this.workflowActionService.disableWorkflowModification();
+ // Edit mode with write access is the only state that makes the operator
properties (and the
+ // embedded canvas) modifiable here; every other state locks them, so a
reader -- or a writer
+ // just viewing -- cannot change the workflow through this page.
+ if (this.authoring && this.canEdit) {
+ this.workflowActionService.enableWorkflowModification();
+ } else {
+ this.workflowActionService.disableWorkflowModification();
+ }
Review Comment:
This does not remain the only enablement path:
`ExecuteWorkflowService.updateWorkflowActionLock()` calls
`enableWorkflowModification()` whenever a run completes, fails, is killed, or
is reset (`execute-workflow.service.ts:385-400`). For a writable workflow
viewed outside authoring mode, finishing a Form View run therefore flips the
global flag back on; the locked preview then exposes the context-menu
view-result command (which intentionally is not gated by `structureLocked`) and
that command mutates/autosaves the shared graph. Reapply the Form View's
authoring lock after terminal execution-state transitions so a writer merely
viewing still cannot modify the 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]
