mengw15 commented on issue #8011: URL: https://github.com/apache/texera/issues/8011#issuecomment-5458547497
Read through the series (and the `formview-mvp` branch) ahead of reviewing the PRs. Three design-level points, raised here once rather than piecemeal on the slices: **1. agent-service will erase the form definition on save.** `agent-service` keeps its own copy of `WorkflowContent` (`agent-service/src/types/workflow.ts`) and rebuilds the content object from that allowlist before POSTing `/workflow/persist` (`workflow-state.ts`, `getWorkflowContent()`). Any agent edit to a parameterized workflow drops `parameterization`, while `is_parameterized` stays on — the form becomes an empty shell. The same copy already lacks `settings.executionMode`, so an agent save silently resets the execution mode today. The plan probably needs an agent-service slice (or fold it into formview-pr3). **2. The read-only path needs an explicit decision.** For a user with READ access, the MVP form is fully editable: the input cards are standalone form groups not covered by the modification lock, so typed values go into the shared-editing doc (co-editors see them live), every autosave is then rejected by `/workflow/persist` with 403 and surfaces the "Could not save" toast on each debounce, and Run still works because execution is gated on computing-unit access rather than workflow access. This path is reachable from the normal UI: the dashboard entry-link rewrite and the form page's redirect both check `formViewEnabled && isParameterized` but not `readonly`, so a workflow shared read-only (or any public one, since `hasReadAccess` is true for those) opens straight into it. Worth deciding once at the design level — redirect read-only users to the canvas like the flag-off case, render the inputs read-only (view and run with the stored values), or scope the form to write-access user s for the first release. **3. Question, not blocking: why an `is_parameterized` column instead of deriving from content?** The single-workflow paths don't need it — `retrieveWorkflow` returns the content, and the form page already redirects based on the loaded workflow. The column seems load-bearing only for the dashboard listing affordances (icon, entry-link rewrite, list-page toggle), since listings don't select `content`. If that is the rationale it's a reasonable trade — worth recording it in formview-pr2 together with the cost it imposes on `/persist` (the extra fetch-before-update needed so a plain save doesn't reset the flag). -- 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]
