This is an automated email from the ASF dual-hosted git repository. github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-8498-0e549fa4e5cc611cf532d9aa8b81de08a3c5fce5 in repository https://gitbox.apache.org/repos/asf/texera.git
commit 3488d37034acb70664c6554dd1bea792ad9590a3 Author: yangzhang75 <[email protected]> AuthorDate: Fri Sep 11 02:30:11 2026 +0000 fix(workflow): a plain save no longer clobbers is_public (#8498) ### What changes were proposed in this PR? Closes #8496. A regression from #8125: since then, every workflow's second autosave (and every one after it) fails with 500, silently on the operator canvas and as "Could not save" on the Form View. `persistWorkflow` wrote `is_public` from the request. The frontend feeds the saved row straight back as its metadata, and that row names the flag `isPublic` while the rest of the frontend calls it `isPublished`, so the very next save went out without the flag, the update wrote NULL into a NOT NULL column, and the request failed. A stale `isPublic: false` on a save could likewise un-publish a published workflow. - Backend: `saveWorkflowFields` now writes name, description and content only. Publishing stays with `/public` and `/private`, `default_view` with `/set-default-view`, and the timestamps are not rewritten, so a save can never clobber a concurrent change to any of them. - Frontend: `WorkflowPersistService.persistWorkflow` no longer sends `isPublic` (the endpoint does not read it, and the value is not reliably known after the first save), and `WorkflowUtilService.parseWorkflowInfo` carries a persist response's `isPublic` over to `isPublished`, so metadata fed back from a save keeps the publish state instead of dropping it. The Form View stack is not blocked by this: #8455 touches none of these files, and #8456 touches `workflow-persist.service.ts` only in `createWorkflow` (adding `defaultView`), a different function; a dry-run merge of the two is clean. ### Any related issues, documentation, discussions? Closes #8496. Found while verifying #8455 on a flag-on instance (parent #8011). ### How was this PR tested? Backend: `WorkflowResourceSpec` gains two tests, a save carrying no flag neither fails nor changes `is_public` after `/public`, and a save carrying `false` does not un-publish; the existing default-view save test was updated to send no flag, as the frontend does. `WorkflowResourceSpec` and `PublishedCopySchemaSpec` pass (87 tests), scalafmt clean. Frontend: the persist spec asserts the save payload carries no `isPublic` and that the response's `isPublic` comes back as `isPublished`; `parseWorkflowInfo` gains tests for the carry-over and for leaving a present `isPublished` alone. Full suite passes (5777), changed lines fully covered, eslint, prettier and the production (AOT) build pass. End to end, against a running stack rebuilt with this change: the exact second-save payload that returned 500 now returns 200 with `is_public` unchanged; a create-through-persist with the new payload inserts with `is_public = false`; and in a headless browser the Form View renames a workflow twice with every `/api/workflow/persist` answering 200, no `isPublic` key in any request body, and no "Could not save". ### Was this PR authored or co-authored using generative AI tooling? Yes. Generated-by: Claude Code (Claude Fable 5.1, Anthropic). Co-authored with Claude, reviewed line by line by the author before submission. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01FVvP3ttj22f9LB4p9u2anY Co-authored-by: Claude Fable 5.1 <[email protected]> Co-authored-by: Meng Wang <[email protected]> --- .../dashboard/user/workflow/WorkflowResource.scala | 12 ++++--- .../dashboard/file/WorkflowResourceSpec.scala | 40 +++++++++++++++++++--- .../workflow-persist.service.spec.ts | 10 ++++-- .../workflow-persist/workflow-persist.service.ts | 5 ++- .../util/workflow-util.service.spec.ts | 15 ++++++++ .../workflow-graph/util/workflow-util.service.ts | 8 +++++ 6 files changed, 77 insertions(+), 13 deletions(-) diff --git a/amber/src/main/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowResource.scala b/amber/src/main/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowResource.scala index b8bead4b0e..17cd7a11fa 100644 --- a/amber/src/main/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowResource.scala +++ b/amber/src/main/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowResource.scala @@ -489,10 +489,13 @@ class WorkflowResource extends LazyLogging { } /** - * Persists a plain save by updating only the fields the client sends - * (name/description/content/is_public). It deliberately leaves `default_view` untouched -- - * that column is owned by /set-default-view alone -- so a save can never clobber a - * concurrent change. Timestamps are likewise not rewritten here. + * Persists a plain save by updating only what a save is: name, description and content. + * `is_public` is not written here. Publishing has its own endpoints (/public, /private), and + * a save payload does not reliably carry the flag: the frontend feeds the saved row straight + * back as its metadata, where the flag has another name, so the very next autosave arrives + * without it. Writing that null violated the column's NOT NULL constraint and every second + * save failed with 500. `default_view` is likewise owned by /set-default-view alone, and the + * timestamps are not rewritten here, so a save can never clobber a concurrent change to either. */ private def saveWorkflowFields(workflow: Workflow): Unit = { context @@ -500,7 +503,6 @@ class WorkflowResource extends LazyLogging { .set(WORKFLOW.NAME, workflow.getName) .set(WORKFLOW.DESCRIPTION, workflow.getDescription) .set(WORKFLOW.CONTENT, workflow.getContent) - .set(WORKFLOW.IS_PUBLIC, workflow.getIsPublic) .where(WORKFLOW.WID.eq(workflow.getWid)) .execute() } diff --git a/amber/src/test/scala/org/apache/texera/web/resource/dashboard/file/WorkflowResourceSpec.scala b/amber/src/test/scala/org/apache/texera/web/resource/dashboard/file/WorkflowResourceSpec.scala index 88f429a3e6..d197808d00 100644 --- a/amber/src/test/scala/org/apache/texera/web/resource/dashboard/file/WorkflowResourceSpec.scala +++ b/amber/src/test/scala/org/apache/texera/web/resource/dashboard/file/WorkflowResourceSpec.scala @@ -1340,9 +1340,9 @@ class WorkflowResourceSpec assert(defaultView(wid) == DefaultViewEnum.CANVAS) } - // A plain save (persistWorkflow) only writes the fields the client sends -- name, - // description, content, is_public -- and never `default_view`, so saving the canvas must - // not reset the default view. The edit payload mirrors what the frontend sends. + // A plain save (persistWorkflow) only writes name, description and content -- never + // `default_view`, so saving the canvas must not reset the default view. The edit payload mirrors + // what the frontend sends. it should "survive a subsequent save of the workflow" in { val wid = persistFreshWorkflow("param_survives_save") workflowResource.setDefaultView(wid, DefaultViewRequest("FORM"), sessionUser1) @@ -1351,7 +1351,6 @@ class WorkflowResourceSpec edit.setWid(wid) edit.setName("param_survives_save_edited") edit.setContent("{\"operators\":[],\"links\":[]}") - edit.setIsPublic(false) workflowResource.persistWorkflow(edit, sessionUser1) assert( @@ -1360,6 +1359,39 @@ class WorkflowResourceSpec ) } + // The frontend feeds the saved row back as its metadata, where the publish flag has another + // name, so the very next autosave arrives with isPublic unset. A save must neither fail on that + // (the column is NOT NULL, and writing the null made every second save a 500) nor rewrite the + // flag: publishing is /public and /private's job alone. + "/persist API" should "neither fail nor change is_public when the save carries no flag" in { + val wid = persistFreshWorkflow("persist_keeps_public") + workflowResource.makePublic(wid, sessionUser1) + + val edit = new Workflow() + edit.setWid(wid) + edit.setName("persist_keeps_public_edited") + edit.setContent("{\"operators\":[],\"links\":[]}") + // isPublic deliberately left null, exactly as the frontend's second save sends it + val saved = workflowResource.persistWorkflow(edit, sessionUser1) + + assert(saved.getName == "persist_keeps_public_edited") + assert(saved.getIsPublic, "a plain save must not touch the publish flag") + } + + it should "not un-publish a workflow when the save says isPublic = false" in { + val wid = persistFreshWorkflow("persist_ignores_flag") + workflowResource.makePublic(wid, sessionUser1) + + val edit = new Workflow() + edit.setWid(wid) + edit.setName("persist_ignores_flag_edited") + edit.setContent("{\"operators\":[],\"links\":[]}") + edit.setIsPublic(false) + val saved = workflowResource.persistWorkflow(edit, sessionUser1) + + assert(saved.getIsPublic, "a stale flag on a save must not un-publish the workflow") + } + // A biologist's path is hub -> clone -> use, so a copy has to stay usable. it should "be inherited by a duplicated workflow" in { val wid = persistFreshWorkflow("param_source") diff --git a/frontend/src/app/common/service/workflow-persist/workflow-persist.service.spec.ts b/frontend/src/app/common/service/workflow-persist/workflow-persist.service.spec.ts index a5e4463037..1ae30d331c 100644 --- a/frontend/src/app/common/service/workflow-persist/workflow-persist.service.spec.ts +++ b/frontend/src/app/common/service/workflow-persist/workflow-persist.service.spec.ts @@ -202,19 +202,23 @@ describe("WorkflowPersistService", () => { const req = httpTestingController.expectOne(`${API}/${WORKFLOW_PERSIST_URL}`); expect(req.request.method).toBe("POST"); + // The publish flag is not part of a save: the endpoint does not read it, and sending a + // stale copy is what used to null the column after the first save. expect(req.request.body).toEqual({ wid: 9, name: "my wf", description: "a description", content: JSON.stringify(validContent), - isPublic: true, }); - req.flush({ wid: 9, name: "my wf", content: '{"operators":[]}' }); + // The saved row comes back with the flag under the backend's name; the response the + // caller sees carries it as isPublished, so metadata fed back from a save stays complete. + req.flush({ wid: 9, name: "my wf", content: '{"operators":[]}', isPublic: true }); // valid workflow -> no error notification, and string content is parsed expect(errorSpy).not.toHaveBeenCalled(); expect(result?.content).toEqual({ operators: [] }); + expect(result?.isPublished).toBe(1); }); it("persistWorkflow notifies the user when the workflow is broken but still POSTs", () => { @@ -235,7 +239,7 @@ describe("WorkflowPersistService", () => { ); const req = httpTestingController.expectOne(`${API}/${WORKFLOW_PERSIST_URL}`); - expect(req.request.body.isPublic).toBe(false); + expect("isPublic" in req.request.body).toBe(false); req.flush({ wid: 1, name: "broken", content: '{"operators":[]}' }); }); diff --git a/frontend/src/app/common/service/workflow-persist/workflow-persist.service.ts b/frontend/src/app/common/service/workflow-persist/workflow-persist.service.ts index 8e2203addd..9b8f4741bd 100644 --- a/frontend/src/app/common/service/workflow-persist/workflow-persist.service.ts +++ b/frontend/src/app/common/service/workflow-persist/workflow-persist.service.ts @@ -75,13 +75,16 @@ export class WorkflowPersistService { ); } + // A save carries name, description and content only. The publish flag is not sent: the + // backend does not read it on this endpoint (publishing goes through /public and /private), + // and it is not reliably known here anyway, since the metadata fed back after a save names + // it differently (see WorkflowUtilService.parseWorkflowInfo). return this.http .post<Workflow>(`${AppSettings.getApiEndpoint()}/${WORKFLOW_PERSIST_URL}`, { wid: workflow.wid, name: workflow.name, description: workflow.description, content: JSON.stringify(workflow.content), - isPublic: workflow.isPublished, }) .pipe( filter((updatedWorkflow: Workflow) => updatedWorkflow != null), diff --git a/frontend/src/app/workspace/service/workflow-graph/util/workflow-util.service.spec.ts b/frontend/src/app/workspace/service/workflow-graph/util/workflow-util.service.spec.ts index 4146e0704a..61c04a0825 100644 --- a/frontend/src/app/workspace/service/workflow-graph/util/workflow-util.service.spec.ts +++ b/frontend/src/app/workspace/service/workflow-graph/util/workflow-util.service.spec.ts @@ -219,6 +219,21 @@ describe("WorkflowUtilService", () => { expect(parsed.content).toBe(content); }); + // The persist endpoint returns the stored row, which names the publish flag isPublic; the rest + // of the frontend knows it as isPublished. Without the carry-over, a save fed back as metadata + // lost the flag, and the next save went out without it. + it("should carry the persist response's isPublic over to isPublished", () => { + const fromPersist = { wid: 1, name: "n", content: "{}", isPublic: true } as unknown as Workflow; + + expect(WorkflowUtilService.parseWorkflowInfo(fromPersist).isPublished).toBe(1); + }); + + it("should leave an isPublished the payload already carries alone", () => { + const fromRetrieve = { wid: 1, name: "n", content: "{}", isPublished: 0, isPublic: true } as unknown as Workflow; + + expect(WorkflowUtilService.parseWorkflowInfo(fromRetrieve).isPublished).toBe(0); + }); + it("should create a fresh comment box at the default position", () => { const commentBox = workflowUtilService.getNewCommentBox(); diff --git a/frontend/src/app/workspace/service/workflow-graph/util/workflow-util.service.ts b/frontend/src/app/workspace/service/workflow-graph/util/workflow-util.service.ts index 64681965b4..0760b8841d 100644 --- a/frontend/src/app/workspace/service/workflow-graph/util/workflow-util.service.ts +++ b/frontend/src/app/workspace/service/workflow-graph/util/workflow-util.service.ts @@ -182,6 +182,14 @@ export class WorkflowUtilService { if (workflow != null && typeof workflow.content === "string") { workflow.content = jsonCast<WorkflowContent>(workflow.content); } + // The persist endpoint answers with the stored row, whose publish flag is named isPublic; + // every other workflow endpoint, and the Workflow type, call it isPublished. Carry it across, + // or the metadata a save feeds back would silently drop the publish state until the next + // full load. + const stored = workflow as Workflow & { isPublic?: boolean }; + if (workflow != null && workflow.isPublished === undefined && stored.isPublic !== undefined) { + workflow.isPublished = Number(stored.isPublic); + } return workflow; }
