diff --git a/amber/src/main/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowPublishService.scala b/amber/src/main/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowPublishService.scala index 62d15fde230..61483f25600 100644 --- a/amber/src/main/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowPublishService.scala +++ b/amber/src/main/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowPublishService.scala @@ -22,7 +22,7 @@ package org.apache.texera.web.resource.dashboard.user.workflow import com.typesafe.scalalogging.LazyLogging import org.apache.texera.amber.util.JSONUtils.objectMapper import org.apache.texera.dao.SqlServer -import org.apache.texera.dao.jooq.generated.Tables.WORKFLOW +import org.apache.texera.dao.jooq.generated.Tables.{WORKFLOW, WORKFLOW_VERSION} import org.apache.texera.dao.jooq.generated.enums.DefaultViewEnum import org.apache.texera.dao.jooq.generated.tables.daos.WorkflowDao import org.apache.texera.dao.jooq.generated.tables.pojos.Workflow @@ -30,6 +30,8 @@ import org.jooq.{Condition, DSLContext} import javax.ws.rs.{BadRequestException, NotFoundException} import scala.jdk.CollectionConverters.CollectionHasAsScala + +import java.sql.Timestamp import scala.util.Try /** @@ -54,10 +56,14 @@ object WorkflowPublishService extends LazyLogging { * What the share dialog asks about: whether the workflow is public, whether a version is pinned, * and whether that pin is holding edits back -- the last is true when pinning again would publish * something, and always false while following. + * + * @param pinnedVersionTime the date naming the pinned version in both the dialog and the + * revision panel. */ case class PublishStatus( isPublished: Boolean, isPinned: Boolean, + pinnedVersionTime: Option[Timestamp], hasUnpublishedChanges: Boolean ) @@ -112,11 +118,14 @@ object WorkflowPublishService extends LazyLogging { * two statements, so a `/private` landing between them would otherwise be undone here, putting a * workflow the author had just taken down back on show under a frozen copy. * - * @return how many rows it matched -- zero for a workflow that is missing or not published. + * @return how many rows it matched -- zero for a workflow that is missing or not published. * + * `versionId` names the row in the revision history that replays to this copy, so the history can + * mark which version is the published one. */ - private def writePin(wid: Integer): Int = - context + private def writePin(ctx: DSLContext, wid: Integer, versionId: Integer): Unit = + ctx .update(WORKFLOW) + .set(WORKFLOW.PUBLISHED_VERSION_ID, versionId) .set(WORKFLOW.PUBLISHED_CONTENT, WORKFLOW.CONTENT) .set(WORKFLOW.PUBLISHED_NAME, WORKFLOW.NAME) .set(WORKFLOW.PUBLISHED_DESCRIPTION, WORKFLOW.DESCRIPTION) @@ -129,9 +138,6 @@ object WorkflowPublishService extends LazyLogging { * row only with every frozen column set on a public workflow, or with every one of them NULL, so * clearing them one at a time -- or clearing them after `is_public` -- would be rejected. * - * `published_version_id` is named by that constraint as well but is not touched here, for the - * same reason [[writePin]] does not set it: nothing writes it yet, so it is NULL on every row. - * * Dropping a pin is offered on a published workflow only, so that condition travels in the same * statement: a `/private` landing between the guard and here cannot be answered with "unpinned, * following latest" about a workflow that is now down. Unpublishing carries no such condition -- @@ -142,6 +148,7 @@ object WorkflowPublishService extends LazyLogging { private def clearPin(wid: Integer, alsoUnpublish: Boolean = false): Int = { val cleared = context .update(WORKFLOW) + .set(WORKFLOW.PUBLISHED_VERSION_ID, null.asInstanceOf[Integer]) .set(WORKFLOW.PUBLISHED_CONTENT, null.asInstanceOf[String]) .set(WORKFLOW.PUBLISHED_NAME, null.asInstanceOf[String]) .set(WORKFLOW.PUBLISHED_DESCRIPTION, null.asInstanceOf[String]) @@ -161,9 +168,35 @@ object WorkflowPublishService extends LazyLogging { * matching means one of two things, and the caller is told which. */ def pinLatest(wid: Integer): PublishStatus = { - if (writePin(wid) == 0) { - requireWorkflow(wid) // 404s if it is gone - throw new BadRequestException(s"Workflow $wid is not published") + context.transaction { txConfig => + val ctx = org.jooq.impl.DSL.using(txConfig) + // Locked, not merely read: two pins racing would both read before either wrote, and both + // insert an anchor. The lock is also what holds the publication check below: a `/private` + // arriving after it has to wait for this transaction rather than slipping under it. + val workflow = ctx + .selectFrom(WORKFLOW) + .where(WORKFLOW.WID.eq(wid)) + .forUpdate() + .fetchOneInto(classOf[Workflow]) + if (workflow == null) { + throw new NotFoundException(s"Workflow $wid not found") + } + // Before the anchor, not after: a pin that cannot land must not leave a version row behind. + if (!workflow.getIsPublic) { + throw new BadRequestException(s"Workflow $wid is not published") + } + + // An anchor in the revision history for the copy being pinned: its delta is the identity patch, + // so replaying this row returns what was published however many edits pile up later. An + // existing version row cannot stand in, since one replays to the content as it was *before* + // the change it records. Pinning again unchanged reuses the anchor rather than adding a twin. + val anchorVid = Option(workflow.getPublishedVersionId) + .filter(_ => + Option(workflow.getPublishedContent).exists(sameContent(_, workflow.getContent)) + ) + .getOrElse(WorkflowVersionResource.insertNewVersion(wid, ctx = ctx).getVid) + + writePin(ctx, wid, anchorVid) } logger.info(s"Workflow $wid pinned to its latest content") statusOf(wid) @@ -193,6 +226,22 @@ object WorkflowPublishService extends LazyLogging { logger.info(s"Workflow $wid unpublished") } + /** Read from the version row, so the dialog and the revision panel print one date rather than two. */ + private def pinnedVersionTimeOf(versionId: Integer): Option[Timestamp] = + Option( + context + .select(WORKFLOW_VERSION.CREATION_TIME) + .from(WORKFLOW_VERSION) + .where(WORKFLOW_VERSION.VID.eq(versionId)) + .fetchOneInto(classOf[Timestamp]) + ) + + /** The date a public viewer should see: that of the version on show, not of an edit they cannot. */ + def publicModifiedTime(workflow: Workflow): Timestamp = + Option(workflow.getPublishedVersionId) + .flatMap(pinnedVersionTimeOf) + .getOrElse(workflow.getLastModifiedTime) + /** Whether a version is pinned, and whether it is holding edits back. */ def statusOf(wid: Integer): PublishStatus = { val workflow = requireWorkflow(wid) @@ -200,6 +249,7 @@ object WorkflowPublishService extends LazyLogging { PublishStatus( isPublished = workflow.getIsPublic, isPinned = pinned, + pinnedVersionTime = Option(workflow.getPublishedVersionId).flatMap(pinnedVersionTimeOf), // Literally "what the public sees is not what you have": whatever [[publicCopyOf]] freezes is // what this compares, on values rather than version ids, so an edit and its undo cancel out. hasUnpublishedChanges = differs(publicCopyOf(workflow), workingCopyOf(workflow)) 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 c0f4fc4c3ca..66f9426976c 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 @@ -720,6 +720,10 @@ class WorkflowResource extends LazyLogging { ) } + /** + * Publishes the workflow. The public follows the author's latest content until a version is + * pinned; see [[pinLatest]]. + */ @PUT @RolesAllowed(Array("REGULAR", "ADMIN")) @Path("/public/{wid}") @@ -956,14 +960,16 @@ class WorkflowResource extends LazyLogging { } // Name and description come from the public copy for the same reason as the content: a pin has // to hold everything on show. - val publicCopy = WorkflowPublishService.publicCopyOf(workflow.into(classOf[Workflow])) + val stored = workflow.into(classOf[Workflow]) + val publicCopy = WorkflowPublishService.publicCopyOf(stored) WorkflowWithPrivilege( publicCopy.name, publicCopy.description, workflow.getWid, publicCopy.content, workflow.getCreationTime, - workflow.getLastModifiedTime, + // Dated by the version on show, not by the author's most recent private edit. + WorkflowPublishService.publicModifiedTime(stored), workflow.getIsPublic, readonly = true, // The view freezes with the copy: the form's definition lives inside the content. 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 c218a1d4ef8..faefabb6393 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 @@ -127,11 +127,15 @@ object WorkflowVersionResource { * * @param wid */ - def insertNewVersion(wid: Integer, content: String = "[]"): WorkflowVersion = { + def insertNewVersion( + wid: Integer, + content: String = "[]", + ctx: DSLContext = context + ): WorkflowVersion = { val workflowVersion = new WorkflowVersion() workflowVersion.setContent(content) workflowVersion.setWid(wid) - workflowVersionDao.insert(workflowVersion) + new WorkflowVersionDao(ctx.configuration).insert(workflowVersion) workflowVersion } @@ -195,39 +199,45 @@ object WorkflowVersionResource { * @return */ private def encodeVersionImportance( - currentVersions: List[WorkflowVersion] + currentVersions: List[WorkflowVersion], + publicVersionId: Option[Integer] ): List[VersionEntry] = { var impEncodedVersions: List[VersionEntry] = List() + // A pin's anchor is not a version the author made: it carries the identity patch and appears the + // moment they pin. It is always shown, so they can find and restore what the public has, but it + // must not start the aggregation window -- the save it was pinned from is seconds older, and + // would otherwise be folded into it and disappear from the panel. + def isAnchor(version: WorkflowVersion): Boolean = publicVersionId.contains(version.getVid) + val lastVersion = currentVersions.head - var lastVersionTime = lastVersion.getCreationTime + var lastVersionTime: Option[Timestamp] = + if (isAnchor(lastVersion)) None else Some(lastVersion.getCreationTime) impEncodedVersions = impEncodedVersions :+ VersionEntry( lastVersion.getVid, lastVersion.getCreationTime, lastVersion.getContent, - true - ) // the first (latest) - // version is important even if it is positional + true, + isAnchor(lastVersion) + ) // the first (latest) version is important even if it is positional var versionImportance: Boolean = true for (version <- currentVersions.tail) { - if ( - isWithinTimeLimit( - lastVersionTime, - version.getCreationTime - ) - ) { + if (isAnchor(version)) { + versionImportance = true + } else if (lastVersionTime.exists(isWithinTimeLimit(_, version.getCreationTime))) { versionImportance = false } // try reducing unnecessary check of positional versions // because parsing the Json string is expensive else { - lastVersionTime = version.getCreationTime + lastVersionTime = Some(version.getCreationTime) versionImportance = isVersionImportant(version.getContent) } impEncodedVersions = impEncodedVersions :+ VersionEntry( version.getVid, version.getCreationTime, version.getContent, - versionImportance + versionImportance, + isAnchor(version) ) } impEncodedVersions @@ -326,7 +336,10 @@ object WorkflowVersionResource { vId: Integer, creationTime: Timestamp, content: String, - importance: Boolean + importance: Boolean, + // True for the one version the Hub is serving right now, so the author can tell at a glance + // where the public copy sits relative to what they are editing. + isCurrentlyPublic: Boolean ) } @@ -360,7 +373,8 @@ class WorkflowVersionResource { .orderBy(WORKFLOW_VERSION.CREATION_TIME.desc()) .fetchInto(classOf[WorkflowVersion]) .asScala - .toList + .toList, + Option(workflowDao.fetchOneByWid(wid)).flatMap(w => Option(w.getPublishedVersionId)) ) } } diff --git a/amber/src/test/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowPublishSpec.scala b/amber/src/test/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowPublishSpec.scala index 23b3a5bafe5..9ba7cbcb0a8 100644 --- a/amber/src/test/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowPublishSpec.scala +++ b/amber/src/test/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowPublishSpec.scala @@ -48,7 +48,8 @@ import javax.ws.rs.{BadRequestException, ForbiddenException, NotFoundException} /** * Covers the publish state a workflow can be in -- following the author's latest content, as * publishing has always done, or holding a pinned copy of the version the author froze -- and the - * read paths that decide which of the two a caller is served, listings and search among them. + * read paths that decide which of the two a caller is served, listings and search among them, and + * the anchor a pin leaves in the revision history. */ class WorkflowPublishSpec extends AnyFlatSpec @@ -144,14 +145,17 @@ class WorkflowPublishSpec private val aWorkflowUpdate = """(?is)\s*update\s+(?:\S*\.)?["`\[]?workflow["`\]]?\s+set\b.*""".r - private def interleaving(interleaved: () => Unit)(act: => Unit): Unit = { + private def interleaving( + interleaved: () => Unit, + at: String => Boolean = aWorkflowUpdate.matches + )(act: => Unit): Unit = { var pending = true val configuration = getDSLContext.configuration().asInstanceOf[DefaultConfiguration] val previousListeners = configuration.executeListenerProviders() configuration.set(new DefaultExecuteListenerProvider(new ExecuteListener { override def executeStart(ctx: ExecuteContext): Unit = { - val sql = Option(ctx.sql()).getOrElse("") - if (pending && aWorkflowUpdate.matches(sql)) { + val sql = Option(ctx.sql()).getOrElse("").toLowerCase + if (pending && at(sql)) { pending = false interleaved() } @@ -160,8 +164,9 @@ class WorkflowPublishSpec try act finally configuration.set(previousListeners: _*) withClue("nothing was interleaved, so this proves nothing: ") { pending shouldBe false } - } /** Clone records the caller's IP, and that is the only thing it wants from the request. - */ + } + + /** Clone records the caller's IP, and that is the only thing it wants from the request. */ private def fakeRequest(): HttpServletRequest = Proxy .newProxyInstance( @@ -330,10 +335,14 @@ class WorkflowPublishSpec // A pin that read the row and wrote what it had read would freeze the version before a save // landing in that window -- and the author, who had just pinned, would be told they have // unpublished changes. Each column is copied from its own row instead, so there is no window. + // + // Interleaved on the way in rather than mid-transaction: pinning now locks the row first, so a + // save arriving after that waits for the pin to finish -- which is the point of the lock. This + // is the last moment a save can still get in front of it. val wid = createWorkflow("pin_takes_the_row_as_it_stands") workflowResource.makePublic(wid, ownerSession) - interleaving(() => edit(wid, editedContent)) { + interleaving(() => edit(wid, editedContent), at = _.contains("for update")) { workflowResource.pinLatest(wid, ownerSession) } @@ -345,11 +354,14 @@ class WorkflowPublishSpec // The "is it published?" check and the pin are two statements. An unpublish landing between // them used to be undone by the pin's own `is_public = true`, putting a workflow the author had // just taken down back on the Hub -- and pinned, so the copy they removed is what the public - // reads. + // reads. Interleaved on the way in: once the pin has the row's lock, an unpublish waits for it. val wid = createWorkflow("pin_cannot_republish") publishPinned(wid) - interleaving(() => workflowResource.makePrivate(wid, ownerSession)) { + interleaving( + () => workflowResource.makePrivate(wid, ownerSession), + at = _.contains("for update") + ) { a[BadRequestException] should be thrownBy workflowResource.pinLatest(wid, ownerSession) } @@ -1251,4 +1263,103 @@ class WorkflowPublishSpec edit(following, editedContent) driftOf(following) shouldBe false } + + behavior of "the revision history" + + it should "leave an anchor in the revision history the author can identify" in { + val wid = createWorkflow("publish_leaves_anchor") + publishPinned(wid) + + val marked = versionResource + .retrieveVersionsOfWorkflow(wid, ownerSession) + .filter(_.isCurrentlyPublic) + marked should have size 1 + marked.head.vId shouldBe workflowDao.fetchOneByWid(wid).getPublishedVersionId + // Both collapsing rules would otherwise hide it, and a hidden anchor is useless to the author. + marked.head.importance shouldBe true + // The revision panel prints this row's creation time and the share dialog prints the pinned + // version's date. They are one row, so they have to be one value: the same version dated a + // second apart in two panels reads as two versions. + marked.head.creationTime shouldBe statusOf(wid).pinnedVersionTime.get + } + + it should "keep the save a pin was taken from visible in the revision panel" in { + // The anchor lands seconds after the save it freezes, and the panel folds versions that land + // close together into the newest of them. Letting the anchor be that newest one would hide the + // author's own save behind a row they never made. + val wid = createWorkflow("pin_does_not_swallow_the_save") + edit(wid, editedContent) + publishPinned(wid) + + val shown = versionResource + .retrieveVersionsOfWorkflow(wid, ownerSession) + .filter(_.importance) + // The anchor, and the save it was taken from. + shown.count(_.isCurrentlyPublic) shouldBe 1 + shown.size should be >= 2 + } + + it should "move the mark when the pin moves, leaving the old anchor unmarked" in { + val wid = createWorkflow("only_the_live_one_is_marked") + publishPinned(wid) + val first = workflowDao.fetchOneByWid(wid).getPublishedVersionId + edit(wid, editedContent) + workflowResource.pinLatest(wid, ownerSession) + val second = workflowDao.fetchOneByWid(wid).getPublishedVersionId + + second should not be first + versionResource + .retrieveVersionsOfWorkflow(wid, ownerSession) + .filter(_.isCurrentlyPublic) + .map(_.vId) shouldBe List(second) + } + + it should "replay the anchor to exactly what was published, however much is edited after" in { + val wid = createWorkflow("anchor_replays_to_published") + publishPinned(wid) + val anchor = workflowDao.fetchOneByWid(wid).getPublishedVersionId + + edit(wid, editedContent) + edit(wid, """{"operators":[],"note":"later_still"}""") + + versionResource + .retrieveWorkflowVersion(wid, anchor, ownerSession) + .getContent shouldBe publishedContent + } + + it should "not record a second anchor when pinning again without having edited" in { + // Otherwise a repeated click would litter the author's revision panel with rows that all replay + // to the same graph. + val wid = createWorkflow("republish_without_edits") + val before = versionResource.retrieveVersionsOfWorkflow(wid, ownerSession).size + publishPinned(wid) + val anchor = workflowDao.fetchOneByWid(wid).getPublishedVersionId + workflowResource.pinLatest(wid, ownerSession) + workflowResource.pinLatest(wid, ownerSession) + + workflowDao.fetchOneByWid(wid).getPublishedVersionId shouldBe anchor + versionResource.retrieveVersionsOfWorkflow(wid, ownerSession) should have size (before + 1) + } + + it should "keep the version the pin was taken from after the pin is dropped" in { + val wid = createWorkflow("unpin_keeps_history") + publishPinned(wid) + + workflowResource.unpin(wid, ownerSession) + + versionResource.retrieveVersionsOfWorkflow(wid, ownerSession) should not be empty + workflowDao.fetchOneByWid(wid).getPublishedVersionId shouldBe null + } + + it should "date the public view by the version on show" in { + // A public viewer is told when what they are looking at was published, not when the author last + // touched a copy they cannot see. + val wid = createWorkflow("public_view_is_dated_by_the_pin") + publishPinned(wid) + val pinnedAt = statusOf(wid).pinnedVersionTime.get + + edit(wid, editedContent) + + workflowResource.retrievePublicWorkflow(wid).lastModifiedTime shouldBe pinnedAt + } } diff --git a/amber/src/test/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowVersionResourceSpec.scala b/amber/src/test/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowVersionResourceSpec.scala index 91b0b1c1be0..35475ec9ca3 100644 --- a/amber/src/test/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowVersionResourceSpec.scala +++ b/amber/src/test/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowVersionResourceSpec.scala @@ -551,10 +551,40 @@ class WorkflowVersionResourceSpec version(3, 0L, positional), // latest — important regardless of content version(2, TimeUnit.SECONDS.toMillis(1), meaningful), // within the aggregate window version(1, TimeUnit.DAYS.toMillis(30), meaningful) // outside it, judged on content - ) + ), + Option.empty[Integer] ) encoded.map(_.vId) shouldBe List(3, 2, 1) encoded.map(_.importance) shouldBe List(true, false, true) + encoded.map(_.isCurrentlyPublic) shouldBe List(false, false, false) + } + + it should "keep the version on public show visible even when the rules would collapse it" in { + val base = 1_700_000_000_000L + def version(vid: Int, offsetMillis: Long, content: String): WorkflowVersion = { + val v = new WorkflowVersion + v.setVid(vid) + v.setWid(testWorkflowWid) + v.setContent(content) + v.setCreationTime(new Timestamp(base - offsetMillis)) + v + } + + val positional = """[{"op":"replace","path":"/operatorPositions/op-1/x","value":5}]""" + + // Version 2 lands inside the aggregation window and carries a positional-only + // patch, so both collapsing rules would hide it. Being the published one wins. + val encoded = WorkflowVersionResource invokePrivate encodeVersionImportance( + List( + version(3, 0L, positional), + version(2, TimeUnit.SECONDS.toMillis(1), positional), + version(1, TimeUnit.SECONDS.toMillis(2), positional) + ), + Option(Integer.valueOf(2)) + ) + + encoded.map(_.importance) shouldBe List(true, true, false) + encoded.map(_.isCurrentlyPublic) shouldBe List(false, true, false) } } diff --git a/common/config/src/main/resources/gui.conf b/common/config/src/main/resources/gui.conf index d770b4ffd20..9c6f3cabe5a 100644 --- a/common/config/src/main/resources/gui.conf +++ b/common/config/src/main/resources/gui.conf @@ -95,6 +95,11 @@ gui { form-view-enabled = true form-view-enabled = ${?GUI_WORKFLOW_WORKSPACE_FORM_VIEW_ENABLED} + # whether an author may pin a version as the public copy of a published workflow, instead of + # the public always following their latest content + version-pinning-enabled = true + version-pinning-enabled = ${?GUI_WORKFLOW_WORKSPACE_VERSION_PINNING_ENABLED} + # Whether to connect to local or production shared editing server. Set to true if you have # reverse proxy set up for y-websocket. production-shared-editing-server = false diff --git a/common/config/src/main/scala/org/apache/texera/common/config/GuiConfig.scala b/common/config/src/main/scala/org/apache/texera/common/config/GuiConfig.scala index 6283a8c225d..54eea914bed 100644 --- a/common/config/src/main/scala/org/apache/texera/common/config/GuiConfig.scala +++ b/common/config/src/main/scala/org/apache/texera/common/config/GuiConfig.scala @@ -61,6 +61,8 @@ object GuiConfig { conf.getBoolean("gui.workflow-workspace.timetravel-enabled") val guiWorkflowWorkspaceFormViewEnabled: Boolean = conf.getBoolean("gui.workflow-workspace.form-view-enabled") + val guiWorkflowWorkspaceVersionPinningEnabled: Boolean = + conf.getBoolean("gui.workflow-workspace.version-pinning-enabled") val guiWorkflowWorkspaceProductionSharedEditingServer: Boolean = conf.getBoolean("gui.workflow-workspace.production-shared-editing-server") val guiWorkflowWorkspacePythonLanguageServerPort: String = diff --git a/common/config/src/test/scala/org/apache/texera/common/config/GuiConfigSpec.scala b/common/config/src/test/scala/org/apache/texera/common/config/GuiConfigSpec.scala index cfc978de3ab..6279d6d3ccb 100644 --- a/common/config/src/test/scala/org/apache/texera/common/config/GuiConfigSpec.scala +++ b/common/config/src/test/scala/org/apache/texera/common/config/GuiConfigSpec.scala @@ -66,6 +66,10 @@ class GuiConfigSpec extends AnyFlatSpec with Matchers { ifUnset("GUI_WORKFLOW_WORKSPACE_FORM_VIEW_ENABLED")( GuiConfig.guiWorkflowWorkspaceFormViewEnabled shouldBe true ) + // Version pinning is on by default now that the whole feature has landed. + ifUnset("GUI_WORKFLOW_WORKSPACE_VERSION_PINNING_ENABLED")( + GuiConfig.guiWorkflowWorkspaceVersionPinningEnabled shouldBe true + ) ifUnset("GUI_WORKFLOW_WORKSPACE_PRODUCTION_SHARED_EDITING_SERVER")( GuiConfig.guiWorkflowWorkspaceProductionSharedEditingServer shouldBe false ) diff --git a/config-service/src/main/scala/org/apache/texera/service/resource/ConfigResource.scala b/config-service/src/main/scala/org/apache/texera/service/resource/ConfigResource.scala index e3dff08575b..7a4c1dc9dc4 100644 --- a/config-service/src/main/scala/org/apache/texera/service/resource/ConfigResource.scala +++ b/config-service/src/main/scala/org/apache/texera/service/resource/ConfigResource.scala @@ -92,6 +92,7 @@ class ConfigResource { "asyncRenderingEnabled" -> GuiConfig.guiWorkflowWorkspaceAsyncRenderingEnabled, "timetravelEnabled" -> GuiConfig.guiWorkflowWorkspaceTimetravelEnabled, "formViewEnabled" -> GuiConfig.guiWorkflowWorkspaceFormViewEnabled, + "versionPinningEnabled" -> GuiConfig.guiWorkflowWorkspaceVersionPinningEnabled, "productionSharedEditingServer" -> GuiConfig.guiWorkflowWorkspaceProductionSharedEditingServer, "defaultExecutionMode" -> GuiConfig.guiWorkflowWorkspaceDefaultExecutionMode, "workflowEmailNotificationEnabled" -> GuiConfig.guiWorkflowWorkspaceWorkflowEmailNotificationEnabled, diff --git a/frontend/src/app/common/service/gui-config.service.mock.ts b/frontend/src/app/common/service/gui-config.service.mock.ts index e108856beda..1f85b53d965 100644 --- a/frontend/src/app/common/service/gui-config.service.mock.ts +++ b/frontend/src/app/common/service/gui-config.service.mock.ts @@ -43,6 +43,7 @@ export class MockGuiConfigService { asyncRenderingEnabled: false, timetravelEnabled: false, formViewEnabled: false, + versionPinningEnabled: false, productionSharedEditingServer: false, pythonLanguageServerPort: "3000", defaultDataTransferBatchSize: 100, 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 7773458b5f1..c5246cb57f1 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 @@ -21,6 +21,7 @@ import { TestBed } from "@angular/core/testing"; import { HttpClientTestingModule, HttpTestingController } from "@angular/common/http/testing"; import { WorkflowPersistService, + WorkflowPublishStatus, WORKFLOW_BASE_URL, WORKFLOW_ID_URL, WORKFLOW_OWNER_URL, @@ -99,6 +100,68 @@ describe("WorkflowPersistService", () => { httpTestingController.expectOne(request => request.method === "POST"); }); + it("should publish without pinning anything", () => { + service.updateWorkflowIsPublished(7, true).subscribe(); + const request = httpTestingController.expectOne(req => req.url.endsWith("workflow/public/7")); + expect(request.request.method).toEqual("PUT"); + }); + + it("should pin the latest version through the pin endpoint", () => { + let received: WorkflowPublishStatus | undefined; + service.pinLatestVersion(7).subscribe(status => (received = status)); + + const request = httpTestingController.expectOne(req => req.url.endsWith("workflow/pin/7")); + expect(request.request.method).toEqual("POST"); + request.flush({ + isPublished: true, + isPinned: true, + pinnedVersionTime: 1700000000000, + hasUnpublishedChanges: false, + }); + + expect(received?.pinnedVersionTime).toEqual(1700000000000); + expect(received?.hasUnpublishedChanges).toBe(false); + }); + + it("should drop the pin through the same path with DELETE", () => { + let received: WorkflowPublishStatus | undefined; + service.unpinVersion(7).subscribe(status => (received = status)); + + const request = httpTestingController.expectOne(req => req.url.endsWith("workflow/pin/7")); + expect(request.request.method).toEqual("DELETE"); + request.flush({ isPublished: true, isPinned: false, hasUnpublishedChanges: false }); + + expect(received?.isPinned).toBe(false); + }); + + it("should announce a save once it lands, not when it is sent", () => { + // Panels that describe the saved copy re-read on this. Announcing on the request rather than the + // response would have them re-read the state the save was about to replace. + let announced = 0; + service.getWorkflowPersistedStream().subscribe(() => announced++); + + const workflow = { wid: 7, name: "n", content: { operators: [], links: [] } } as unknown as Workflow; + service.persistWorkflow(workflow).subscribe(); + const request = httpTestingController.expectOne(req => req.url.endsWith("workflow/persist")); + expect(announced).toBe(0); + + request.flush({ wid: 7, name: "n", content: '{"operators":[]}' }); + + expect(announced).toBe(1); + }); + + it("should report unpublished changes from the publish status endpoint", () => { + let received: WorkflowPublishStatus | undefined; + service.getPublishStatus(7).subscribe(status => (received = status)); + + const request = httpTestingController.expectOne(req => req.url.endsWith("workflow/publish-status/7")); + expect(request.request.method).toEqual("GET"); + request.flush({ isPublished: true, isPinned: true, hasUnpublishedChanges: true }); + + expect(received?.isPinned).toBe(true); + expect(received?.hasUnpublishedChanges).toBe(true); + }); + it("should check if workflow content and name returned correctly", () => { service .createWorkflow(jsonCast(testContent), "testname") 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 fa61b112008..59e81ffb503 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 @@ -53,6 +53,17 @@ export const WORKFLOW_SET_DEFAULT_VIEW_URL = WORKFLOW_BASE_URL + "/set-default-v export const DEFAULT_WORKFLOW_NAME = "Untitled workflow"; +/** A published workflow follows the author's latest until they pin a version. */ +export interface WorkflowPublishStatus { + isPublished: boolean; + /** Whether a version is pinned. False means the public follows the author's latest. */ + isPinned: boolean; + /** When the pinned version was created, which is how the dialog names it. */ + pinnedVersionTime?: number; + /** Whether a pin is holding edits back. Always false while following. */ + hasUnpublishedChanges: boolean; +} + @Injectable({ providedIn: "root", }) @@ -85,6 +96,9 @@ export class WorkflowPersistService { private pendingSaves = 0; private readonly savesDrained = new Subject(); + /** Fires when a save lands: the write, not the keystroke, is what changes the saved copy. */ + private workflowPersisted = new Subject(); + constructor( private http: HttpClient, private notificationService: NotificationService, @@ -172,7 +186,8 @@ export class WorkflowPersistService { }) .pipe( filter((updatedWorkflow: Workflow) => updatedWorkflow != null), - map(WorkflowUtilService.parseWorkflowInfo) + map(WorkflowUtilService.parseWorkflowInfo), + tap(() => this.workflowPersisted.next()) ); // Replayed, so a caller that subscribes after the queue has already relayed the outcome (a // save that was quick, or a synchronous test double) still receives it. @@ -182,6 +197,11 @@ export class WorkflowPersistService { return result.asObservable(); } + /** Emits when a save lands, so panels describing the saved copy can re-read it. */ + public getWorkflowPersistedStream(): Observable { + return this.workflowPersisted.asObservable(); + } + /** * creates a workflow and insert it to backend database and return its information * @param newWorkflowName @@ -274,6 +294,8 @@ export class WorkflowPersistService { name: name, }) .pipe( + // A pin freezes the name too, so a rename is a save like any other. + tap(() => this.workflowPersisted.next()), catchError((error: unknown) => { // @ts-ignore this.notificationService.error(error.error.message); @@ -292,6 +314,8 @@ export class WorkflowPersistService { description: description, }) .pipe( + // Frozen by a pin like the name and the canvas are. + tap(() => this.workflowPersisted.next()), catchError((error: unknown) => { // @ts-ignore this.notificationService.error(error.error.message); @@ -304,6 +328,10 @@ export class WorkflowPersistService { return this.http.get(`${AppSettings.getApiEndpoint()}/${WORKFLOW_BASE_URL}/type/${wid}`, { responseType: "text" }); } + /** + * Publishes the workflow, or unpublishes it. A published workflow follows the author's latest + * content until a version is pinned; see {@link pinLatestVersion}. + */ public updateWorkflowIsPublished(wid: number, isPublished: boolean): Observable { if (isPublished) { return this.http.put(`${AppSettings.getApiEndpoint()}/${WORKFLOW_BASE_URL}/public/${wid}`, null); @@ -312,6 +340,26 @@ export class WorkflowPersistService { } } + /** Pins the author's current version as the public copy, so later edits stop reaching the public. */ + public pinLatestVersion(wid: number): Observable { + return this.http.post( + `${AppSettings.getApiEndpoint()}/${WORKFLOW_BASE_URL}/pin/${wid}`, + null + ); + } + + /** Drops the pin, so the public follows the author's latest content again. */ + public unpinVersion(wid: number): Observable { + return this.http.delete(`${AppSettings.getApiEndpoint()}/${WORKFLOW_BASE_URL}/pin/${wid}`); + } + + /** Whether the workflow is published, whether a version is pinned, and whether it holds edits back. */ + public getPublishStatus(wid: number): Observable { + return this.http.get( + `${AppSettings.getApiEndpoint()}/${WORKFLOW_BASE_URL}/publish-status/${wid}` + ); + } + public setWorkflowPersistFlag(flag: boolean): void { this.workflowPersistFlag = flag; } diff --git a/frontend/src/app/common/type/gui-config.ts b/frontend/src/app/common/type/gui-config.ts index c84c104416a..97344c014d4 100644 --- a/frontend/src/app/common/type/gui-config.ts +++ b/frontend/src/app/common/type/gui-config.ts @@ -34,6 +34,7 @@ export interface GuiConfig { asyncRenderingEnabled: boolean; timetravelEnabled: boolean; formViewEnabled: boolean; + versionPinningEnabled: boolean; productionSharedEditingServer: boolean; pythonLanguageServerPort: string; defaultDataTransferBatchSize: number; diff --git a/frontend/src/app/dashboard/component/user/list-item/card-item/card-item.component.spec.ts b/frontend/src/app/dashboard/component/user/list-item/card-item/card-item.component.spec.ts index 4aa3be86299..9da4d356dea 100644 --- a/frontend/src/app/dashboard/component/user/list-item/card-item/card-item.component.spec.ts +++ b/frontend/src/app/dashboard/component/user/list-item/card-item/card-item.component.spec.ts @@ -230,6 +230,21 @@ describe("CardItemComponent", () => { expect(component.entryLink).toEqual([HUB_WORKFLOW_RESULT_DETAIL, "7"]); }); + it("should route an owner to the hub detail view when the public copy is behind", () => { + // The hub entry advertises the pinned version, so it must open the preview of that version -- + // where the author can clone it -- rather than the editor holding something else. + component.currentUid = 42; + component.entry = makeWorkflowEntry({ id: 7, accessibleUserIds: [42] }); + (component.entry.workflow as any).hasUnpublishedChanges = true; + component.ngOnChanges({ entry: { currentValue: component.entry } as any }); + expect(component.entryLink).toEqual([HUB_WORKFLOW_RESULT_DETAIL, "7"]); + + // ...but their own listings still open the editor. + component.isPrivateSearch = true; + component.ngOnChanges({ entry: { currentValue: component.entry } as any }); + expect(component.entryLink).toEqual([USER_WORKSPACE, "7"]); + }); + it("should format counts as kilo for values >= 1000", () => { expect(component.formatCount(999)).toBe("999"); expect(component.formatCount(1500)).toBe("1.5k"); diff --git a/frontend/src/app/dashboard/component/user/list-item/card-item/card-item.component.ts b/frontend/src/app/dashboard/component/user/list-item/card-item/card-item.component.ts index a35b4742080..44e657cc935 100644 --- a/frontend/src/app/dashboard/component/user/list-item/card-item/card-item.component.ts +++ b/frontend/src/app/dashboard/component/user/list-item/card-item/card-item.component.ts @@ -235,7 +235,7 @@ export class CardItemComponent implements OnChanges { this.disableDelete = !descriptor.isOwner(this.entry); this.canDownload = descriptor.download !== undefined; this.canShare = descriptor.retrieveOwners !== undefined; - this.entryLink = this.resourceRegistry.entryLink(this.entry, this.currentUid); + this.entryLink = this.resourceRegistry.entryLink(this.entry, this.currentUid, this.isPrivateSearch); // Same landing rule as the list row (default-view-landing.ts): a form-default workflow shows // the Form View icon and opens in its form, so the card and the row never disagree. this.defaultsToForm = defaultsToFormView(this.entry, this.config.env.formViewEnabled); diff --git a/frontend/src/app/dashboard/component/user/list-item/list-item.component.spec.ts b/frontend/src/app/dashboard/component/user/list-item/list-item.component.spec.ts index 997c0996b96..68d95062d95 100644 --- a/frontend/src/app/dashboard/component/user/list-item/list-item.component.spec.ts +++ b/frontend/src/app/dashboard/component/user/list-item/list-item.component.spec.ts @@ -314,6 +314,35 @@ describe("ListItemComponent", () => { expect(component.entryLink).toEqual([HUB_WORKFLOW_RESULT_DETAIL, "101"]); }); + it("routes an owned workflow whose public copy is behind to the hub detail page", () => { + // The hub entry advertises the pinned version, so it must open the preview of that version -- + // where the author can clone it -- rather than the editor holding something else. + component.currentUid = 1; + component.entry = { + id: 102, + type: "workflow", + workflow: { isOwner: true, hasUnpublishedChanges: true }, + accessibleUserIds: [1], + ...baseStats, + } as unknown as DashboardEntry; + component.initializeEntry(); + expect(component.entryLink).toEqual([HUB_WORKFLOW_RESULT_DETAIL, "102"]); + }); + + it("keeps the author's own listings pointing at the editor", () => { + component.currentUid = 1; + component.isPrivateSearch = true; + component.entry = { + id: 103, + type: "workflow", + workflow: { isOwner: true, hasUnpublishedChanges: true }, + accessibleUserIds: [1], + ...baseStats, + } as unknown as DashboardEntry; + component.initializeEntry(); + expect(component.entryLink).toEqual([USER_WORKSPACE, "103"]); + }); + it("routes owned datasets to the user dataset page", () => { component.currentUid = 1; component.entry = { diff --git a/frontend/src/app/dashboard/component/user/list-item/list-item.component.ts b/frontend/src/app/dashboard/component/user/list-item/list-item.component.ts index 5f35b3d2896..35ec29c1fe7 100644 --- a/frontend/src/app/dashboard/component/user/list-item/list-item.component.ts +++ b/frontend/src/app/dashboard/component/user/list-item/list-item.component.ts @@ -151,7 +151,7 @@ export class ListItemComponent implements OnChanges { this.disableDelete = !descriptor.isOwner(this.entry); this.canDownload = descriptor.download !== undefined; this.canShare = descriptor.retrieveOwners !== undefined; - this.entryLink = this.resourceRegistry.entryLink(this.entry, this.currentUid); + this.entryLink = this.resourceRegistry.entryLink(this.entry, this.currentUid, this.isPrivateSearch); if (descriptor.hasSize && typeof this.entry.id === "number") { this.size = this.entry.size; } diff --git a/frontend/src/app/dashboard/component/user/share-access/share-access.component.html b/frontend/src/app/dashboard/component/user/share-access/share-access.component.html index 8128f333633..8fd1b38a242 100644 --- a/frontend/src/app/dashboard/component/user/share-access/share-access.component.html +++ b/frontend/src/app/dashboard/component/user/share-access/share-access.component.html @@ -55,6 +55,72 @@ + +
+ + + + +
+ + + + 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 ec41db898c2..675baa9d5ef 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 @@ -22,7 +22,7 @@ import { HttpClientTestingModule } from "@angular/common/http/testing"; import { NoopAnimationsModule } from "@angular/platform-browser/animations"; import { By } from "@angular/platform-browser"; import { HttpErrorResponse } from "@angular/common/http"; -import { of, throwError } from "rxjs"; +import { Subject, of, throwError } from "rxjs"; import { NZ_MODAL_DATA, NzModalRef, NzModalService } from "ng-zorro-antd/modal"; import { NzMessageService } from "ng-zorro-antd/message"; @@ -35,6 +35,7 @@ import { NotificationService } from "../../../../common/service/notification/not import { DatasetService } from "../../../service/user/dataset/dataset.service"; import { WorkflowPersistService } from "src/app/common/service/workflow-persist/workflow-persist.service"; import { WorkflowActionService } from "src/app/workspace/service/workflow-graph/model/workflow-action.service"; +import { GuiConfigService } from "src/app/common/service/gui-config.service"; import { Privilege } from "../../../type/share-access.interface"; import { ResourceRegistryService } from "../../../service/user/resource-registry/resource-registry.service"; import { ResourceDescriptor } from "../../../type/resource-descriptor"; @@ -47,6 +48,7 @@ interface SetupOptions { currentEmail?: string | undefined; /** What the registry answers for the kind, in place of the shipped descriptor. */ descriptor?: ResourceDescriptor; + versionPinningEnabled?: boolean; } /** @@ -71,7 +73,7 @@ describe("ShareAccessComponent", () => { let messageSpy: { error: ReturnType }; let modalRefSpy: { close: ReturnType }; let modalServiceSpy: { create: ReturnType }; - let workflowPersistSpy: { + let workflowPersistSpy: any /* the publish-status spies vary per test */ & { getWorkflowIsPublished: ReturnType; updateWorkflowIsPublished: ReturnType; }; @@ -79,7 +81,10 @@ describe("ShareAccessComponent", () => { getDataset: ReturnType; updateDatasetPublicity: ReturnType; }; - let workflowActionSpy: { setWorkflowIsPublished: ReturnType }; + let workflowActionSpy: { + setWorkflowIsPublished: ReturnType; + getWorkflow: ReturnType; + }; let userServiceCurrentEmail: string | undefined; // The component reads publicity back after writing it, so the doubles have to hold state: // the workflow endpoint sets it absolutely, the dataset one toggles. @@ -87,6 +92,8 @@ describe("ShareAccessComponent", () => { let datasetPublished: boolean; let capturedModalConfigs: any[]; /** The NzModalRef stubs handed back by modalService.create, in creation order. */ + /** Stands in for the editor's autosave landing, which is what the panel has to follow. */ + let persisted: Subject; let capturedModalRefs: { close: ReturnType }[]; /** * The fixture built by the most recent setupComponent() call, for the template-level tests. @@ -96,7 +103,15 @@ describe("ShareAccessComponent", () => { let fixture: ComponentFixture; function setupComponent(opts: SetupOptions = {}): ShareAccessComponent { - const { type = "workflow", id = 1, inWorkspace = false, currentEmail = "me@example.com" } = opts; + const { + type = "workflow", + id = 1, + inWorkspace = false, + currentEmail = "me@example.com", + // On for the cases that are about the panel; the feature ships behind this flag, and the + // case below covers what the dialog looks like while it is off. + versionPinningEnabled = true, + } = opts; userServiceCurrentEmail = currentEmail; TestBed.configureTestingModule({ @@ -118,6 +133,7 @@ describe("ShareAccessComponent", () => { { provide: WorkflowPersistService, useValue: workflowPersistSpy }, { provide: DatasetService, useValue: datasetServiceSpy }, { provide: WorkflowActionService, useValue: workflowActionSpy }, + { provide: GuiConfigService, useValue: { env: { versionPinningEnabled } } }, ], }); if (opts.descriptor) { @@ -128,9 +144,34 @@ describe("ShareAccessComponent", () => { return fixture.componentInstance; } + /** Everything the publish panel renders, for the assertions about what the author actually reads. */ + function publishLineText(): string { + fixture.detectChanges(); + return (fixture.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[] { + fixture.detectChanges(); + return Array.from(fixture.nativeElement.querySelectorAll(".ant-segmented-item-label")).map((b: any) => + (b.textContent ?? "").trim() + ); + } + + /** The card under the line, which only a pin holding edits back puts on screen. */ + function publishNoteText(): string { + fixture.detectChanges(); + return (fixture.nativeElement.querySelector(".publish-card")?.textContent ?? "").replace(/\s+/g, " ").trim(); + } + beforeEach(() => { TestBed.resetTestingModule(); fixture = undefined as unknown as ComponentFixture; + persisted = new Subject(); capturedModalConfigs = []; capturedModalRefs = []; gmailSpy = { sendEmail: vi.fn() }; @@ -153,12 +194,19 @@ describe("ShareAccessComponent", () => { }; workflowPublished = false; datasetPublished = false; + const followingStatus = { isPublished: true, isPinned: false, hasUnpublishedChanges: false }; workflowPersistSpy = { getWorkflowIsPublished: vi.fn(() => of(workflowPublished ? "Public" : "Private")), updateWorkflowIsPublished: vi.fn((_id: number, next: boolean) => { workflowPublished = next; return of(null); }), + getPublishStatus: vi.fn().mockReturnValue(of(followingStatus)), + getWorkflowPersistedStream: vi.fn().mockReturnValue(persisted.asObservable()), + isWorkflowPersistEnabled: vi.fn().mockReturnValue(true), + persistWorkflow: vi.fn().mockReturnValue(of({ wid: 3 })), + pinLatestVersion: vi.fn().mockReturnValue(of({ ...followingStatus, isPinned: true })), + unpinVersion: vi.fn().mockReturnValue(of(followingStatus)), }; datasetServiceSpy = { getDataset: vi.fn(() => of({ dataset: { isPublic: datasetPublished } })), @@ -167,7 +215,11 @@ describe("ShareAccessComponent", () => { return of(null); }), }; - workflowActionSpy = { setWorkflowIsPublished: vi.fn() }; + workflowActionSpy = { + setWorkflowIsPublished: vi.fn(), + // The publish panel never writes; it only re-reads once a save of the author's own lands. + getWorkflow: vi.fn().mockReturnValue({ wid: 3, name: "w", content: { operators: [], links: [] } }), + }; }); function getFooterButton(config: any, label: string): { onClick: () => void } { @@ -1127,4 +1179,340 @@ describe("ShareAccessComponent", () => { expect(datasetServiceSpy.updateDatasetPublicity).not.toHaveBeenCalled(); }); }); + + 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", () => { + workflowPublished = true; + const c = setupComponent({ type: "workflow", id: 3, ...asOwner }); + expect(c.publishStatus).toBeDefined(); + + c.setPublished(false); + + 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", () => { + workflowPublished = false; + const c = setupComponent({ type: "workflow", id: 3, ...asOwner }); + expect(workflowPersistSpy.getPublishStatus).not.toHaveBeenCalled(); + + c.setPublished(true); + + 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("shows nothing and asks nothing while the feature is off", () => { + // The series ships dark: until the last PR turns the flag on, the dialog is exactly what it + // is today, and it does not even ask the server about a state it cannot show. + workflowPersistSpy.getWorkflowIsPublished.mockReturnValue(of("Public")); + workflowPersistSpy.getPublishStatus.mockReturnValue( + of({ isPublished: true, isPinned: true, hasUnpublishedChanges: true }) + ); + + setupComponent({ type: "workflow", id: 3, versionPinningEnabled: false, ...asOwner }); + + expect(publishLineText()).toBe(""); + expect(workflowPersistSpy.getPublishStatus).not.toHaveBeenCalled(); + }); + + it("writes nothing when it opens", () => { + // The dialog reports on the saved copy rather than forcing the canvas to become it: the canvas + // is empty while a workflow loads, and a save nobody asked for could write that emptiness over + // every operator the workflow had. + 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).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 }); + + fixture.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(""); + }); + }); }); 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 b5baf5d9dba..fe631351fed 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,13 +27,18 @@ 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 { catchError, of, switchMap } from "rxjs"; +import { catchError, forkJoin, of, switchMap } from "rxjs"; import { NzMessageService } from "ng-zorro-antd/message"; import { WorkflowActionService } from "src/app/workspace/service/workflow-graph/model/workflow-action.service"; +import { + WorkflowPersistService, + WorkflowPublishStatus, +} from "src/app/common/service/workflow-persist/workflow-persist.service"; +import { GuiConfigService } from "../../../../common/service/gui-config.service"; import { ResourceRegistryService } from "../../../service/user/resource-registry/resource-registry.service"; import { ResourceDescriptor } from "../../../type/resource-descriptor"; import { EntityType } from "../../../../hub/service/hub.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"; @@ -46,6 +51,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({ @@ -73,6 +80,12 @@ import { NzTooltipDirective } from "ng-zorro-antd/tooltip"; NgFor, NzTagComponent, NzTooltipDirective, + NzSegmentedComponent, + NzBadgeComponent, + NgSwitch, + NgSwitchCase, + NgSwitchDefault, + DatePipe, ], }) export class ShareAccessComponent implements OnInit, OnDestroy { @@ -91,6 +104,16 @@ export class ShareAccessComponent implements OnInit, OnDestroy { isPublic: boolean | null = null; /** Undefined for kinds the registry does not carry, i.e. computing units. */ private readonly descriptor: ResourceDescriptor | undefined; + /** 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(); @@ -103,7 +126,9 @@ export class ShareAccessComponent implements OnInit, OnDestroy { private message: NzMessageService, private modalService: NzModalService, private workflowActionService: WorkflowActionService, + private workflowPersistService: WorkflowPersistService, private resourceRegistry: ResourceRegistryService, + protected config: GuiConfigService, private modalRef: NzModalRef ) { this.validateForm = this.formBuilder.group({ @@ -114,6 +139,61 @@ export class ShareAccessComponent implements OnInit, OnDestroy { this.descriptor = this.resourceRegistry.find(this.type as EntityType); } + /** Only for a published workflow the user can publish -- the endpoint requires write access. */ + private refreshPublishStatus(): void { + if (!this.config.env.versionPinningEnabled) { + return; + } + 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)); + } + get hasWriteAccess(): boolean { if (!this.currentEmail) { return false; @@ -126,35 +206,54 @@ 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; - }); - // Stays null for kinds that cannot be published, which is what hides the publish buttons. - // A failed request leaves it null too, and the buttons are equally gone, so say so: without - // this the dialog looked complete while quietly offering one control fewer, and the only way - // to find out was the network tab. - this.descriptor - ?.isPublic?.(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 + // panel only applies to a published workflow. + // Stays undefined for kinds that cannot be published, which is what hides the publish buttons. + forkJoin([ + this.accessService.getAccessList(this.type, this.id), + this.accessService.getOwner(this.type, this.id), + this.descriptor?.isPublic?.(this.id) ?? of(undefined), + ]) .pipe(untilDestroyed(this)) .subscribe({ - next: isPublic => (this.isPublic = isPublic), + next: ([accessList, owner, isPublic]) => { + this.accessList = accessList; + this.owner = owner; + if (isPublic !== undefined) { + this.isPublic = isPublic; + } + if (this.type !== "workflow") { + return; + } + // Answered from the saved copy, which the editor writes a few seconds after the last edit. + // Deliberately not forced to save first: the canvas is not always the workflow -- it is + // empty while one loads, and stays empty if the collaborative model never arrives -- so a + // save nobody asked for could write that emptiness over every operator the workflow had. + // The subscription below picks the answer up again as soon as the autosave lands. + this.refreshPublishStatus(); + }, error: () => { - // `ngOnInit` is re-entered as a refresh after an access change, so a value from the - // previous read may still be here. Drop it: keeping it would leave the buttons on - // screen, showing a state nothing has confirmed, while the toast says they are gone. + // A failed read leaves the publish controls hidden, so say so: without this the dialog + // looked complete while quietly offering one control fewer, and the only way to find out + // was the network tab. `ngOnInit` is re-entered as a refresh after an access change, so a + // value from the previous read may still be here -- drop it rather than leave a state + // nothing has confirmed on screen while the toast says it is gone. this.isPublic = null; + this.publishStatus = undefined; this.notificationService.error( `Could not read whether this ${this.type} is public, so that choice is not shown.` ); }, }); + if (this.type === "workflow") { + // 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()); + } } ngOnDestroy(): void { @@ -350,7 +449,11 @@ export class ShareAccessComponent implements OnInit, OnDestroy { const cloneWarning = this.descriptor?.affordances?.clonable ? ", along with the right to clone your work" : ""; const modal: NzModalRef = this.modalService.create({ nzTitle: "Notice", - nzContent: `Publishing your ${this.type} would grant all Texera users read access to your ${this.type}${cloneWarning}.`, + nzContent: + `Publishing your ${this.type} would grant all Texera users read access to your ${this.type}${cloneWarning}.` + + (this.type === "workflow" + ? " The public will follow your latest version as you save it, unless you pin one." + : ""), nzFooter: [ { label: "Cancel", @@ -414,6 +517,7 @@ export class ShareAccessComponent implements OnInit, OnDestroy { if (this.inWorkspace) { this.workflowActionService.setWorkflowIsPublished(published ? 1 : 0); } + this.refreshPublishStatus(); this.notificationService.success(`${label} ${published ? "published" : "unpublished"} successfully`); }, error: (error: unknown) => { diff --git a/frontend/src/app/dashboard/service/user/resource-registry/resource-registry.service.ts b/frontend/src/app/dashboard/service/user/resource-registry/resource-registry.service.ts index d096f2b3230..59879aaaee4 100644 --- a/frontend/src/app/dashboard/service/user/resource-registry/resource-registry.service.ts +++ b/frontend/src/app/dashboard/service/user/resource-registry/resource-registry.service.ts @@ -98,8 +98,12 @@ export class ResourceRegistryService { /** * Where an entry's card links to: the owner-facing page when the viewer can reach it, the hub page * otherwise. An entry with no route, or one not yet persisted, links nowhere. + * + * Access alone is not enough outside the viewer's own listings: a hub entry shows the public copy, + * so an author whose working copy has moved on behind a pin is sent to that copy -- what the entry + * advertised, and where they can clone it -- rather than into their editor showing something else. */ - public entryLink(entry: DashboardEntry, currentUid: number | undefined): string[] { + public entryLink(entry: DashboardEntry, currentUid: number | undefined, isPrivateSearch = false): string[] { const descriptor = this.get(entry.type); if (descriptor.privateRoute === undefined || typeof entry.id !== "number") { return []; @@ -109,6 +113,11 @@ export class ResourceRegistryService { } const reachableByViewer = currentUid !== undefined && (entry.ownerId === currentUid || entry.accessibleUserIds.includes(currentUid)); - return [reachableByViewer ? descriptor.privateRoute : descriptor.hubRoute, String(entry.id)]; + const advertisesAnOlderCopy = + !isPrivateSearch && entry.type === "workflow" && entry.workflow?.hasUnpublishedChanges === true; + return [ + reachableByViewer && !advertisesAnOlderCopy ? descriptor.privateRoute : descriptor.hubRoute, + String(entry.id), + ]; } } diff --git a/frontend/src/app/dashboard/service/user/workflow-version/workflow-version.service.ts b/frontend/src/app/dashboard/service/user/workflow-version/workflow-version.service.ts index 1b527e35cb9..cfaa08c87df 100644 --- a/frontend/src/app/dashboard/service/user/workflow-version/workflow-version.service.ts +++ b/frontend/src/app/dashboard/service/user/workflow-version/workflow-version.service.ts @@ -18,7 +18,7 @@ */ import { Injectable } from "@angular/core"; -import { BehaviorSubject, Observable } from "rxjs"; +import { BehaviorSubject, Observable, Subject } from "rxjs"; import { WorkflowActionService } from "../../../../workspace/service/workflow-graph/model/workflow-action.service"; import { Workflow, WorkflowContent } from "../../../../common/type/workflow"; import { WorkflowPersistService } from "../../../../common/service/workflow-persist/workflow-persist.service"; @@ -58,6 +58,7 @@ export class WorkflowVersionService { private differentOpIDsList: DifferentOpIDsList = { modified: [], added: [], deleted: [] }; public selectedVersionId = new BehaviorSubject(null); public selectedDisplayedVersionId = new BehaviorSubject(null); + private publicVersionChanged = new Subject(); constructor( private workflowActionService: WorkflowActionService, @@ -91,6 +92,20 @@ export class WorkflowVersionService { return this.displayParticularWorkflowVersion.asObservable(); } + /** + * Announces that the version the Hub serves may have moved, so anything marking it re-reads. + * + * Pinning happens in the share dialog, which is a modal over the workspace and knows nothing about + * the panels underneath it. Announcing the change rather than reaching for them keeps that one-way. + */ + public notifyPublicVersionChanged(): void { + this.publicVersionChanged.next(); + } + + public getPublicVersionChangedStream(): Observable { + return this.publicVersionChanged.asObservable(); + } + public get canRestoreVersion(): boolean { return this.modificationEnabledBeforeTempWorkflow !== undefined && this.modificationEnabledBeforeTempWorkflow; } diff --git a/frontend/src/app/dashboard/type/dashboard-workflow.interface.ts b/frontend/src/app/dashboard/type/dashboard-workflow.interface.ts index 1c8f68328b0..a8ecd3a5df9 100644 --- a/frontend/src/app/dashboard/type/dashboard-workflow.interface.ts +++ b/frontend/src/app/dashboard/type/dashboard-workflow.interface.ts @@ -26,4 +26,6 @@ export interface DashboardWorkflow { accessLevel: string; ownerId: number; coverImage: string | null; + /** Whether the copy on public show is behind the author's working copy. */ + hasUnpublishedChanges?: boolean; } diff --git a/frontend/src/app/dashboard/type/workflow-version-entry.ts b/frontend/src/app/dashboard/type/workflow-version-entry.ts index 535cd75f65b..a1caefd2317 100644 --- a/frontend/src/app/dashboard/type/workflow-version-entry.ts +++ b/frontend/src/app/dashboard/type/workflow-version-entry.ts @@ -23,6 +23,8 @@ export interface WorkflowVersionEntry creationTime: number; content: string; importance: boolean; + /** True for the one version the Hub is serving right now. */ + isCurrentlyPublic?: boolean; }> {} export interface WorkflowVersionCollapsableEntry extends WorkflowVersionEntry { diff --git a/frontend/src/app/workspace/component/left-panel/versions-list/versions-list.component.html b/frontend/src/app/workspace/component/left-panel/versions-list/versions-list.component.html index bed3bd1567a..30ebc8fc1b3 100644 --- a/frontend/src/app/workspace/component/left-panel/versions-list/versions-list.component.html +++ b/frontend/src/app/workspace/component/left-panel/versions-list/versions-list.component.html @@ -54,6 +54,18 @@ (click)="getVersion(row.vId, getDisplayedVersionId(i, l), i)" class="version-link"> {{row.creationTime | date:'MM/dd/yy HH:mm:ss'}} + + + + Currently public + diff --git a/frontend/src/app/workspace/component/left-panel/versions-list/versions-list.component.scss b/frontend/src/app/workspace/component/left-panel/versions-list/versions-list.component.scss index 4bbb072b2e2..6571a49222b 100644 --- a/frontend/src/app/workspace/component/left-panel/versions-list/versions-list.component.scss +++ b/frontend/src/app/workspace/component/left-panel/versions-list/versions-list.component.scss @@ -37,3 +37,22 @@ font-size: 11px; padding: 0; } + +// The version on public show, as a quiet secondary line under the timestamp rather than a tag: +// no border, no fill, colour does the work. +.publication { + display: flex; + align-items: center; + gap: 5px; + margin-top: 1px; + font-size: 10px; + line-height: 14px; + color: #1890ff; +} + +.publication-mark { + width: 5px; + height: 5px; + border-radius: 50%; + background: #1890ff; +} diff --git a/frontend/src/app/workspace/component/left-panel/versions-list/versions-list.component.spec.ts b/frontend/src/app/workspace/component/left-panel/versions-list/versions-list.component.spec.ts index 827c41e4db9..918f780dcd3 100644 --- a/frontend/src/app/workspace/component/left-panel/versions-list/versions-list.component.spec.ts +++ b/frontend/src/app/workspace/component/left-panel/versions-list/versions-list.component.spec.ts @@ -157,6 +157,46 @@ describe("VersionsListComponent", () => { expect(unhighlightSpy).toHaveBeenCalledWith(highlights); }); + it("marks the version the Hub is serving, and only that one", () => { + const entries: WorkflowVersionEntry[] = [ + { vId: 9, creationTime: 300, content: "c", importance: true }, + { vId: 8, creationTime: 200, content: "b", importance: true, isCurrentlyPublic: true }, + { vId: 7, creationTime: 100, content: "a", importance: true }, + ]; + (component.route.snapshot.params as any).id = 42; + vi.spyOn(workflowVersionService, "retrieveVersionsOfWorkflow").mockReturnValue(of(entries)); + + component.ngOnInit(); + + expect(component.versionsList?.map(v => v.isCurrentlyPublic)).toEqual([undefined, true, undefined]); + }); + + it("moves the mark when the pinned version changes while the panel is open", () => { + // Pinning happens in the share dialog, over this panel: a mark read once would still be on + // whichever version was public when the panel was opened. + const before: WorkflowVersionEntry[] = [ + { vId: 9, creationTime: 300, content: "c", importance: true }, + { vId: 8, creationTime: 200, content: "b", importance: true, isCurrentlyPublic: true }, + ]; + const after: WorkflowVersionEntry[] = [ + { vId: 9, creationTime: 300, content: "c", importance: true, isCurrentlyPublic: true }, + { vId: 8, creationTime: 200, content: "b", importance: true }, + ]; + (component.route.snapshot.params as any).id = 42; + const retrieve = vi + .spyOn(workflowVersionService, "retrieveVersionsOfWorkflow") + .mockReturnValueOnce(of(before)) + .mockReturnValueOnce(of(after)); + + component.ngOnInit(); + expect(component.versionsList?.map(v => v.isCurrentlyPublic)).toEqual([undefined, true]); + + workflowVersionService.notifyPublicVersionChanged(); + + expect(retrieve).toHaveBeenCalledTimes(2); + expect(component.versionsList?.map(v => v.isCurrentlyPublic)).toEqual([true, undefined]); + }); + it("should not retrieve versions when the route has no workflow id", () => { (component.route.snapshot.params as any).id = undefined; const retrieveSpy = vi.spyOn(workflowVersionService, "retrieveVersionsOfWorkflow"); diff --git a/frontend/src/app/workspace/component/left-panel/versions-list/versions-list.component.ts b/frontend/src/app/workspace/component/left-panel/versions-list/versions-list.component.ts index 7aca366a9d6..2a80f2747c4 100644 --- a/frontend/src/app/workspace/component/left-panel/versions-list/versions-list.component.ts +++ b/frontend/src/app/workspace/component/left-panel/versions-list/versions-list.component.ts @@ -100,6 +100,16 @@ export class VersionsListComponent implements OnInit { if (wid === undefined) { return; } + this.loadVersions(wid); + // Which version the Hub serves is chosen in the share dialog, on top of this panel: without + // re-reading, the mark would still be on whichever version was public when the panel was opened. + this.workflowVersionService + .getPublicVersionChangedStream() + .pipe(untilDestroyed(this)) + .subscribe(() => this.loadVersions(wid)); + } + + private loadVersions(wid: number): void { this.workflowVersionService .retrieveVersionsOfWorkflow(wid) .pipe(untilDestroyed(this)) @@ -109,6 +119,7 @@ export class VersionsListComponent implements OnInit { creationTime: version.creationTime, content: version.content, importance: version.importance, + isCurrentlyPublic: version.isCurrentlyPublic, expand: false, })); }); diff --git a/frontend/src/app/workspace/component/menu/menu.component.spec.ts b/frontend/src/app/workspace/component/menu/menu.component.spec.ts index abcb1c619f7..9df510d4720 100644 --- a/frontend/src/app/workspace/component/menu/menu.component.spec.ts +++ b/frontend/src/app/workspace/component/menu/menu.component.spec.ts @@ -1231,6 +1231,19 @@ describe("MenuComponent", () => { expect(createSpy).not.toHaveBeenCalled(); }); + + it("tells the versions panel to re-read once the dialog closes", async () => { + // The dialog is where a version gets pinned, and the panel behind it marks the pinned one. + // A real id, because the dialog does not open without one. + component.workflowId = 7; + vi.spyOn(workflowPersistService, "retrieveOwners").mockReturnValue(of([])); + vi.spyOn(modalService, "create").mockReturnValue({ afterClose: of(undefined) } as unknown as NzModalRef); + const announce = vi.spyOn(component.workflowVersionService, "notifyPublicVersionChanged"); + + await component.onClickOpenShareAccess(); + + expect(announce).toHaveBeenCalled(); + }); }); it("onClickCreateNewWorkflow resets the graph and navigates back to root", () => { diff --git a/frontend/src/app/workspace/component/menu/menu.component.ts b/frontend/src/app/workspace/component/menu/menu.component.ts index b81eb7a9532..d658c1238f6 100644 --- a/frontend/src/app/workspace/component/menu/menu.component.ts +++ b/frontend/src/app/workspace/component/menu/menu.component.ts @@ -404,7 +404,11 @@ export class MenuComponent implements OnInit, OnDestroy { modalRef.afterClose.pipe(untilDestroyed(this)).subscribe(result => { if (result?.userRevokedOwnAccess) { this.router.navigate([USER_WORKFLOW]); + return; } + // The dialog is where a version gets pinned, and the versions panel behind it marks the pinned + // one. Announcing on close rather than on each pin because the panel is covered until then. + this.workflowVersionService.notifyPublicVersionChanged(); }); }