From f8b02912489645ec493a1811ca0350a34ed69bfd Mon Sep 17 00:00:00 2001 From: Yang Zhang Date: Fri, 21 Aug 2026 23:11:52 -0700 Subject: [PATCH 1/6] feat(workflow): add the published-copy columns A public workflow follows the author's latest content today: every save reaches the Hub immediately. Pinning a version as the public copy needs somewhere to keep that copy, which is what these columns are. `is_public` stays the on/off switch. `published_content` is the pin: NULL means the workflow follows the author's latest, which is what every workflow does today, so the migration changes nothing anyone can see. `published_name` and `published_description` travel with it because a pin has to hold everything on show, and `workflow_version` stores no metadata at all -- only content deltas. `published_version_id` names the version row holding that copy, so the revision panel can mark it and the author can restore it. The copy is materialized rather than replayed from `workflow_version` because those rows are reverse JSON-Patch deltas: serving a pinned workflow would mean folding every newer patch back from the author's current content on each public read, and a computed value is something the fulltext index cannot cover. A CHECK constraint makes "private but pinned" unrepresentable, and a PGroonga index mirrors the latest-content one so public search can match the frozen copy. Adding columns changes the arity of the generated positional constructor, so the three copy-producing paths (clone, duplicate, restore-a-version) now build their POJO with setters -- which is also what stops a later column from silently shifting a null into the wrong field. Part of #7828. --- .../user/workflow/WorkflowResource.scala | 31 ++++--- .../workflow/WorkflowVersionResource.scala | 11 +-- .../workflow/PublishedCopySchemaSpec.scala | 93 +++++++++++++++++++ sql/changelog.xml | 5 + sql/texera_ddl.sql | 31 ++++++- sql/updates/41.sql | 93 +++++++++++++++++++ 6 files changed, 244 insertions(+), 20 deletions(-) create mode 100644 amber/src/test/scala/org/apache/texera/web/resource/dashboard/user/workflow/PublishedCopySchemaSpec.scala create mode 100644 sql/updates/41.sql 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 2d438e8dc75..e7fecc06c07 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 @@ -143,6 +143,21 @@ object WorkflowResource { case class WorkflowIDs(wids: List[Integer], pid: Option[Integer]) + /** + * A workflow POJO for the copy-producing paths (clone, duplicate, restore-a-version). + * + * Built with setters rather than the positional constructor, so that adding a column cannot + * silently shift a null into the wrong field -- as adding the published-copy columns would. + */ + def newUnpublishedWorkflow(name: String, description: String, content: String): Workflow = { + val workflow = new Workflow() + workflow.setName(name) + workflow.setDescription(description) + workflow.setContent(content) + workflow.setIsPublic(false) + workflow + } + private def updateWorkflowField( workflow: Workflow, sessionUser: SessionUser, @@ -504,14 +519,10 @@ class WorkflowResource extends LazyLogging { for (wid <- workflowIDs.wids) { val oldWorkflow: Workflow = workflowDao.fetchOneByWid(wid) val newWorkflow = createWorkflow( - new Workflow( - null, + newUnpublishedWorkflow( oldWorkflow.getName + "_copy", oldWorkflow.getDescription, - assignNewOperatorIds(oldWorkflow.getContent), - null, - null, - false + assignNewOperatorIds(oldWorkflow.getContent) ), sessionUser ) @@ -554,14 +565,10 @@ class WorkflowResource extends LazyLogging { } val oldWorkflow: Workflow = workflowDao.fetchOneByWid(wid) val newWorkflow: DashboardWorkflow = createWorkflow( - new Workflow( - null, + newUnpublishedWorkflow( oldWorkflow.getName + "_clone", oldWorkflow.getDescription, - assignNewOperatorIds(oldWorkflow.getContent), - null, - null, - false + assignNewOperatorIds(oldWorkflow.getContent) ), sessionUser ) diff --git a/amber/src/main/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowVersionResource.scala b/amber/src/main/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowVersionResource.scala index e0664b7c1d4..de1f880a834 100644 --- a/amber/src/main/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowVersionResource.scala +++ b/amber/src/main/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowVersionResource.scala @@ -30,7 +30,8 @@ import org.apache.texera.dao.jooq.generated.tables.daos.{WorkflowDao, WorkflowVe import org.apache.texera.dao.jooq.generated.tables.pojos.{Workflow, WorkflowVersion} import org.apache.texera.web.resource.dashboard.user.workflow.WorkflowResource.{ DashboardWorkflow, - assignNewOperatorIds + assignNewOperatorIds, + newUnpublishedWorkflow } import org.apache.texera.web.resource.dashboard.user.workflow.WorkflowVersionResource._ import org.jooq.DSLContext @@ -428,14 +429,10 @@ class WorkflowVersionResource { val newWorkflow: DashboardWorkflow = try { workflowResource.createWorkflow( - new Workflow( - null, + newUnpublishedWorkflow( newWorkflowName, workflowVersion.getDescription, - assignNewOperatorIds(workflowVersion.getContent), - null, - null, - false + assignNewOperatorIds(workflowVersion.getContent) ), sessionUser ) diff --git a/amber/src/test/scala/org/apache/texera/web/resource/dashboard/user/workflow/PublishedCopySchemaSpec.scala b/amber/src/test/scala/org/apache/texera/web/resource/dashboard/user/workflow/PublishedCopySchemaSpec.scala new file mode 100644 index 00000000000..08a87869c56 --- /dev/null +++ b/amber/src/test/scala/org/apache/texera/web/resource/dashboard/user/workflow/PublishedCopySchemaSpec.scala @@ -0,0 +1,93 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.texera.web.resource.dashboard.user.workflow + +import org.apache.texera.dao.MockTexeraDB +import org.apache.texera.dao.jooq.generated.tables.daos.WorkflowDao +import org.apache.texera.dao.jooq.generated.tables.pojos.Workflow +import org.jooq.exception.DataAccessException +import org.scalatest.BeforeAndAfterAll +import org.scalatest.flatspec.AnyFlatSpec +import org.scalatest.matchers.should.Matchers + +/** + * The columns a pinned public copy lives in, and the constraint that keeps them honest. + * + * Nothing writes them yet: this covers what the migration alone guarantees -- that a workflow + * public today keeps behaving as it does, and that a private workflow can never carry a pin. + */ +class PublishedCopySchemaSpec + extends AnyFlatSpec + with Matchers + with BeforeAndAfterAll + with MockTexeraDB { + + private var workflowDao: WorkflowDao = _ + + override protected def beforeAll(): Unit = { + initializeDBAndReplaceDSLContext() + workflowDao = new WorkflowDao(getDSLContext.configuration()) + } + + /** A workflow as it exists before anything pins it: content only, nothing frozen. */ + private def insertWorkflow(name: String, isPublic: Boolean): Workflow = { + val workflow = new Workflow() + workflow.setName(name) + workflow.setDescription("a workflow") + workflow.setContent("""{"operators":[]}""") + workflow.setIsPublic(isPublic) + workflowDao.insert(workflow) + workflowDao.fetchOneByWid(workflow.getWid) + } + + behavior of "the published-copy columns" + + it should "leave every workflow following the author's latest" in { + // The migration adds columns and no backfill, so a workflow that was public before it ran shows + // exactly what it showed: nothing is frozen, which is the state the rest of the feature calls + // "following". + val stored = insertWorkflow("migration_changes_nothing", isPublic = true) + + stored.getPublishedContent shouldBe null + stored.getPublishedName shouldBe null + stored.getPublishedDescription shouldBe null + stored.getPublishedVersionId shouldBe null + } + + it should "let a public workflow carry a frozen copy" in { + val stored = insertWorkflow("public_may_be_pinned", isPublic = true) + stored.setPublishedContent("""{"operators":[]}""") + stored.setPublishedName("frozen name") + stored.setPublishedDescription("frozen description") + + workflowDao.update(stored) + + workflowDao.fetchOneByWid(stored.getWid).getPublishedName shouldBe "frozen name" + } + + it should "refuse a private workflow that carries a frozen copy" in { + // A pin only means something while the workflow is public. The database rejects the other case, + // so no code path can leave one behind -- unpublishing has to clear the copy. + val stored = insertWorkflow("private_cannot_be_pinned", isPublic = false) + stored.setPublishedContent("""{"operators":[]}""") + + a[DataAccessException] should be thrownBy workflowDao.update(stored) + } +} diff --git a/sql/changelog.xml b/sql/changelog.xml index 3debbad196e..2f4a389bf6c 100644 --- a/sql/changelog.xml +++ b/sql/changelog.xml @@ -114,6 +114,11 @@ + + + + + +
+ + + + +
+ + + + The public is on an older version. + The public sees your current version, pinned. + The public sees your latest, updated on every save. + +
+ + + +
+
+
+ Pinned to {{ publishStatus.pinnedVersionTime | date: publicationTimeFormat }} +
+
Your later edits stay private
+
+ +
+ +
+ Frozen as you keep editing. Restore it any time from the version panel, where it is marked + Currently public. +
+
+
+
+
diff --git a/frontend/src/app/dashboard/component/user/share-access/share-access.component.scss b/frontend/src/app/dashboard/component/user/share-access/share-access.component.scss index 1a74945e159..401747d706b 100644 --- a/frontend/src/app/dashboard/component/user/share-access/share-access.component.scss +++ b/frontend/src/app/dashboard/component/user/share-access/share-access.component.scss @@ -36,25 +36,163 @@ line-height: 1; } -.access-button-group { - display: flex; - flex-direction: row; - justify-content: space-evenly; - align-items: center; -} - .access-button { display: flex; + flex: 1; flex-direction: column; align-items: center; justify-content: space-evenly; height: 150px; - width: 300px; padding: 20px; - margin-bottom: 24px; border-radius: 10px; } +// Laid out with a gap rather than space-evenly so the tiles span the full width of the dialog body. +// space-evenly left them inset, which the full-width publish-state strip below could not line up with. +.access-button-group { + display: flex; + gap: 24px; + align-items: stretch; + margin-bottom: 24px; +} + +.access-button-group:has(+ .publish-anchor) { + margin-bottom: 8px; +} + +// The publish panel. Nested under one root so nothing here reaches the Private/Public tiles, the +// invite form or the access list around it. +.publish-anchor { + margin-bottom: 24px; + + // Sized to the panel's own scale; everything else about the switch is the component's. + // ::ng-deep because these are the control's own nodes: our styles are scoped to this component, + // and its inner DOM belongs to ng-zorro. + .publish-seg ::ng-deep { + font-size: 13.5px; + // Between antd's default and a pill: square enough to sit with the tiles above, round enough to + // echo the card below, whose corners are 12px. + border-radius: 12px; + + .ant-segmented-item, + .ant-segmented-thumb { + border-radius: 9px; + } + + .ant-segmented-item-label { + padding: 3px 16px; + } + + // Quicker than the default: the thumb only confirms a choice the author already made, and a + // slide they have to wait out reads as the app thinking rather than as an answer. + .ant-segmented-thumb { + transition-duration: 0.16s; + } + + // The side in force takes the colour of the state it puts the workflow in -- green while the + // public follows along, amber once a version is held. Without it the only mark is a white tile, + // which is easy to miss at a glance. + .ant-segmented-item-label[aria-selected="true"] { + font-weight: 600; + color: #12a05e; + } + } + + // Amber once a version is held, matching the dot and the sentence below. + &:not([data-state="follow"]) .publish-seg ::ng-deep .ant-segmented-item-label[aria-selected="true"] { + color: #e0951c; + } + + .publish-status { + display: flex; + align-items: center; + gap: 12px; + padding-top: 16px; + } + + .publish-say { + flex: 1; + min-width: 0; + font-size: 14px; + line-height: 1.45; + color: #5f666f; + + b { + font-weight: 580; + color: #1c1f26; + } + } + + // The one state that leaves the author something to decide gets a card: which version is public, + // what it costs them, and the act that ends it -- in that order, left to right. + .publish-card { + display: flex; + align-items: center; + gap: 12px; + margin-top: 12px; + padding: 12px 12px 12px 16px; + background: #f6f7f9; + border: 1px solid #e6e9ee; + border-radius: 12px; + } + + .publish-card-text { + flex: 1; + min-width: 0; + } + + .publish-card-title { + font-size: 13px; + font-weight: 580; + color: #1c1f26; + // Digits of equal width, so a date that ticks does not shuffle the words around it. + font-variant-numeric: tabular-nums; + } + + .publish-card-meta { + margin-top: 2px; + font-size: 11.5px; + line-height: 1.3; + color: #9299a2; + } + + .publish-card-action { + flex: none; + padding: 9px 14px; + border: none; + border-radius: 8px; + background: #ddeafe; + color: #2f74f0; + font: inherit; + font-size: 13px; + font-weight: 580; + white-space: nowrap; + cursor: pointer; + transition: 0.14s; + + &:hover:not([disabled]) { + background: #d0e1fd; + } + + &[disabled] { + cursor: default; + opacity: 0.55; + } + } + + .publish-hint { + margin-top: 12px; + font-size: 12.5px; + line-height: 1.5; + color: #9299a2; + + b { + font-weight: 540; + color: #5f666f; + } + } +} + .button-icon { font-size: 40px; } diff --git a/frontend/src/app/dashboard/component/user/share-access/share-access.component.spec.ts b/frontend/src/app/dashboard/component/user/share-access/share-access.component.spec.ts index ea3a5af6c9e..a9d8e14b02f 100644 --- a/frontend/src/app/dashboard/component/user/share-access/share-access.component.spec.ts +++ b/frontend/src/app/dashboard/component/user/share-access/share-access.component.spec.ts @@ -17,11 +17,11 @@ * under the License. */ -import { TestBed } from "@angular/core/testing"; +import { ComponentFixture, TestBed } from "@angular/core/testing"; import { HttpClientTestingModule } from "@angular/common/http/testing"; import { NoopAnimationsModule } from "@angular/platform-browser/animations"; import { HttpErrorResponse } from "@angular/common/http"; -import { of, throwError } from "rxjs"; +import { of, Subject, throwError } from "rxjs"; import { NZ_MODAL_DATA, NzModalRef, NzModalService } from "ng-zorro-antd/modal"; import { NzMessageService } from "ng-zorro-antd/message"; @@ -58,14 +58,26 @@ describe("ShareAccessComponent", () => { let workflowPersistSpy: { getWorkflowIsPublished: ReturnType; updateWorkflowIsPublished: ReturnType; + getPublishStatus: ReturnType; + getWorkflowPersistedStream: ReturnType; + isWorkflowPersistEnabled: ReturnType; + persistWorkflow: ReturnType; + pinLatestVersion: ReturnType; + unpinVersion: ReturnType; }; let datasetServiceSpy: { getDataset: ReturnType; updateDatasetPublicity: ReturnType; }; - let workflowActionSpy: { setWorkflowIsPublished: ReturnType }; + let workflowActionSpy: { + setWorkflowIsPublished: ReturnType; + getWorkflow: ReturnType; + }; let userServiceCurrentEmail: string | undefined; let capturedModalConfigs: any[]; + let lastFixture: ComponentFixture | undefined; + /** Stands in for the editor's autosave landing, which is what the line has to follow. */ + let persisted: Subject; function setupComponent(opts: SetupOptions = {}): ShareAccessComponent { const { type = "workflow", id = 1, inWorkspace = false, currentEmail = "me@example.com" } = opts; @@ -94,11 +106,37 @@ describe("ShareAccessComponent", () => { }); const fixture = TestBed.createComponent(ShareAccessComponent); fixture.detectChanges(); + lastFixture = fixture; return fixture.componentInstance; } + /** Everything the publish panel renders, for the assertions about what the author actually reads. */ + function publishLineText(): string { + lastFixture!.detectChanges(); + return (lastFixture!.nativeElement.querySelector(".publish-anchor")?.textContent ?? "").replace(/\s+/g, " ").trim(); + } + + /** + * The two sides of the switch. Which one is highlighted is the control's own business -- it paints + * the selection from an animation frame, which this environment never runs -- so these assert the + * labels and leave the state itself to `publishState`. + */ + function segments(): string[] { + lastFixture!.detectChanges(); + return Array.from(lastFixture!.nativeElement.querySelectorAll(".ant-segmented-item-label")).map((b: any) => + b.textContent.replace(/\s+/g, " ").trim() + ); + } + + /** The card under the line, which only a pin holding edits back puts on screen. */ + function publishNoteText(): string { + lastFixture!.detectChanges(); + return (lastFixture!.nativeElement.querySelector(".publish-card")?.textContent ?? "").replace(/\s+/g, " ").trim(); + } + beforeEach(() => { TestBed.resetTestingModule(); + persisted = new Subject(); capturedModalConfigs = []; gmailSpy = { sendEmail: vi.fn() }; accessServiceSpy = { @@ -116,15 +154,25 @@ describe("ShareAccessComponent", () => { return { close: vi.fn() }; }), }; + const following = { isPublished: true, isPinned: false, hasUnpublishedChanges: false }; workflowPersistSpy = { getWorkflowIsPublished: vi.fn().mockReturnValue(of("Private")), updateWorkflowIsPublished: vi.fn().mockReturnValue(of(null)), + getPublishStatus: vi.fn().mockReturnValue(of(following)), + getWorkflowPersistedStream: vi.fn().mockReturnValue(persisted.asObservable()), + isWorkflowPersistEnabled: vi.fn().mockReturnValue(true), + persistWorkflow: vi.fn().mockReturnValue(of({ wid: 3 })), + pinLatestVersion: vi.fn().mockReturnValue(of({ ...following, isPinned: true })), + unpinVersion: vi.fn().mockReturnValue(of(following)), }; datasetServiceSpy = { getDataset: vi.fn().mockReturnValue(of({ dataset: { isPublic: false } })), updateDatasetPublicity: vi.fn().mockReturnValue(of(null)), }; - workflowActionSpy = { setWorkflowIsPublished: vi.fn() }; + workflowActionSpy = { + setWorkflowIsPublished: vi.fn(), + getWorkflow: vi.fn().mockReturnValue({ wid: 3, name: "w", content: { operators: [], links: [] } }), + }; }); function getFooterButton(config: any, label: string): { onClick: () => void } { @@ -170,6 +218,344 @@ describe("ShareAccessComponent", () => { }); }); + describe("publish state line", () => { + // The owner of the workflow, so hasWriteAccess is true. + const asOwner = { currentEmail: "owner@example.com" }; + + it("does not fetch publish status for an unpublished workflow", () => { + workflowPersistSpy.getWorkflowIsPublished.mockReturnValue(of("Private")); + const c = setupComponent({ type: "workflow", id: 3, ...asOwner }); + expect(workflowPersistSpy.getPublishStatus).not.toHaveBeenCalled(); + expect(c.publishStatus).toBeUndefined(); + }); + + it("reports unpublished changes on a pinned workflow", () => { + workflowPersistSpy.getWorkflowIsPublished.mockReturnValue(of("Public")); + workflowPersistSpy.getPublishStatus.mockReturnValue( + of({ isPublished: true, isPinned: true, hasUnpublishedChanges: true }) + ); + const c = setupComponent({ type: "workflow", id: 3, ...asOwner }); + expect(workflowPersistSpy.getPublishStatus).toHaveBeenCalledWith(3); + expect(c.publishStatus?.hasUnpublishedChanges).toBe(true); + }); + + it("does not fetch publish status for a viewer who cannot publish", () => { + workflowPersistSpy.getWorkflowIsPublished.mockReturnValue(of("Public")); + accessServiceSpy.getAccessList.mockReturnValue( + of([{ email: "reader@example.com", name: "R", privilege: Privilege.READ }]) + ); + const c = setupComponent({ type: "workflow", id: 3, currentEmail: "reader@example.com" }); + expect(workflowPersistSpy.getPublishStatus).not.toHaveBeenCalled(); + expect(c.publishStatus).toBeUndefined(); + }); + + it("clears the pending state after moving the pin forward", () => { + workflowPersistSpy.getWorkflowIsPublished.mockReturnValue(of("Public")); + workflowPersistSpy.getPublishStatus.mockReturnValue( + of({ isPublished: true, isPinned: true, hasUnpublishedChanges: true }) + ); + workflowPersistSpy.pinLatestVersion.mockReturnValue( + of({ isPublished: true, isPinned: true, hasUnpublishedChanges: false }) + ); + const c = setupComponent({ type: "workflow", id: 3, ...asOwner }); + + c.choosePublicCopy(true, true); + + expect(workflowPersistSpy.pinLatestVersion).toHaveBeenCalledWith(3); + expect(c.publishStatus?.hasUnpublishedChanges).toBe(false); + expect(c.isPinning).toBe(false); + }); + + it("goes back to following when the pin is dropped", () => { + workflowPersistSpy.getWorkflowIsPublished.mockReturnValue(of("Public")); + workflowPersistSpy.getPublishStatus.mockReturnValue( + of({ isPublished: true, isPinned: true, hasUnpublishedChanges: true }) + ); + workflowPersistSpy.unpinVersion.mockReturnValue( + of({ isPublished: true, isPinned: false, hasUnpublishedChanges: false }) + ); + const c = setupComponent({ type: "workflow", id: 3, ...asOwner }); + + c.choosePublicCopy(false); + + expect(workflowPersistSpy.unpinVersion).toHaveBeenCalledWith(3); + expect(c.publishStatus?.isPinned).toBe(false); + // Nothing is held back once the public follows the latest. + expect(c.publishStatus?.hasUnpublishedChanges).toBe(false); + expect(c.isPinning).toBe(false); + }); + + it("names the pinned version by its date once the author has moved on", () => { + // The date is how the same version is named in the revision panel, which is where the author + // goes to restore it. Naming it any other way would leave them matching two descriptions. + workflowPersistSpy.getWorkflowIsPublished.mockReturnValue(of("Public")); + workflowPersistSpy.getPublishStatus.mockReturnValue( + of({ + isPublished: true, + isPinned: true, + hasUnpublishedChanges: true, + pinnedVersionTime: Date.parse("2026-08-12T23:59:00"), + }) + ); + setupComponent({ type: "workflow", id: 3, ...asOwner }); + + expect(publishLineText()).toContain("Pinned to Aug 12, 23:59:00"); + expect(publishLineText()).toContain("Your later edits stay private"); + }); + + it("says the pinned copy is the current one rather than dating it", () => { + // A date here would invite the author to work out whether it is still what they have; saying + // so is the answer to the question they would be asking. + workflowPersistSpy.getWorkflowIsPublished.mockReturnValue(of("Public")); + workflowPersistSpy.getPublishStatus.mockReturnValue( + of({ + isPublished: true, + isPinned: true, + hasUnpublishedChanges: false, + pinnedVersionTime: Date.parse("2026-08-12T23:59:00"), + }) + ); + setupComponent({ type: "workflow", id: 3, ...asOwner }); + + expect(publishLineText()).toContain("your current version"); + expect(publishLineText()).not.toContain("Aug 12"); + }); + + it("prints the pinned time to the second, as the revision panel does", () => { + // The line names the version the revision panel marks, so the two must print it identically -- + // one format, never varied, is what guarantees that. + workflowPersistSpy.getWorkflowIsPublished.mockReturnValue(of("Public")); + workflowPersistSpy.getPublishStatus.mockReturnValue( + of({ + isPublished: true, + isPinned: true, + hasUnpublishedChanges: true, + pinnedVersionTime: Date.parse("2026-08-12T23:58:51"), + }) + ); + setupComponent({ type: "workflow", id: 3, ...asOwner }); + + expect(publishLineText()).toContain("Aug 12, 23:58:51"); + }); + + it("says the same thing about both states, with only the answer changing", () => { + workflowPersistSpy.getWorkflowIsPublished.mockReturnValue(of("Public")); + workflowPersistSpy.getPublishStatus.mockReturnValue( + of({ isPublished: true, isPinned: false, hasUnpublishedChanges: false }) + ); + setupComponent({ type: "workflow", id: 3, ...asOwner }); + + expect(publishLineText()).toContain("The public sees your latest"); + expect(segments()).toEqual(["Follow latest", "Pinned"]); + }); + + it("drops the line when the workflow is unpublished", () => { + workflowPersistSpy.getWorkflowIsPublished.mockReturnValue(of("Public")); + const c = setupComponent({ type: "workflow", id: 3, ...asOwner }); + expect(c.publishStatus).toBeDefined(); + + c.unpublishWorkflow(); + + expect(c.isPublic).toBe(false); + expect(c.publishStatus).toBeUndefined(); + }); + + it("hides the notice when the status request fails", () => { + workflowPersistSpy.getWorkflowIsPublished.mockReturnValue(of("Public")); + workflowPersistSpy.getPublishStatus.mockReturnValue(throwError(() => new Error("boom"))); + const c = setupComponent({ type: "workflow", id: 3, ...asOwner }); + expect(c.publishStatus).toBeUndefined(); + }); + + it("surfaces a failed pin and stops the spinner", () => { + workflowPersistSpy.getWorkflowIsPublished.mockReturnValue(of("Public")); + workflowPersistSpy.getPublishStatus.mockReturnValue( + of({ isPublished: true, isPinned: true, hasUnpublishedChanges: true }) + ); + workflowPersistSpy.pinLatestVersion.mockReturnValue( + throwError(() => new HttpErrorResponse({ error: { message: "nope" }, status: 500 })) + ); + const c = setupComponent({ type: "workflow", id: 3, ...asOwner }); + + c.choosePublicCopy(true, true); + + expect(notificationSpy.error).toHaveBeenCalledWith("nope"); + expect(c.isPinning).toBe(false); + // The pending state stands, so the author can try again. + expect(c.publishStatus?.hasUnpublishedChanges).toBe(true); + }); + + it("never asks for publish status on a dataset", () => { + const c = setupComponent({ type: "dataset", id: 12, ...asOwner }); + expect(workflowPersistSpy.getPublishStatus).not.toHaveBeenCalled(); + expect(c.publishStatus).toBeUndefined(); + }); + + it("fetches the status after publishing from private", () => { + workflowPersistSpy.getWorkflowIsPublished.mockReturnValue(of("Private")); + const c = setupComponent({ type: "workflow", id: 3, ...asOwner }); + expect(workflowPersistSpy.getPublishStatus).not.toHaveBeenCalled(); + + c.publishWorkflow(); + + expect(workflowPersistSpy.updateWorkflowIsPublished).toHaveBeenCalledWith(3, true); + expect(workflowPersistSpy.getPublishStatus).toHaveBeenCalledWith(3); + expect(c.publishStatus).toBeDefined(); + }); + + it("offers both choices, with the state itself naming the one in force", () => { + workflowPersistSpy.getWorkflowIsPublished.mockReturnValue(of("Public")); + workflowPersistSpy.getPublishStatus.mockReturnValue( + of({ isPublished: true, isPinned: true, hasUnpublishedChanges: false }) + ); + const c = setupComponent({ type: "workflow", id: 3, ...asOwner }); + + expect(segments()).toEqual(["Follow latest", "Pinned"]); + expect(c.publishState).toBe("pinned"); + }); + + it("does nothing when the side already in force is picked again", () => { + // Re-picking "Pinned" would otherwise publish whatever has been edited since -- which is the + // card's job, and the card says what it would publish. + workflowPersistSpy.getWorkflowIsPublished.mockReturnValue(of("Public")); + workflowPersistSpy.getPublishStatus.mockReturnValue( + of({ isPublished: true, isPinned: true, hasUnpublishedChanges: true }) + ); + const c = setupComponent({ type: "workflow", id: 3, ...asOwner }); + + c.choosePublicCopy(true); + + expect(workflowPersistSpy.pinLatestVersion).not.toHaveBeenCalled(); + expect(c.publishStatus?.hasUnpublishedChanges).toBe(true); + }); + + it("does nothing when following is picked while already following", () => { + workflowPersistSpy.getWorkflowIsPublished.mockReturnValue(of("Public")); + workflowPersistSpy.getPublishStatus.mockReturnValue( + of({ isPublished: true, isPinned: false, hasUnpublishedChanges: false }) + ); + const c = setupComponent({ type: "workflow", id: 3, ...asOwner }); + + c.choosePublicCopy(false); + + expect(workflowPersistSpy.unpinVersion).not.toHaveBeenCalled(); + }); + + it("names the pinned version and the way out while it is holding edits back", () => { + // The card is the answer to "my edits are not public": which version is out there, what it + // costs, and the act that ends it. In every other state there is nothing to decide. + workflowPersistSpy.getWorkflowIsPublished.mockReturnValue(of("Public")); + workflowPersistSpy.getPublishStatus.mockReturnValue( + of({ + isPublished: true, + isPinned: true, + hasUnpublishedChanges: true, + pinnedVersionTime: Date.parse("2026-08-12T23:59:00"), + }) + ); + setupComponent({ type: "workflow", id: 3, ...asOwner }); + + expect(publishNoteText()).toContain("Pinned to Aug 12, 23:59:00"); + expect(publishNoteText()).toContain("Your later edits stay private"); + expect(publishNoteText()).toContain("Update to current"); + }); + + it("points at the version panel once the pinned copy is what the author has", () => { + // The recovery path when they do move on, named where they are already looking. + workflowPersistSpy.getWorkflowIsPublished.mockReturnValue(of("Public")); + workflowPersistSpy.getPublishStatus.mockReturnValue( + of({ isPublished: true, isPinned: true, hasUnpublishedChanges: false }) + ); + setupComponent({ type: "workflow", id: 3, ...asOwner }); + + expect(publishLineText()).toContain("Currently public"); + }); + + it("saves what the editor is holding before asking about it", () => { + // The editor saves a few seconds after the last edit, so a dialog opened inside that window + // would otherwise report on the canvas as it was before those edits. + workflowPersistSpy.getWorkflowIsPublished.mockReturnValue(of("Public")); + workflowPersistSpy.getPublishStatus.mockReturnValue( + of({ isPublished: true, isPinned: true, hasUnpublishedChanges: false }) + ); + setupComponent({ type: "workflow", id: 3, inWorkspace: true, ...asOwner }); + + expect(workflowPersistSpy.persistWorkflow).toHaveBeenCalled(); + }); + + it("does not write anything back while a version is being previewed", () => { + // Previewing turns persistence off; saving here would write the preview back as the author's + // own work. + workflowPersistSpy.getWorkflowIsPublished.mockReturnValue(of("Public")); + workflowPersistSpy.isWorkflowPersistEnabled.mockReturnValue(false); + setupComponent({ type: "workflow", id: 3, inWorkspace: true, ...asOwner }); + + expect(workflowPersistSpy.persistWorkflow).not.toHaveBeenCalled(); + }); + + it("has nothing to flush when opened outside the workspace", () => { + workflowPersistSpy.getWorkflowIsPublished.mockReturnValue(of("Public")); + setupComponent({ type: "workflow", id: 3, ...asOwner }); + + expect(workflowPersistSpy.persistWorkflow).not.toHaveBeenCalled(); + }); + + it("re-reads the state when a save lands", () => { + // The editor saves on a debounce, so the answer the dialog fetched when it opened describes a + // workflow the author may already have moved past. Nothing else tells it -- the canvas changing + // is not the save landing, and until the save lands the server still answers with the old copy. + workflowPersistSpy.getWorkflowIsPublished.mockReturnValue(of("Public")); + workflowPersistSpy.getPublishStatus.mockReturnValue( + of({ isPublished: true, isPinned: true, hasUnpublishedChanges: false }) + ); + setupComponent({ type: "workflow", id: 3, ...asOwner }); + expect(publishNoteText()).toBe(""); + + workflowPersistSpy.getPublishStatus.mockReturnValue( + of({ isPublished: true, isPinned: true, hasUnpublishedChanges: true }) + ); + persisted.next({ wid: 3 }); + + expect(workflowPersistSpy.getPublishStatus).toHaveBeenCalledTimes(2); + expect(publishNoteText()).toContain("Update to current"); + }); + + it("says nothing extra when the pinned copy is what the author has", () => { + workflowPersistSpy.getWorkflowIsPublished.mockReturnValue(of("Public")); + workflowPersistSpy.getPublishStatus.mockReturnValue( + of({ isPublished: true, isPinned: true, hasUnpublishedChanges: false }) + ); + setupComponent({ type: "workflow", id: 3, ...asOwner }); + + expect(publishNoteText()).toBe(""); + }); + + it("publishes the edits a pin was holding back, from the note", () => { + workflowPersistSpy.getWorkflowIsPublished.mockReturnValue(of("Public")); + workflowPersistSpy.getPublishStatus.mockReturnValue( + of({ isPublished: true, isPinned: true, hasUnpublishedChanges: true }) + ); + workflowPersistSpy.pinLatestVersion.mockReturnValue( + of({ isPublished: true, isPinned: true, hasUnpublishedChanges: false }) + ); + const c = setupComponent({ type: "workflow", id: 3, ...asOwner }); + + lastFixture!.nativeElement.querySelector(".publish-card-action").click(); + + expect(workflowPersistSpy.pinLatestVersion).toHaveBeenCalledWith(3); + expect(c.publishStatus?.hasUnpublishedChanges).toBe(false); + // Answered, so the question goes away. + expect(publishNoteText()).toBe(""); + }); + + it("says nothing about the copies before the status arrives", () => { + workflowPersistSpy.getWorkflowIsPublished.mockReturnValue(of("Private")); + const c = setupComponent({ type: "workflow", id: 3, ...asOwner }); + expect(c.publishStatus).toBeUndefined(); + expect(publishLineText()).toBe(""); + }); + }); + describe("handleInputConfirm", () => { it("splits input on whitespace, commas, and semicolons into emailTags", () => { const c = setupComponent(); diff --git a/frontend/src/app/dashboard/component/user/share-access/share-access.component.ts b/frontend/src/app/dashboard/component/user/share-access/share-access.component.ts index 81e3739d88b..bfb52673c96 100644 --- a/frontend/src/app/dashboard/component/user/share-access/share-access.component.ts +++ b/frontend/src/app/dashboard/component/user/share-access/share-access.component.ts @@ -27,12 +27,16 @@ import { GmailService } from "../../../../common/service/gmail/gmail.service"; import { NZ_MODAL_DATA, NzModalRef, NzModalService } from "ng-zorro-antd/modal"; import { NotificationService } from "../../../../common/service/notification/notification.service"; import { HttpErrorResponse } from "@angular/common/http"; +import { forkJoin } from "rxjs"; import { USER_DATASET, USER_PROJECT, USER_WORKFLOW } from "../../../../app-routing.constant"; import { NzMessageService } from "ng-zorro-antd/message"; import { DatasetService } from "../../../service/user/dataset/dataset.service"; -import { WorkflowPersistService } from "src/app/common/service/workflow-persist/workflow-persist.service"; +import { + WorkflowPersistService, + WorkflowPublishStatus, +} from "src/app/common/service/workflow-persist/workflow-persist.service"; import { WorkflowActionService } from "src/app/workspace/service/workflow-graph/model/workflow-action.service"; -import { NgIf, NgFor } from "@angular/common"; +import { NgIf, NgFor, NgSwitch, NgSwitchCase, NgSwitchDefault, DatePipe } from "@angular/common"; import { NzSpaceCompactItemDirective } from "ng-zorro-antd/space"; import { NzButtonComponent } from "ng-zorro-antd/button"; import { NzWaveDirective } from "ng-zorro-antd/core/wave"; @@ -45,6 +49,8 @@ import { NzInputDirective } from "ng-zorro-antd/input"; import { NzAutocompleteTriggerDirective, NzAutocompleteComponent } from "ng-zorro-antd/auto-complete"; import { NzTagComponent } from "ng-zorro-antd/tag"; import { NzTooltipDirective } from "ng-zorro-antd/tooltip"; +import { NzSegmentedComponent } from "ng-zorro-antd/segmented"; +import { NzBadgeComponent } from "ng-zorro-antd/badge"; @UntilDestroy() @Component({ @@ -70,8 +76,14 @@ import { NzTooltipDirective } from "ng-zorro-antd/tooltip"; NzAutocompleteTriggerDirective, NzAutocompleteComponent, NgFor, + NgSwitch, + NgSwitchCase, + NgSwitchDefault, + DatePipe, NzTagComponent, NzTooltipDirective, + NzSegmentedComponent, + NzBadgeComponent, ], }) export class ShareAccessComponent implements OnInit, OnDestroy { @@ -88,6 +100,16 @@ export class ShareAccessComponent implements OnInit, OnDestroy { public emailTags: string[] = []; currentEmail: string | undefined = ""; isPublic: boolean | null = null; + /** Undefined until fetched, and for anyone who cannot publish (the endpoint needs write access). */ + publishStatus?: WorkflowPublishStatus; + isPinning = false; + /** The two sides of the switch; the control only takes strings or numbers, so 1 means pinned. */ + readonly publicCopyOptions = [ + { label: "Follow latest", value: 0 }, + { label: "Pinned", value: 1 }, + ]; + /** To the second, always: the revision panel names the same version that way. */ + readonly publicationTimeFormat = "MMM d, HH:mm:ss"; private shouldRefresh = false; @Output() refresh = new EventEmitter(); @@ -123,24 +145,41 @@ export class ShareAccessComponent implements OnInit, OnDestroy { } ngOnInit(): void { - this.accessService - .getAccessList(this.type, this.id) - .pipe(untilDestroyed(this)) - .subscribe(access => (this.accessList = access)); - this.accessService - .getOwner(this.type, this.id) - .pipe(untilDestroyed(this)) - .subscribe(name => { - this.owner = name; - }); + const accessList$ = this.accessService.getAccessList(this.type, this.id); + const owner$ = this.accessService.getOwner(this.type, this.id); + if (this.type === "workflow") { - this.workflowPersistService - .getWorkflowIsPublished(this.id) + // Joined rather than subscribed separately because the publish state depends on all three: + // hasWriteAccess is only answerable once the owner and the access list have landed, and the + // strip only applies to a published workflow. + forkJoin([accessList$, owner$, this.workflowPersistService.getWorkflowIsPublished(this.id)]) .pipe(untilDestroyed(this)) - .subscribe(dashboardWorkflow => { - this.isPublic = dashboardWorkflow === "Public"; + .subscribe(([accessList, owner, workflowType]) => { + this.accessList = accessList; + this.owner = owner; + this.isPublic = workflowType === "Public"; + // Asked of the saved copy, so make sure it is the copy the author is looking at. The + // editor saves a few seconds after the last edit, and this dialog is modal -- so the only + // way to reach it with unsaved edits is to open it inside that window, which is exactly + // when an author comes to check what the public can see. Flushing here answers about the + // canvas in front of them instead of about the canvas as of a few seconds ago. + this.flushPendingEdits(); + this.refreshPublishStatus(); }); - } else if (this.type === "dataset") { + // The panel describes the saved copy and the editor saves on a debounce, so re-read when a + // save lands -- until then the server would still answer with the copy before it. + this.workflowPersistService + .getWorkflowPersistedStream() + .pipe(untilDestroyed(this)) + .subscribe(() => this.refreshPublishStatus()); + return; + } + + accessList$.pipe(untilDestroyed(this)).subscribe(access => (this.accessList = access)); + owner$.pipe(untilDestroyed(this)).subscribe(name => { + this.owner = name; + }); + if (this.type === "dataset") { this.datasetService .getDataset(this.id) .pipe(untilDestroyed(this)) @@ -156,6 +195,76 @@ export class ShareAccessComponent implements OnInit, OnDestroy { } } + /** + * Saves whatever the editor is holding, so the panel describes the canvas in front of the author. + * Only inside the workspace, and only while saving is on: previewing an old version turns + * persistence off, and writing the preview back would be a save nobody asked for. + */ + private flushPendingEdits(): void { + if (!this.inWorkspace || !this.isPublic || !this.hasWriteAccess) { + return; + } + if (!this.workflowPersistService.isWorkflowPersistEnabled()) { + return; + } + this.workflowPersistService + .persistWorkflow(this.workflowActionService.getWorkflow()) + .pipe(untilDestroyed(this)) + .subscribe({ error: () => {} }); + } + + /** Only for a published workflow the user can publish -- the endpoint requires write access. */ + private refreshPublishStatus(): void { + if (this.type !== "workflow" || !this.isPublic || !this.hasWriteAccess) { + this.publishStatus = undefined; + return; + } + this.workflowPersistService + .getPublishStatus(this.id) + .pipe(untilDestroyed(this)) + .subscribe({ + next: status => (this.publishStatus = status), + error: () => (this.publishStatus = undefined), + }); + } + + /** Named as a state, so the switch, the sentence and the dot cannot disagree about which is in force. */ + get publishState(): "follow" | "pinned" | "behind" { + if (!this.publishStatus?.isPinned) { + return "follow"; + } + return this.publishStatus.hasUnpublishedChanges ? "behind" : "pinned"; + } + + /** + * Changes what the public sees, leaving the author's working copy alone. Picking the side already + * in force does nothing, so a stray click cannot silently publish edits made since the pin; + * `republish` is how the card asks for exactly that. + */ + public choosePublicCopy(pinned: boolean, republish = false): void { + if (!this.publishStatus || (pinned === this.publishStatus.isPinned && !republish)) { + return; + } + this.isPinning = true; + const change = pinned + ? { request: this.workflowPersistService.pinLatestVersion(this.id), done: "The public now sees this version" } + : { request: this.workflowPersistService.unpinVersion(this.id), done: "The public now follows your latest" }; + change.request + .pipe(untilDestroyed(this)) + .subscribe({ + next: status => { + this.publishStatus = status; + this.notificationService.success(change.done); + }, + error: (error: unknown) => { + if (error instanceof HttpErrorResponse) { + this.notificationService.error(error.error.message); + } + }, + }) + .add(() => (this.isPinning = false)); + } + public handleInputConfirm(event?: Event): void { if (event) { event.preventDefault(); @@ -344,7 +453,11 @@ export class ShareAccessComponent implements OnInit, OnDestroy { if (!this.isPublic) { const modal: NzModalRef = this.modalService.create({ nzTitle: "Notice", - nzContent: `Publishing your ${this.type} would grant all Texera users read access to your ${this.type} along with the right to clone your work.`, + nzContent: + `Publishing your ${this.type} would grant all Texera users read access to your ${this.type} along with the right to clone your work.` + + (this.type === "workflow" + ? " The public will follow your latest version as you save it, unless you pin one." + : ""), nzFooter: [ { label: "Cancel", @@ -409,6 +522,7 @@ export class ShareAccessComponent implements OnInit, OnDestroy { .subscribe({ next: () => { this.isPublic = true; + this.refreshPublishStatus(); this.notificationService.success("Workflow published successfully"); }, error: (error: unknown) => { @@ -428,6 +542,7 @@ export class ShareAccessComponent implements OnInit, OnDestroy { .subscribe({ next: () => { this.isPublic = false; + this.publishStatus = undefined; this.notificationService.success("Workflow unpublished successfully"); }, error: (error: unknown) => {