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 b8bead4b0ea..17cd7a11fad 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 88f429a3e69..d197808d00e 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 a5e4463037d..1ae30d331cf 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 8e2203addd9..9b8f4741bd1 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(`${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 4146e0704ad..61c04a0825e 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 64681965b42..0760b8841d9 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(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; }