This is an automated email from the ASF dual-hosted git repository.
github-merge-queue[bot] pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/texera.git
The following commit(s) were added to refs/heads/main by this push:
new 3488d37034 fix(workflow): a plain save no longer clobbers is_public
(#8498)
3488d37034 is described below
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;
}
