From f67e00cbd78d1c17c42ec6af4fd082caf4a0540c Mon Sep 17 00:00:00 2001 From: Yang Zhang Date: Sat, 22 Aug 2026 14:35:59 -0700 Subject: [PATCH 1/6] feat(workflow): freeze a pinned version as the public copy A public workflow follows the author's latest content, as publishing has always done. This adds the other state: the author pins the version they have now, and the public copy stops moving until they pin again. `is_public` stays the on/off switch; `published_content` is the pin, NULL while following. `WorkflowPublishService` owns the two states, and three endpoints expose them: POST and DELETE `/workflow/pin/{wid}` to pin and unpin, GET `/workflow/publish-status/{wid}` for what the author is shown. Publishing and unpublishing move through the same service, so unpublishing drops the pin rather than leaving a private workflow carrying one. Two paths are narrowed so a pin can hold. A save wrote the whole row back, so a publish landing while a save was in flight was silently rolled back, and a request body could set the publish columns itself; saves now write only name, description and content. Creating a workflow clears the publish columns for the same reason. Nothing reads the pinned copy yet: every workflow is in the following state it is in today, and nothing on screen changes. Co-Authored-By: Claude Opus 5 --- .../workflow/WorkflowPublishService.scala | 195 ++++++ .../user/workflow/WorkflowResource.scala | 100 ++- .../user/workflow/WorkflowPublishSpec.scala | 587 ++++++++++++++++++ 3 files changed, 866 insertions(+), 16 deletions(-) create mode 100644 amber/src/main/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowPublishService.scala create mode 100644 amber/src/test/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowPublishSpec.scala 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 new file mode 100644 index 00000000000..46b4548e7ad --- /dev/null +++ b/amber/src/main/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowPublishService.scala @@ -0,0 +1,195 @@ +/* + * 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 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.enums.DefaultViewEnum +import org.apache.texera.dao.jooq.generated.tables.daos.WorkflowDao +import org.apache.texera.dao.jooq.generated.tables.pojos.Workflow +import org.jooq.DSLContext + +import javax.ws.rs.NotFoundException +import scala.util.Try + +/** + * Version pinning for public workflows. + * + * A public workflow follows the author's latest, as publishing has always done, until the author + * pins the version they have now: the public then keeps seeing that frozen copy while the author's + * later edits stay in the workflow's own columns until they pin again. + * + * `is_public` stays the on/off switch; `published_content` is the pin, NULL while following. A pin + * freezes everything on public show -- the graph, the title, the description and the view it opens + * in -- because a copy that froze only its graph would still advertise a title nobody published. + * + * Not to be confused with sharing: a user granted access always tracks the author's latest, pin or + * no pin. Only viewers who arrive because the workflow is public are held at the frozen copy. + */ +object WorkflowPublishService extends LazyLogging { + + private def context: DSLContext = SqlServer.getInstance().createDSLContext() + + /** + * 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. + */ + case class PublishStatus( + isPublished: Boolean, + isPinned: Boolean, + hasUnpublishedChanges: Boolean + ) + + /** + * Whether two workflow contents describe the same graph. Compared as parsed trees, because the + * two blobs travel by different routes and the same graph can come back with its whitespace or + * key order rearranged -- reporting that as an edit the public cannot see would be an alarm the + * author cannot clear. + */ + private def sameContent(a: String, b: String): Boolean = + a == b || Try(objectMapper.readTree(a) == objectMapper.readTree(b)).getOrElse(false) + + /** The workflow, or a 404. */ + private def requireWorkflow(wid: Integer): Workflow = + Option(new WorkflowDao(context.configuration).fetchOneByWid(wid)) + .getOrElse(throw new NotFoundException(s"Workflow $wid not found")) + + /** + * Turns publishing on, and touches nothing else. A workflow coming back from private is + * following the author's latest, because unpublishing always drops the pin: coming back should + * not silently put old public content back on show. Called on a workflow that is already public + * it changes nothing, pin included. + */ + def publish(wid: Integer): PublishStatus = { + val updated = context + .update(WORKFLOW) + .set(WORKFLOW.IS_PUBLIC, java.lang.Boolean.TRUE) + .where(WORKFLOW.WID.eq(wid)) + .execute() + if (updated == 0) { + throw new NotFoundException(s"Workflow $wid not found") + } + // Deliberately not "following latest": this turns publishing on and touches nothing else, so a + // workflow that somehow arrives here already pinned stays pinned. + logger.info(s"Workflow $wid published") + statusOf(wid) + } + + /** + * Freezes the author's current copy as the public one, and turns publishing on. The title, the + * description and the default view freeze with the graph: they are as public as it is, and the + * database refuses a pinned copy that carries only part of itself. The view matters because a + * form's definition rides inside the content -- serving the live preference over a frozen graph + * would open a form on a copy that has none. + * + * Each column is copied from its own row rather than from a workflow read a moment earlier, so + * there is no window in which the author's next save lands and the pin freezes the version + * before it -- which would leave them looking at "you have unpublished changes" the instant + * after they pinned. + * + * @return how many rows it matched, so a missing workflow is distinguishable from a done one. + */ + private def writePin(wid: Integer): Int = + context + .update(WORKFLOW) + .set(WORKFLOW.IS_PUBLIC, java.lang.Boolean.TRUE) + .set(WORKFLOW.PUBLISHED_CONTENT, WORKFLOW.CONTENT) + .set(WORKFLOW.PUBLISHED_NAME, WORKFLOW.NAME) + .set(WORKFLOW.PUBLISHED_DESCRIPTION, WORKFLOW.DESCRIPTION) + .set(WORKFLOW.PUBLISHED_DEFAULT_VIEW, WORKFLOW.DEFAULT_VIEW) + .where(WORKFLOW.WID.eq(wid)) + .execute() + + /** + * Clears the pinned copy in one statement, optionally unpublishing too: the constraint accepts a + * 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. + * + * @return how many rows it matched, so a missing workflow is distinguishable from a done one. + */ + private def clearPin(wid: Integer, alsoUnpublish: Boolean = false): Int = { + val cleared = context + .update(WORKFLOW) + .set(WORKFLOW.PUBLISHED_CONTENT, null.asInstanceOf[String]) + .set(WORKFLOW.PUBLISHED_NAME, null.asInstanceOf[String]) + .set(WORKFLOW.PUBLISHED_DESCRIPTION, null.asInstanceOf[String]) + .set(WORKFLOW.PUBLISHED_DEFAULT_VIEW, null.asInstanceOf[DefaultViewEnum]) + val statement = + if (alsoUnpublish) cleared.set(WORKFLOW.IS_PUBLIC, java.lang.Boolean.FALSE) else cleared + statement.where(WORKFLOW.WID.eq(wid)).execute() + } + + /** Pins the current content as the public copy. Moving a pin forward is the same operation. */ + def pinLatest(wid: Integer): PublishStatus = { + if (writePin(wid) == 0) { + throw new NotFoundException(s"Workflow $wid not found") + } + logger.info(s"Workflow $wid pinned to its latest content") + statusOf(wid) + } + + /** + * Drops the pin, so the public follows the author's latest again. The workflow stays public. + */ + def unpin(wid: Integer): PublishStatus = { + if (clearPin(wid) == 0) { + throw new NotFoundException(s"Workflow $wid not found") + } + logger.info(s"Workflow $wid unpinned, following latest") + statusOf(wid) + } + + /** + * Turns publishing off and drops the pin. Publishing again starts in the following state; the + * previous frozen copy is deliberately not remembered, so an unpublish/re-publish cycle cannot + * silently restore old public content. + */ + def unpublish(wid: Integer): Unit = { + if (clearPin(wid, alsoUnpublish = true) == 0) { + throw new NotFoundException(s"Workflow $wid not found") + } + logger.info(s"Workflow $wid unpublished") + } + + /** Whether a version is pinned, and whether it is holding edits back. */ + def statusOf(wid: Integer): PublishStatus = { + val workflow = requireWorkflow(wid) + val pinned = workflow.getPublishedContent != null + PublishStatus( + isPublished = workflow.getIsPublic, + isPinned = pinned, + // Literally "what the public sees is not what you have". Every field the pin freezes counts: + // a rename the public cannot see is held back exactly as an edit to the graph is. Compared on + // values rather than on a version id, so that an edit and its undo report nothing held back. + hasUnpublishedChanges = pinned && ( + !sameContent(workflow.getPublishedContent, workflow.getContent) || + workflow.getPublishedName != workflow.getName || + workflow.getPublishedDescription != workflow.getDescription || + workflow.getPublishedDefaultView != workflow.getDefaultView + ) + ) + } +} 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 17cd7a11fad..2b8f738376c 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 @@ -35,13 +35,14 @@ import org.apache.texera.dao.jooq.generated.tables.daos.{ WorkflowUserAccessDao } import org.apache.texera.dao.jooq.generated.tables.pojos._ +import org.apache.texera.dao.jooq.generated.tables.records.WorkflowRecord import org.apache.texera.service.util.LargeBinaryManager import org.apache.texera.web.resource.dashboard.hub.EntityType import org.apache.texera.web.service.WarehouseReadGuard import org.apache.texera.web.resource.dashboard.hub.HubResource.recordCloneAction import org.apache.texera.web.resource.dashboard.user.workflow.WorkflowResource._ import org.jooq.impl.DSL.{noCondition, max} -import org.jooq.{Condition, DSLContext, Record10, Result, SelectOnConditionStep} +import org.jooq.{Condition, DSLContext, Record10, Result, SelectOnConditionStep, TableField} import java.sql.Timestamp import java.util @@ -164,12 +165,21 @@ object WorkflowResource { workflow } - private def updateWorkflowField( - workflow: Workflow, + /** + * Writes one field of a workflow, and only that field. The endpoints take a whole `Workflow` + * from the client, of which exactly two things are used: which workflow, and the new value. + * + * Reading the row and writing the whole POJO back would carry every other column with it, so + * anything landing between the read and the write was silently rewritten to whatever the read had + * seen: a save reverted, a publish undone, or -- since a pin travels in those columns too -- a + * workflow the author had just unpublished put back on public show under its frozen copy. + */ + private def updateWorkflowField[T]( + wid: Integer, sessionUser: SessionUser, - updateFunction: Workflow => Unit + field: TableField[WorkflowRecord, T], + value: T ): Unit = { - val wid = workflow.getWid val user = sessionUser.getUser if ( @@ -178,9 +188,7 @@ object WorkflowResource { user.getUid ) ) { - val userWorkflow = workflowDao.fetchOneByWid(wid) - updateFunction(userWorkflow) - workflowDao.update(userWorkflow) + context.update(WORKFLOW).set(field, value).where(WORKFLOW.WID.eq(wid)).execute() } else { throw new ForbiddenException("No sufficient access privilege.") } @@ -706,7 +714,7 @@ class WorkflowResource extends LazyLogging { workflow: Workflow, @Auth sessionUser: SessionUser ): Unit = { - updateWorkflowField(workflow, sessionUser, _.setName(workflow.getName)) + updateWorkflowField(workflow.getWid, sessionUser, WORKFLOW.NAME, workflow.getName) } @POST @@ -718,7 +726,12 @@ class WorkflowResource extends LazyLogging { workflow: Workflow, @Auth sessionUser: SessionUser ): Unit = { - updateWorkflowField(workflow, sessionUser, _.setDescription(workflow.getDescription)) + updateWorkflowField( + workflow.getWid, + sessionUser, + WORKFLOW.DESCRIPTION, + workflow.getDescription + ) } @PUT @@ -728,9 +741,7 @@ class WorkflowResource extends LazyLogging { if (!WorkflowAccessResource.hasWriteAccess(wid, user.getUid)) { throw new ForbiddenException(s"You do not have permission to modify workflow $wid") } - val workflow: Workflow = workflowDao.fetchOneByWid(wid) - workflow.setIsPublic(true) - workflowDao.update(workflow) + WorkflowPublishService.publish(wid) } @PUT @@ -740,9 +751,66 @@ class WorkflowResource extends LazyLogging { if (!WorkflowAccessResource.hasWriteAccess(wid, user.getUid)) { throw new ForbiddenException(s"You do not have permission to modify workflow $wid") } - val workflow: Workflow = workflowDao.fetchOneByWid(wid) - workflow.setIsPublic(false) - workflowDao.update(workflow) + WorkflowPublishService.unpublish(wid) + } + + /** + * Pins the author's current version as the public copy, so later edits stop reaching the public. + * Also how a pin moves forward, which is the only way edits become public while one is in place. + */ + @POST + @Produces(Array(MediaType.APPLICATION_JSON)) + @RolesAllowed(Array("REGULAR", "ADMIN")) + @Path("/pin/{wid}") + def pinLatest( + @PathParam("wid") wid: Integer, + @Auth user: SessionUser + ): WorkflowPublishService.PublishStatus = { + requirePublishable(wid, user) + WorkflowPublishService.pinLatest(wid) + } + + /** + * Drops the pin, so the public follows the author's latest again. Guarded on write access, the + * same as pinning: whoever may pin may undo it. + */ + @DELETE + @Produces(Array(MediaType.APPLICATION_JSON)) + @RolesAllowed(Array("REGULAR", "ADMIN")) + @Path("/pin/{wid}") + def unpin( + @PathParam("wid") wid: Integer, + @Auth user: SessionUser + ): WorkflowPublishService.PublishStatus = { + requirePublishable(wid, user) + WorkflowPublishService.unpin(wid) + } + + /** What the share dialog's publish panel reads: published, pinned, and holding edits back. */ + @GET + @Produces(Array(MediaType.APPLICATION_JSON)) + @RolesAllowed(Array("REGULAR", "ADMIN")) + @Path("/publish-status/{wid}") + def getPublishStatus( + @PathParam("wid") wid: Integer, + @Auth user: SessionUser + ): WorkflowPublishService.PublishStatus = { + // Write access rather than read: whether edits are being held back is nobody else's business. + requireWriteAccess(wid, user) + WorkflowPublishService.statusOf(wid) + } + + private def requireWriteAccess(wid: Integer, user: SessionUser): Unit = + if (!WorkflowAccessResource.hasWriteAccess(wid, user.getUid)) { + throw new ForbiddenException(s"You do not have permission to modify workflow $wid") + } + + /** What the pin endpoints need: writable by this user, and published in the first place. */ + private def requirePublishable(wid: Integer, user: SessionUser): Unit = { + requireWriteAccess(wid, user) + if (!WorkflowAccessResource.isPublic(wid)) { + throw new BadRequestException(s"Workflow $wid is not published") + } } /** 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 new file mode 100644 index 00000000000..e0e4c8b92cc --- /dev/null +++ b/amber/src/test/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowPublishSpec.scala @@ -0,0 +1,587 @@ +/* + * 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.auth.SessionUser +import org.apache.texera.dao.MockTexeraDB +import org.apache.texera.dao.jooq.generated.Tables.{WORKFLOW, WORKFLOW_USER_ACCESS} +import org.apache.texera.dao.jooq.generated.enums.{DefaultViewEnum, PrivilegeEnum, UserRoleEnum} +import org.apache.texera.dao.jooq.generated.tables.daos.{ + UserDao, + WorkflowDao, + WorkflowUserAccessDao +} +import org.apache.texera.dao.jooq.generated.tables.pojos.{User, Workflow, WorkflowUserAccess} +import org.scalatest.BeforeAndAfterAll +import org.scalatest.flatspec.AnyFlatSpec +import org.scalatest.matchers.should.Matchers + +import org.jooq.{ExecuteContext, ExecuteListener} +import org.jooq.impl.{DefaultConfiguration, DefaultExecuteListenerProvider} + +import java.time.OffsetDateTime +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. + * + * Only the state itself is covered here: nothing serves the pinned copy to a reader yet, so the + * assertions are about which copy each operation leaves stored. + */ +class WorkflowPublishSpec + extends AnyFlatSpec + with BeforeAndAfterAll + with Matchers + with MockTexeraDB { + + private val exampleCreationTime = OffsetDateTime.parse("2025-01-01T00:00:00Z") + + private def makeUser(uid: Int, name: String): User = { + val user = new User + user.setUid(Integer.valueOf(uid)) + user.setName(name) + user.setEmail(s"$name@example.com") + user.setRole(UserRoleEnum.ADMIN) + user.setComment("test") + user.setAccountCreationTime(exampleCreationTime) + user + } + + /** The author. */ + private val owner = makeUser(1, "publish_owner") + + /** A stranger: no access of their own, so nothing about this workflow is theirs to change. */ + private val stranger = makeUser(2, "publish_stranger") + + private val ownerSession = new SessionUser(owner) + private val strangerSession = new SessionUser(stranger) + + private val workflowResource = new WorkflowResource() + + private val publishedContent = """{"operators":[],"note":"content_as_published"}""" + private val editedContent = """{"operators":[],"note":"content_only_a_draft"}""" + + private def workflowDao = new WorkflowDao(getDSLContext.configuration()) + + override protected def beforeAll(): Unit = { + initializeDBAndReplaceDSLContext() + val userDao = new UserDao(getDSLContext.configuration()) + userDao.insert(owner) + userDao.insert(stranger) + } + + override protected def afterAll(): Unit = shutdownDB() + + /** Creates a workflow owned by `owner` holding [[publishedContent]]. */ + private def createWorkflow(name: String): Integer = { + val workflow = new Workflow() + workflow.setName(name) + workflow.setDescription("a workflow") + workflow.setContent(publishedContent) + workflowResource.createWorkflow(workflow, ownerSession).workflow.getWid + } + + /** + * Publishes and pins in one step, which is the state most of these tests are about. Publishing on + * its own leaves the workflow following the author's latest; pinning is what freezes a copy. + */ + private def publishPinned(wid: Integer): WorkflowPublishService.PublishStatus = { + workflowResource.makePublic(wid, ownerSession) + workflowResource.pinLatest(wid, ownerSession) + } + + /** Saves `content` as the author's working copy, the way an autosave would. */ + private def edit(wid: Integer, content: String): Unit = { + val workflow = workflowDao.fetchOneByWid(wid) + workflow.setContent(content) + workflowResource.persistWorkflow(workflow, ownerSession) + } + + /** Renames and re-describes the author's working copy, the way the dashboard does. */ + private def relabel(wid: Integer, name: String, description: String): Unit = { + val workflow = workflowDao.fetchOneByWid(wid) + workflow.setName(name) + workflow.setDescription(description) + workflowResource.persistWorkflow(workflow, ownerSession) + } + + /** + * Runs `interleaved` in the last moment before `act` sends its own write, which is where a second + * request slips in unnoticed. Driven off the statement itself rather than off a thread, so the + * ordering is the same on every run. + */ + /** + * An UPDATE of the workflow table itself, whatever quoting and spacing jOOQ renders it with -- + * and not one of workflow_version or the access tables, since matching those would let a later + * change to one of these paths interleave at the wrong moment and leave the test passing for the + * wrong reason. A rendering this fails to recognise turns the test red rather than skipping it, + * because [[interleaving]] insists that something was interleaved. + */ + private val aWorkflowUpdate = + """(?is)\s*update\s+(?:\S*\.)?["`\[]?workflow["`\]]?\s+set\b.*""".r + + private def interleaving(interleaved: () => Unit)(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)) { + pending = false + interleaved() + } + } + })) + try act + finally configuration.set(previousListeners: _*) + withClue("nothing was interleaved, so this proves nothing: ") { pending shouldBe false } + } + + private def statusOf(wid: Integer): WorkflowPublishService.PublishStatus = + workflowResource.getPublishStatus(wid, ownerSession) + + /** Grants `stranger` explicit access, which makes them a collaborator rather than an outsider. */ + private def grantAccess(wid: Integer, privilege: PrivilegeEnum): Unit = + new WorkflowUserAccessDao(getDSLContext.configuration()) + .insert(new WorkflowUserAccess(stranger.getUid, wid, privilege)) + + private def revokeAccess(wid: Integer): Unit = + getDSLContext + .deleteFrom(WORKFLOW_USER_ACCESS) + .where(WORKFLOW_USER_ACCESS.WID.eq(wid).and(WORKFLOW_USER_ACCESS.UID.eq(stranger.getUid))) + .execute() + + behavior of "publishing" + + it should "follow the author's latest by default" in { + val wid = createWorkflow("publish_follows_latest") + workflowResource.makePublic(wid, ownerSession) + + val status = statusOf(wid) + status.isPublished shouldBe true + status.isPinned shouldBe false + // Nothing is frozen, so nothing is held back however much the author edits. + status.hasUnpublishedChanges shouldBe false + workflowDao.fetchOneByWid(wid).getPublishedContent shouldBe null + + edit(wid, editedContent) + statusOf(wid).hasUnpublishedChanges shouldBe false + } + + it should "pin the current version as the public copy" in { + val wid = createWorkflow("pins_current_version") + val status = publishPinned(wid) + + status.isPublished shouldBe true + status.isPinned shouldBe true + status.hasUnpublishedChanges shouldBe false + workflowDao.fetchOneByWid(wid).getPublishedContent shouldBe publishedContent + } + + it should "pin a workflow that has no description" in { + // description is nullable and the constraint does not ask for published_description, so a + // workflow saved without one has to pin like any other rather than fail on the way in. + val workflow = new Workflow() + workflow.setName("pins_without_a_description") + workflow.setContent(publishedContent) + val wid = workflowResource.createWorkflow(workflow, ownerSession).workflow.getWid + + val status = publishPinned(wid) + + status.isPinned shouldBe true + status.hasUnpublishedChanges shouldBe false + val stored = workflowDao.fetchOneByWid(wid) + stored.getDescription shouldBe null + stored.getPublishedDescription shouldBe null + + // ...and writing one afterwards is an unpublished change like any other. + relabel(wid, "pins_without_a_description", "described later") + statusOf(wid).hasUnpublishedChanges shouldBe true + } + + it should "follow the author's latest again once the pin is dropped" in { + val wid = createWorkflow("unpin_follows_latest") + publishPinned(wid) + edit(wid, editedContent) + + val status = workflowResource.unpin(wid, ownerSession) + + status.isPublished shouldBe true + status.isPinned shouldBe false + status.hasUnpublishedChanges shouldBe false + // Still public; only the frozen copy is gone. + val stored = workflowDao.fetchOneByWid(wid) + stored.getIsPublic shouldBe true + stored.getPublishedContent shouldBe null + } + + it should "freeze the default view with the copy" in { + // The form's definition rides inside the content, so pinning a canvas version and then switching + // the workflow to the form view would otherwise leave the public opening a form that the frozen + // copy does not contain. + val wid = createWorkflow("view_freezes_with_the_copy") + publishPinned(wid) + + getDSLContext + .update(WORKFLOW) + .set(WORKFLOW.DEFAULT_VIEW, DefaultViewEnum.FORM) + .where(WORKFLOW.WID.eq(wid)) + .execute() + + val stored = workflowDao.fetchOneByWid(wid) + stored.getDefaultView shouldBe DefaultViewEnum.FORM + stored.getPublishedDefaultView shouldBe DefaultViewEnum.CANVAS + } + + it should "clear the frozen default view when the pin is dropped" in { + val wid = createWorkflow("view_clears_with_the_pin") + publishPinned(wid) + workflowDao.fetchOneByWid(wid).getPublishedDefaultView shouldBe DefaultViewEnum.CANVAS + + workflowResource.unpin(wid, ownerSession) + + workflowDao.fetchOneByWid(wid).getPublishedDefaultView shouldBe null + } + + it should "leave the pinned copy untouched when the author edits afterwards" in { + val wid = createWorkflow("edit_stays_private") + publishPinned(wid) + + edit(wid, editedContent) + + val stored = workflowDao.fetchOneByWid(wid) + // The author's own working copy has moved on... + stored.getContent shouldBe editedContent + // ...but the copy that was frozen has not. + stored.getPublishedContent shouldBe publishedContent + statusOf(wid).hasUnpublishedChanges shouldBe true + } + + it should "pin the save that lands while it is pinning, not the version before it" in { + // 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. + val wid = createWorkflow("pin_takes_the_row_as_it_stands") + workflowResource.makePublic(wid, ownerSession) + + interleaving(() => edit(wid, editedContent)) { + workflowResource.pinLatest(wid, ownerSession) + } + + workflowDao.fetchOneByWid(wid).getPublishedContent shouldBe editedContent + statusOf(wid).hasUnpublishedChanges shouldBe false + } + + it should "move the pin forward to the author's current version" in { + val wid = createWorkflow("repin_updates_public") + publishPinned(wid) + edit(wid, editedContent) + + val status = workflowResource.pinLatest(wid, ownerSession) + + status.isPinned shouldBe true + status.hasUnpublishedChanges shouldBe false + workflowDao.fetchOneByWid(wid).getPublishedContent shouldBe editedContent + } + + it should "count a rename as an unpublished change" in { + // The pin freezes the title too, so the public is still being shown the old one -- the panel has + // to say so, or the author reads "nothing held back" while the hub disagrees with their editor. + val wid = createWorkflow("rename_counts_as_drift") + publishPinned(wid) + statusOf(wid).hasUnpublishedChanges shouldBe false + + relabel(wid, "renamed_after_pinning", "a workflow") + + statusOf(wid).hasUnpublishedChanges shouldBe true + } + + it should "count a description edit as an unpublished change" in { + val wid = createWorkflow("description_counts_as_drift") + publishPinned(wid) + + relabel(wid, "description_counts_as_drift", "rewritten after pinning") + + statusOf(wid).hasUnpublishedChanges shouldBe true + } + + it should "count a change of view as an unpublished change" in { + val wid = createWorkflow("view_counts_as_drift") + publishPinned(wid) + + getDSLContext + .update(WORKFLOW) + .set(WORKFLOW.DEFAULT_VIEW, DefaultViewEnum.FORM) + .where(WORKFLOW.WID.eq(wid)) + .execute() + + statusOf(wid).hasUnpublishedChanges shouldBe true + } + + it should "report no unpublished changes when an edit is undone" in { + val wid = createWorkflow("undo_clears_badge") + publishPinned(wid) + + edit(wid, editedContent) + statusOf(wid).hasUnpublishedChanges shouldBe true + + edit(wid, publishedContent) + statusOf(wid).hasUnpublishedChanges shouldBe false + } + + it should "report no unpublished changes when the same graph comes back rearranged" in { + // The two copies travel by different routes, and the editor is free to hand back the same graph + // with its keys in another order. Reporting that as an edit is an alarm the author cannot clear. + val wid = createWorkflow("reformat_is_not_an_edit") + publishPinned(wid) + + edit(wid, """{ "note":"content_as_published", "operators": [] }""") + + statusOf(wid).hasUnpublishedChanges shouldBe false + } + + it should "do nothing when unpinning a workflow that is following" in { + // The endpoint is reachable whatever the dialog shows, and asking for the state it is already in + // is not an error -- it just has nothing to clear. + val wid = createWorkflow("unpin_while_following") + workflowResource.makePublic(wid, ownerSession) + + val status = workflowResource.unpin(wid, ownerSession) + + status.isPublished shouldBe true + status.isPinned shouldBe false + workflowDao.fetchOneByWid(wid).getIsPublic shouldBe true + } + + it should "leave a pin alone when the workflow is published again" in { + // Publishing is an on/off switch and this one is already on, so it has nothing to turn: the + // frozen copy is not quietly dropped underneath a public that is reading it. + val wid = createWorkflow("republish_keeps_the_pin") + publishPinned(wid) + + workflowResource.makePublic(wid, ownerSession) + + val stored = workflowDao.fetchOneByWid(wid) + stored.getIsPublic shouldBe true + stored.getPublishedContent shouldBe publishedContent + statusOf(wid).isPinned shouldBe true + } + + it should "report drift rather than fail when a copy is not valid JSON" in { + // content is free text as far as the database is concerned, so the comparison has to survive a + // blob it cannot parse. Falling back to "these differ" is the safe direction: the author is told + // the public is behind, rather than the dialog throwing at them. + val wid = createWorkflow("unparsable_content") + publishPinned(wid) + + getDSLContext + .update(WORKFLOW) + .set(WORKFLOW.CONTENT, "not json at all") + .where(WORKFLOW.WID.eq(wid)) + .execute() + + statusOf(wid).hasUnpublishedChanges shouldBe true + } + + it should "drop the pinned copy on unpublish" in { + val wid = createWorkflow("unpublish_clears_pin") + publishPinned(wid) + + workflowResource.makePrivate(wid, ownerSession) + + val stored = workflowDao.fetchOneByWid(wid) + stored.getIsPublic shouldBe false + stored.getPublishedContent shouldBe null + } + + it should "not resurrect the previous pin after unpublish and re-publish" in { + val wid = createWorkflow("unpublish_then_publish") + publishPinned(wid) + edit(wid, editedContent) + workflowResource.makePrivate(wid, ownerSession) + + // Publishing again starts in the following state; the copy that used to be public is gone. + workflowResource.makePublic(wid, ownerSession) + + statusOf(wid).isPinned shouldBe false + workflowDao.fetchOneByWid(wid).getPublishedContent shouldBe null + } + + it should "publish a workflow that is created already public" in { + val workflow = new Workflow() + workflow.setName("created_public") + workflow.setDescription("a workflow") + workflow.setContent(publishedContent) + workflow.setIsPublic(true) + val wid = workflowResource.createWorkflow(workflow, ownerSession).workflow.getWid + + // Asking for a public workflow up front lands in the same following state as any other new + // public workflow, rather than being pinned by surprise. + val stored = workflowDao.fetchOneByWid(wid) + stored.getIsPublic shouldBe true + stored.getPublishedContent shouldBe null + } + + it should "reject publishing by a user without write access" in { + val wid = createWorkflow("publish_requires_write") + a[ForbiddenException] should be thrownBy workflowResource.makePublic(wid, strangerSession) + } + + it should "refuse to pin, unpin or report status without write access" in { + val wid = createWorkflow("pin_requires_write") + publishPinned(wid) + a[ForbiddenException] should be thrownBy workflowResource.pinLatest(wid, strangerSession) + a[ForbiddenException] should be thrownBy workflowResource.unpin(wid, strangerSession) + a[ForbiddenException] should be thrownBy workflowResource.getPublishStatus(wid, strangerSession) + } + + it should "reject pinning and unpinning a workflow that is not published" in { + val wid = createWorkflow("pin_requires_published") + a[BadRequestException] should be thrownBy workflowResource.pinLatest(wid, ownerSession) + a[BadRequestException] should be thrownBy workflowResource.unpin(wid, ownerSession) + } + + it should "answer 404 for every operation on a workflow that does not exist" in { + // Asked of the service rather than the endpoints: a missing workflow has no access row either, + // so the endpoints answer 403 first and never reach these. 404 is the service's own contract. + val missing = Integer.valueOf(987654) + a[NotFoundException] should be thrownBy WorkflowPublishService.publish(missing) + a[NotFoundException] should be thrownBy WorkflowPublishService.pinLatest(missing) + a[NotFoundException] should be thrownBy WorkflowPublishService.unpin(missing) + a[NotFoundException] should be thrownBy WorkflowPublishService.unpublish(missing) + a[NotFoundException] should be thrownBy WorkflowPublishService.statusOf(missing) + } + + behavior of "saving a published workflow" + + it should "not roll back a publish that lands while a save is in flight" in { + // A save used to carry `is_public` along. An editor open since before the workflow was + // published holds a snapshot saying private, and saving it put that back -- taking a pinned + // workflow private underneath its own frozen copy, which the database refuses outright, so the + // author was left with an editor that could no longer save. The save no longer names the column. + val wid = createWorkflow("save_cannot_roll_back_publish") + + // The snapshot an editor opened before any of this was published. + val stale = workflowDao.fetchOneByWid(wid) + stale.getIsPublic shouldBe false + + publishPinned(wid) + + stale.setContent("""{"operators":[],"note":"from_a_stale_client"}""") + workflowResource.persistWorkflow(stale, ownerSession) + + // The save went through, and it moved the working copy only. + val stored = workflowDao.fetchOneByWid(wid) + stored.getContent shouldBe """{"operators":[],"note":"from_a_stale_client"}""" + stored.getIsPublic shouldBe true + stored.getPublishedContent shouldBe publishedContent + } + + it should "not let a save change the publish state" in { + val wid = createWorkflow("save_cannot_publish") + publishPinned(wid) + + // A stale or hostile client sending the whole POJO back with the publish columns rewritten. + val tampered = workflowDao.fetchOneByWid(wid) + tampered.setContent(editedContent) + tampered.setIsPublic(false) + tampered.setPublishedContent(editedContent) + workflowResource.persistWorkflow(tampered, ownerSession) + + val stored = workflowDao.fetchOneByWid(wid) + stored.getIsPublic shouldBe true + stored.getPublishedContent shouldBe publishedContent + } + + it should "not let a collaborator's save change the publish state" in { + val wid = createWorkflow("collaborator_cannot_publish") + publishPinned(wid) + grantAccess(wid, PrivilegeEnum.WRITE) + + try { + val tampered = workflowDao.fetchOneByWid(wid) + tampered.setContent(editedContent) + tampered.setIsPublic(false) + tampered.setPublishedContent(editedContent) + workflowResource.persistWorkflow(tampered, strangerSession) + + val stored = workflowDao.fetchOneByWid(wid) + stored.getIsPublic shouldBe true + stored.getPublishedContent shouldBe publishedContent + } finally revokeAccess(wid) + } + + it should "not let a rename undo a publish that lands first" in { + // A rename used to read the whole row and write it all back, so a publish landing in that window + // was reverted to what the read had seen: the author pressed Public, was told it worked, and the + // workflow was private again. + val wid = createWorkflow("rename_cannot_undo_publish") + + interleaving(() => publishPinned(wid)) { + val body = new Workflow() + body.setWid(wid) + body.setName("renamed_during_a_publish") + workflowResource.updateWorkflowName(body, ownerSession) + } + + val stored = workflowDao.fetchOneByWid(wid) + stored.getName shouldBe "renamed_during_a_publish" + stored.getIsPublic shouldBe true + stored.getPublishedContent shouldBe publishedContent + } + + it should "not let a rename put an unpublished workflow back on show" in { + // The same window, the other way round, and the one that matters: the author takes the workflow + // down, and a rename in flight restores the row as it was -- public, still carrying the frozen + // copy the public had been reading. + val wid = createWorkflow("rename_cannot_republish") + publishPinned(wid) + + interleaving(() => workflowResource.makePrivate(wid, ownerSession)) { + val body = new Workflow() + body.setWid(wid) + body.setName("renamed_during_an_unpublish") + workflowResource.updateWorkflowName(body, ownerSession) + } + + val stored = workflowDao.fetchOneByWid(wid) + stored.getName shouldBe "renamed_during_an_unpublish" + stored.getIsPublic shouldBe false + stored.getPublishedContent shouldBe null + } + + it should "not let a rename change the publish state" in { + val wid = createWorkflow("rename_cannot_publish") + publishPinned(wid) + + val tampered = workflowDao.fetchOneByWid(wid) + tampered.setName("renamed") + tampered.setIsPublic(false) + tampered.setPublishedContent(editedContent) + workflowResource.updateWorkflowName(tampered, ownerSession) + + val stored = workflowDao.fetchOneByWid(wid) + stored.getName shouldBe "renamed" + stored.getIsPublic shouldBe true + stored.getPublishedContent shouldBe publishedContent + } +} From bf743fbd0d41ffb17f1e5643147deb8944664a25 Mon Sep 17 00:00:00 2001 From: Yang Zhang Date: Sat, 22 Aug 2026 14:54:23 -0700 Subject: [PATCH 2/6] feat(workflow): serve the pinned copy to public viewers With a version pinned, a workflow has two copies: the author's working copy and the frozen one on public show. This routes every read that serves a viewer without granted access through the frozen copy, and freezes the name and description with the graph. `WorkflowPublishService.publicCopyOf` returns the three fields as a group, so a surface cannot pick up the published graph under a title the author has not published; `WorkflowAccessResource.hasGrantedAccess` is the seam that decides which copy a caller gets. Granted access -- owner, shared, project member -- keeps tracking the author's latest, because sharing is not publishing. Name and description freeze because they are as public as the graph: if only the graph froze, a report about a title could be answered by editing the title while the pinned copy still advertised it. Routed through it: opening a workflow, the hub's read, Clone, Duplicate, `/workflow_name`, `/workflow_description` and the size a listing shows. A workflow that follows the author's latest -- every workflow today -- is served exactly what it is served now. Co-Authored-By: Claude Opus 5 --- .../workflow/WorkflowAccessResource.scala | 9 + .../workflow/WorkflowPublishService.scala | 62 +++- .../user/workflow/WorkflowResource.scala | 130 ++++--- .../workflow/WorkflowVersionResource.scala | 17 +- .../user/workflow/WorkflowPublishSpec.scala | 343 +++++++++++++++++- 5 files changed, 502 insertions(+), 59 deletions(-) diff --git a/amber/src/main/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowAccessResource.scala b/amber/src/main/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowAccessResource.scala index 09353d257f9..8f975f90413 100644 --- a/amber/src/main/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowAccessResource.scala +++ b/amber/src/main/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowAccessResource.scala @@ -92,6 +92,15 @@ object WorkflowAccessResource { } } + /** + * Whether the user was granted access to the workflow itself, rather than merely being able to + * read it because it is public. Granted access sees the author's working copy; public access is + * held at the pinned copy while one is pinned. + */ + def hasGrantedAccess(wid: Integer, uid: Integer): Boolean = { + !getPrivilege(wid, uid).eq(PrivilegeEnum.NONE) + } + def isPublic(wid: Integer): Boolean = { context .select(WORKFLOW.IS_PUBLIC) 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 46b4548e7ad..e7306ad9b38 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 @@ -181,15 +181,61 @@ object WorkflowPublishService extends LazyLogging { PublishStatus( isPublished = workflow.getIsPublic, isPinned = pinned, - // Literally "what the public sees is not what you have". Every field the pin freezes counts: - // a rename the public cannot see is held back exactly as an edit to the graph is. Compared on - // values rather than on a version id, so that an edit and its undo report nothing held back. - hasUnpublishedChanges = pinned && ( - !sameContent(workflow.getPublishedContent, workflow.getContent) || - workflow.getPublishedName != workflow.getName || - workflow.getPublishedDescription != workflow.getDescription || - workflow.getPublishedDefaultView != workflow.getDefaultView + // 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)) + ) + } + + /** + * Every field of the copy, so that a rename the public cannot see is held back exactly as an edit + * to the graph is. Content is compared as a tree: a restore can rearrange whitespace, and calling + * that drift alarms nobody. + */ + private def differs(public: PublicCopy, working: PublicCopy): Boolean = + public.name != working.name || + public.description != working.description || + public.defaultView != working.defaultView || + !sameContent(public.content, working.content) + + /** + * Everything about a workflow that is on public show, carried together so that a caller cannot + * serve the frozen graph under the author's live title, or open the author's chosen view on a + * copy that does not contain it. + */ + case class PublicCopy( + name: String, + description: String, + content: String, + defaultView: DefaultViewEnum + ) + + /** What every public surface must serve, as a group so no field is the one that gets forgotten. */ + def publicCopyOf(workflow: Workflow): PublicCopy = + if (workflow.getPublishedContent == null) workingCopyOf(workflow) + else + PublicCopy( + workflow.getPublishedName, + workflow.getPublishedDescription, + workflow.getPublishedContent, + workflow.getPublishedDefaultView ) + + /** The author's own copy, in the same shape. */ + private def workingCopyOf(workflow: Workflow): PublicCopy = + PublicCopy( + workflow.getName, + workflow.getDescription, + workflow.getContent, + workflow.getDefaultView ) + + /** As [[publicCopyOf]], for callers holding only a wid. 404s unless the workflow is public. */ + def publicCopyOf(wid: Integer): PublicCopy = { + val workflow = requireWorkflow(wid) + if (!workflow.getIsPublic) { + throw new NotFoundException(s"Workflow $wid is not public") + } + publicCopyOf(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 2b8f738376c..6337715293c 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 @@ -96,11 +96,7 @@ object WorkflowResource { private def insertWorkflow(workflow: Workflow, user: User): Unit = { // A workflow is born with nothing pinned. The endpoint takes a whole Workflow, so without this // a request body could seed a published copy of its own choosing. - workflow.setPublishedVersionId(null) - workflow.setPublishedContent(null) - workflow.setPublishedName(null) - workflow.setPublishedDescription(null) - workflow.setPublishedDefaultView(null) + clearPublishState(workflow) workflowDao.insert(workflow) workflowOfUserDao.insert(new WorkflowOfUser(user.getUid, workflow.getWid)) workflowUserAccessDao.insert( @@ -144,11 +140,19 @@ object WorkflowResource { case class WorkflowIDs(wids: List[Integer]) + /** Clears every column that describes a pinned public copy. */ + private def clearPublishState(workflow: Workflow): Unit = { + workflow.setPublishedVersionId(null) + workflow.setPublishedContent(null) + workflow.setPublishedName(null) + workflow.setPublishedDescription(null) + workflow.setPublishedDefaultView(null) + } + /** - * 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. + * A workflow POJO for the copy-producing paths (clone, duplicate, restore-a-version). Copies start + * unpublished, and setters rather than the positional constructor keep a new column from silently + * shifting a null into the wrong field. */ def newUnpublishedWorkflow( name: String, @@ -162,9 +166,46 @@ object WorkflowResource { workflow.setContent(content) workflow.setIsPublic(false) workflow.setDefaultView(defaultView) + clearPublishState(workflow) workflow } + /** + * The copy a viewer may see, every field at once: a user granted access to the workflow itself, + * as its owner or through a share, sees the author's working copy; everyone else is here only + * because the workflow is public, and gets the public copy. Taken as a group so a copy cannot end + * up carrying the published graph under a title the author never published. + */ + private def copyVisibleTo(workflow: Workflow, uid: Integer): WorkflowPublishService.PublicCopy = + if (uid != null && WorkflowAccessResource.hasGrantedAccess(workflow.getWid, uid)) { + WorkflowPublishService.PublicCopy( + workflow.getName, + workflow.getDescription, + workflow.getContent, + workflow.getDefaultView + ) + } else { + WorkflowPublishService.publicCopyOf(workflow) + } + + /** + * One column as the public sees it: the frozen value while a pin is in place, the live one + * otherwise. Keyed on the pin rather than on the frozen value being non-null, so a pinned + * workflow can never fall through to what the author is editing. + */ + private def publicField( + wid: Integer, + frozen: TableField[_, String], + live: TableField[_, String] + ) = + Option( + context + .select(WORKFLOW.PUBLISHED_CONTENT, frozen, live) + .from(WORKFLOW) + .where(WORKFLOW.WID.eq(wid)) + .fetchOne() + ).map(row => if (row.value1() != null) row.value2() else row.value3()).orNull + /** * Writes one field of a workflow, and only that field. The endpoints take a whole `Workflow` * from the client, of which exactly two things are used: which workflow, and the new value. @@ -434,16 +475,20 @@ class WorkflowResource extends LazyLogging { ): WorkflowWithPrivilege = { if (WorkflowAccessResource.hasReadAccess(wid, user.getUid)) { val workflow = workflowDao.fetchOneByWid(wid) + // A user who only reaches this workflow because it is public is served the pinned copy, not + // the author's in-progress edits -- all of it, so the graph cannot arrive under a title the + // author has not published, or in a view the frozen graph does not support. + val visible = copyVisibleTo(workflow, user.getUid) WorkflowWithPrivilege( - workflow.getName, - workflow.getDescription, + visible.name, + visible.description, workflow.getWid, - workflow.getContent, + visible.content, workflow.getCreationTime, workflow.getLastModifiedTime, workflow.getIsPublic, !WorkflowAccessResource.hasWriteAccess(wid, user.getUid), - workflow.getDefaultView + visible.defaultView ) } else { throw new ForbiddenException("No sufficient access privilege.") @@ -544,13 +589,16 @@ class WorkflowResource extends LazyLogging { context.transaction { txConfig => for (wid <- workflowIDs.wids) { val oldWorkflow: Workflow = workflowDao.fetchOneByWid(wid) + // Reached only because it is public? Then the copy is of the published version, title and + // description included. + val source = copyVisibleTo(oldWorkflow, user.getUid) val newWorkflow = createWorkflow( newUnpublishedWorkflow( - oldWorkflow.getName + "_copy", - oldWorkflow.getDescription, - assignNewOperatorIds(oldWorkflow.getContent), - // the default view is part of the workflow, so a copy keeps it - oldWorkflow.getDefaultView + source.name + "_copy", + source.description, + assignNewOperatorIds(source.content), + // the default view is part of the copy being taken, so it comes from the same source + source.defaultView ), sessionUser ) @@ -579,13 +627,16 @@ class WorkflowResource extends LazyLogging { throw new ForbiddenException("No sufficient access privilege.") } val oldWorkflow: Workflow = workflowDao.fetchOneByWid(wid) + // The hub shows the public copy, so Clone copies that -- for the author too, who already has + // their latest in the editor. For a private workflow this is the author's own copy. + val source = WorkflowPublishService.publicCopyOf(oldWorkflow) val newWorkflow: DashboardWorkflow = createWorkflow( newUnpublishedWorkflow( - oldWorkflow.getName + "_clone", - oldWorkflow.getDescription, - assignNewOperatorIds(oldWorkflow.getContent), - // a biologist's path is hub -> clone -> use, so the clone must stay usable - oldWorkflow.getDefaultView + source.name + "_clone", + source.description, + assignNewOperatorIds(source.content), + // a biologist's path is hub -> clone -> use, so the clone keeps the view of what they saw + source.defaultView ), sessionUser ) @@ -941,15 +992,10 @@ class WorkflowResource extends LazyLogging { @GET @Path("/workflow_name") def getWorkflowName(@QueryParam("wid") wid: Integer): String = { - context - .select( - WORKFLOW.NAME - ) - .from(WORKFLOW) - .where(WORKFLOW.WID.eq(wid)) - .fetchOneInto(classOf[String]) + publicField(wid, WORKFLOW.PUBLISHED_NAME, WORKFLOW.NAME) } + /** The hub's public view of a workflow: the pinned version if one is pinned, the latest if not. */ @GET @Path("/publicised/{wid}") def retrievePublicWorkflow( @@ -960,29 +1006,27 @@ class WorkflowResource extends LazyLogging { .where(WORKFLOW.WID.eq(wid)) .and(WORKFLOW.IS_PUBLIC.isTrue) .fetchOne() + // 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])) WorkflowWithPrivilege( - workflow.getName, - workflow.getDescription, + publicCopy.name, + publicCopy.description, workflow.getWid, - workflow.getContent, + publicCopy.content, workflow.getCreationTime, workflow.getLastModifiedTime, workflow.getIsPublic, readonly = true, - defaultView = workflow.getDefaultView + // The view freezes with the copy: the form's definition lives inside the content. + defaultView = publicCopy.defaultView ) } @GET @Path("/workflow_description") def getWorkflowDescription(@QueryParam("wid") wid: Integer): String = { - context - .select( - WORKFLOW.DESCRIPTION - ) - .from(WORKFLOW) - .where(WORKFLOW.WID.eq(wid)) - .fetchOneInto(classOf[String]) + publicField(wid, WORKFLOW.PUBLISHED_DESCRIPTION, WORKFLOW.DESCRIPTION) } //TODO Get size from database @@ -998,7 +1042,9 @@ class WorkflowResource extends LazyLogging { .fetch() .asScala .foreach { wf => - result.put(wf.getWid, wf.getContent.length) + // Sized by the copy on show, so the number does not move when the author edits privately. + val onShow = WorkflowPublishService.publicCopyOf(wf.into(classOf[Workflow])) + result.put(wf.getWid, onShow.content.length) } } result 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 3a0a5b61f5d..e84dde72fee 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 @@ -54,6 +54,19 @@ object WorkflowVersionResource { .createDSLContext() private def workflowVersionDao = new WorkflowVersionDao(context.configuration) private def workflowDao = new WorkflowDao(context.configuration) + + /** + * Whether this user may read the workflow's revision history. + * + * Read access is enough while nothing is pinned: the public copy is then the author's latest, and + * its history is the history of what everyone can already see. A pin changes that -- replaying a + * version folds deltas back from the author's *current* content, so handing the history to a + * public viewer would hand them the edits the pin is holding back. + */ + private def canReadHistory(wid: Integer, uid: Integer): Boolean = + WorkflowAccessResource.hasGrantedAccess(wid, uid) || + (WorkflowAccessResource.hasReadAccess(wid, uid) && + Option(workflowDao.fetchOneByWid(wid)).forall(_.getPublishedContent == null)) // constant to indicate versions should be aggregated if they are within the specified time limit private final val AGGREGATE_TIME_LIMIT_MILLSEC = UserSystemConfig.workflowVersionCollapseIntervalInMinutes * 60000 @@ -350,7 +363,7 @@ class WorkflowVersionResource { @Auth sessionUser: SessionUser ): List[VersionEntry] = { val user = sessionUser.getUser - if (!WorkflowAccessResource.hasReadAccess(wid, user.getUid)) { + if (!canReadHistory(wid, user.getUid)) { List() } else { encodeVersionImportance( @@ -385,7 +398,7 @@ class WorkflowVersionResource { @Auth sessionUser: SessionUser ): Workflow = { val user = sessionUser.getUser - if (!WorkflowAccessResource.hasReadAccess(wid, user.getUid)) { + if (!canReadHistory(wid, user.getUid)) { throw new ForbiddenException("No sufficient access privilege.") } else { // fetch all versions equal to and subsequent to the specified version 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 e0e4c8b92cc..dbf3c5eec59 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 @@ -29,22 +29,23 @@ import org.apache.texera.dao.jooq.generated.tables.daos.{ WorkflowUserAccessDao } import org.apache.texera.dao.jooq.generated.tables.pojos.{User, Workflow, WorkflowUserAccess} +import org.apache.texera.web.resource.dashboard.user.workflow.WorkflowResource.WorkflowIDs import org.scalatest.BeforeAndAfterAll import org.scalatest.flatspec.AnyFlatSpec import org.scalatest.matchers.should.Matchers import org.jooq.{ExecuteContext, ExecuteListener} import org.jooq.impl.{DefaultConfiguration, DefaultExecuteListenerProvider} - +import java.lang.reflect.Proxy import java.time.OffsetDateTime +import java.util +import javax.servlet.http.HttpServletRequest 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. - * - * Only the state itself is covered here: nothing serves the pinned copy to a reader yet, so the - * assertions are about which copy each operation leaves stored. + * 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. */ class WorkflowPublishSpec extends AnyFlatSpec @@ -75,6 +76,7 @@ class WorkflowPublishSpec private val strangerSession = new SessionUser(stranger) private val workflowResource = new WorkflowResource() + private val versionResource = new WorkflowVersionResource() private val publishedContent = """{"operators":[],"note":"content_as_published"}""" private val editedContent = """{"operators":[],"note":"content_only_a_draft"}""" @@ -154,7 +156,17 @@ 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. + */ + private def fakeRequest(): HttpServletRequest = + Proxy + .newProxyInstance( + classOf[HttpServletRequest].getClassLoader, + Array[Class[_]](classOf[HttpServletRequest]), + (_: Any, method: java.lang.reflect.Method, _: Array[AnyRef]) => + if (method.getName == "getRemoteAddr") "127.0.0.1" else null + ) + .asInstanceOf[HttpServletRequest] private def statusOf(wid: Integer): WorkflowPublishService.PublishStatus = workflowResource.getPublishStatus(wid, ownerSession) @@ -584,4 +596,321 @@ class WorkflowPublishSpec stored.getIsPublic shouldBe true stored.getPublishedContent shouldBe publishedContent } + + behavior of "name and description" + + it should "freeze the name and description alongside the content" in { + // Malicious text in a description is just as public as the graph, so editing it must not reach + // the public view either -- otherwise a report can be answered by rewording rather than fixing. + val wid = createWorkflow("freeze_metadata") + publishPinned(wid) + + relabel(wid, "renamed_after_publishing", "rewritten after publishing") + + val publicView = workflowResource.retrievePublicWorkflow(wid) + publicView.name shouldBe "freeze_metadata" + publicView.description shouldBe "a workflow" + workflowResource.getWorkflowName(wid) shouldBe "freeze_metadata" + workflowResource.getWorkflowDescription(wid) shouldBe "a workflow" + } + + it should "publish an edited description by moving the pin forward" in { + val wid = createWorkflow("description_publishes_on_repin") + publishPinned(wid) + relabel(wid, "description_publishes_on_repin", "rewritten after publishing") + + workflowResource.pinLatest(wid, ownerSession) + + workflowResource.retrievePublicWorkflow(wid).description shouldBe "rewritten after publishing" + statusOf(wid).hasUnpublishedChanges shouldBe false + } + + behavior of "public read paths" + + it should "show a public viewer the author's latest while nothing is pinned" in { + // Following follows everything a pin would freeze, not just the graph: name and description are + // on public show too, so a viewer must see the live ones or the two halves would disagree. + val wid = createWorkflow("following_serves_latest_to_strangers") + workflowResource.makePublic(wid, ownerSession) + edit(wid, editedContent) + relabel(wid, "renamed_while_following", "described_while_following") + + workflowResource.retrieveWorkflow(wid, strangerSession).content shouldBe editedContent + workflowResource.getWorkflowName(wid) shouldBe "renamed_while_following" + workflowResource.getWorkflowDescription(wid) shouldBe "described_while_following" + + val publicView = workflowResource.retrievePublicWorkflow(wid) + publicView.content shouldBe editedContent + publicView.name shouldBe "renamed_while_following" + publicView.description shouldBe "described_while_following" + } + + it should "open the public view in the pinned copy's own view" in { + // A form's definition lives inside the content, so serving the author's live preference over a + // frozen graph would put a form on a copy that has none. + val wid = createWorkflow("public_view_uses_the_frozen_view") + publishPinned(wid) + + getDSLContext + .update(WORKFLOW) + .set(WORKFLOW.DEFAULT_VIEW, DefaultViewEnum.FORM) + .where(WORKFLOW.WID.eq(wid)) + .execute() + + workflowResource.retrievePublicWorkflow(wid).defaultView shouldBe DefaultViewEnum.CANVAS + // The author keeps their own preference. + workflowDao.fetchOneByWid(wid).getDefaultView shouldBe DefaultViewEnum.FORM + } + + it should "follow the author's view while nothing is pinned" in { + val wid = createWorkflow("public_view_follows_latest") + workflowResource.makePublic(wid, ownerSession) + + getDSLContext + .update(WORKFLOW) + .set(WORKFLOW.DEFAULT_VIEW, DefaultViewEnum.FORM) + .where(WORKFLOW.WID.eq(wid)) + .execute() + + workflowResource.retrievePublicWorkflow(wid).defaultView shouldBe DefaultViewEnum.FORM + } + + it should "serve the published version to a user without granted access" in { + val wid = createWorkflow("read_serves_published") + publishPinned(wid) + edit(wid, editedContent) + + // A stranger reaches this workflow only because it is public. + workflowResource.retrieveWorkflow(wid, strangerSession).content shouldBe publishedContent + // The author keeps seeing their own working copy. + workflowResource.retrieveWorkflow(wid, ownerSession).content shouldBe editedContent + } + + it should "open a pinned workflow under the copy's own title and view, not the author's" in { + // The graph and the label travel together: a public viewer opening this workflow must not get + // the frozen graph under a title the author has not published, and must not be told to open a + // form view over a copy whose content carries no form. + val wid = createWorkflow("open_serves_the_whole_copy") + publishPinned(wid) + edit(wid, editedContent) + relabel(wid, "renamed_after_pinning", "described_after_pinning") + getDSLContext + .update(WORKFLOW) + .set(WORKFLOW.DEFAULT_VIEW, DefaultViewEnum.FORM) + .where(WORKFLOW.WID.eq(wid)) + .execute() + + val asStranger = workflowResource.retrieveWorkflow(wid, strangerSession) + asStranger.content shouldBe publishedContent + asStranger.name shouldBe "open_serves_the_whole_copy" + asStranger.description shouldBe "a workflow" + asStranger.defaultView shouldBe DefaultViewEnum.CANVAS + + // The author opens their own workflow and sees everything they have. + val asOwner = workflowResource.retrieveWorkflow(wid, ownerSession) + asOwner.content shouldBe editedContent + asOwner.name shouldBe "renamed_after_pinning" + asOwner.defaultView shouldBe DefaultViewEnum.FORM + } + + it should "serve the working copy to a collaborator with granted read access" in { + val wid = createWorkflow("collaborator_sees_working_copy") + publishPinned(wid) + edit(wid, editedContent) + grantAccess(wid, PrivilegeEnum.READ) + + try { + // Sharing is not publishing: a collaborator tracks the author's latest content, live. + workflowResource.retrieveWorkflow(wid, strangerSession).content shouldBe editedContent + } finally revokeAccess(wid) + } + + it should "keep serving a collaborator the latest content as the author keeps editing" in { + val wid = createWorkflow("collaborator_tracks_latest") + publishPinned(wid) + grantAccess(wid, PrivilegeEnum.READ) + + try { + val later = """{"operators":[],"note":"later_still"}""" + edit(wid, editedContent) + workflowResource.retrieveWorkflow(wid, strangerSession).content shouldBe editedContent + edit(wid, later) + workflowResource.retrieveWorkflow(wid, strangerSession).content shouldBe later + // ...while the public copy stayed put throughout. + workflowResource.retrievePublicWorkflow(wid).content shouldBe publishedContent + } finally revokeAccess(wid) + } + + it should "not tell a public viewer that the author has unpublished edits" in { + val wid = createWorkflow("draft_state_is_private") + publishPinned(wid) + edit(wid, editedContent) + + a[ForbiddenException] should be thrownBy workflowResource.getPublishStatus(wid, strangerSession) + statusOf(wid).hasUnpublishedChanges shouldBe true + } + + it should "not expose the author's edit history to a public viewer" in { + // The other way into the working copy. Replaying a version folds deltas back from the author's + // *current* content, so a public viewer listing versions and checking one out is handed a draft + // the pin is holding back -- the one thing the frozen copy exists to prevent. + val wid = createWorkflow("history_is_not_public") + publishPinned(wid) + edit(wid, editedContent) + edit(wid, """{"operators":[],"note":"newer_still"}""") + + val ownerVersions = versionResource.retrieveVersionsOfWorkflow(wid, ownerSession) + ownerVersions should not be empty + versionResource.retrieveVersionsOfWorkflow(wid, strangerSession) shouldBe empty + a[ForbiddenException] should be thrownBy + versionResource.retrieveWorkflowVersion(wid, ownerVersions.head.vId, strangerSession) + } + + it should "still expose the history of a public workflow that is not pinned" in { + // Nothing is frozen, so the public copy is the author's latest and its history is the history of + // what everyone can already see. Taking that away would be a change this feature does not need. + val wid = createWorkflow("history_stays_public_while_following") + workflowResource.makePublic(wid, ownerSession) + edit(wid, editedContent) + + val versions = versionResource.retrieveVersionsOfWorkflow(wid, strangerSession) + versions should not be empty + versionResource + .retrieveWorkflowVersion(wid, versions.head.vId, strangerSession) + .getContent should not be empty + } + + it should "still expose the edit history to a collaborator" in { + // Sharing is not publishing: someone granted access tracks the author's latest, history included. + val wid = createWorkflow("history_visible_to_collaborator") + publishPinned(wid) + grantAccess(wid, PrivilegeEnum.READ) + try { + versionResource.retrieveVersionsOfWorkflow(wid, strangerSession) should not be empty + } finally revokeAccess(wid) + } + + it should "refuse to hand out a public copy of a workflow that is not public" in { + // The guard viewers without granted access rely on: no route to a private workflow's content + // may fall through to the public copy just because the caller asked for it by wid. + val wid = createWorkflow("public_copy_requires_public") + a[NotFoundException] should be thrownBy WorkflowPublishService.publicCopyOf(wid) + } + + it should "clone the published version, not the author's latest" in { + val wid = createWorkflow("clone_takes_published") + publishPinned(wid) + edit(wid, editedContent) + + val clonedWid = workflowResource.cloneWorkflow(wid, strangerSession, fakeRequest()) + val cloned = workflowDao.fetchOneByWid(clonedWid) + + cloned.getContent shouldBe publishedContent + // A copy has never been reviewed, so it starts private. + cloned.getIsPublic shouldBe false + } + + it should "clone the published version for the author too" in { + // The hub shows the pinned version, so its Clone button copies that even for the author, whose + // working copy has moved on -- cloning something other than what is on the screen would be the + // surprise, and their latest is already open in the editor. + val wid = createWorkflow("clone_takes_published_for_author") + publishPinned(wid) + edit(wid, editedContent) + + val clonedWid = workflowResource.cloneWorkflow(wid, ownerSession, fakeRequest()) + + workflowDao.fetchOneByWid(clonedWid).getContent shouldBe publishedContent + } + + it should "clone the published name and description, not the edited ones" in { + val wid = createWorkflow("clone_takes_published_metadata") + publishPinned(wid) + relabel(wid, "renamed_after_publishing", "described_after_publishing") + + val cloned = + workflowDao.fetchOneByWid(workflowResource.cloneWorkflow(wid, ownerSession, fakeRequest())) + + cloned.getName shouldBe "clone_takes_published_metadata_clone" + cloned.getDescription shouldBe "a workflow" + } + + it should "still clone the working copy of a workflow that is not public" in { + val wid = createWorkflow("clone_private_takes_working_copy") + edit(wid, editedContent) + + val clonedWid = workflowResource.cloneWorkflow(wid, ownerSession, fakeRequest()) + + workflowDao.fetchOneByWid(clonedWid).getContent shouldBe editedContent + } + + it should "duplicate the published version for a user without granted access" in { + // Title and description too, not just the graph: a copy carrying the published canvas under the + // author's unpublished title would publish the very rename the pin is holding back. + val wid = createWorkflow("duplicate_takes_published") + publishPinned(wid) + edit(wid, editedContent) + relabel(wid, "duplicate_unpublished_name", "unpublished description") + + val duplicated = + workflowResource.duplicateWorkflow(WorkflowIDs(List(wid)), strangerSession) + + duplicated should have size 1 + val copy = workflowDao.fetchOneByWid(duplicated.head.workflow.getWid) + copy.getContent shouldBe publishedContent + copy.getName shouldBe "duplicate_takes_published_copy" + copy.getDescription should not be "unpublished description" + } + + it should "duplicate the owner's own working copy for the owner" in { + val wid = createWorkflow("owner_duplicates_working_copy") + publishPinned(wid) + edit(wid, editedContent) + + val duplicated = workflowResource.duplicateWorkflow(WorkflowIDs(List(wid)), ownerSession) + workflowDao.fetchOneByWid(duplicated.head.workflow.getWid).getContent shouldBe editedContent + } + + it should "start a copy of a published workflow with no publish state of its own" in { + val wid = createWorkflow("copy_starts_clean") + publishPinned(wid) + + val copy = workflowDao.fetchOneByWid( + workflowResource + .duplicateWorkflow(WorkflowIDs(List(wid)), ownerSession) + .head + .workflow + .getWid + ) + + copy.getIsPublic shouldBe false + copy.getPublishedContent shouldBe null + copy.getPublishedName shouldBe null + copy.getPublishedDescription shouldBe null + copy.getPublishedDefaultView shouldBe null + copy.getPublishedVersionId shouldBe null + } + + it should "size a workflow by the copy the caller can see" in { + // Listings show a size next to every card, so it has to describe the copy that card opens: the + // pinned one while a pin is in place, the author's latest otherwise, and a private workflow's + // own content. + val priv = createWorkflow("private_size_uses_content") + edit(priv, editedContent) + workflowResource.getSize(util.Arrays.asList(priv)).get(priv) shouldBe editedContent.length + + val following = createWorkflow("size_follows_latest") + workflowResource.makePublic(following, ownerSession) + edit(following, editedContent + " ") + workflowResource + .getSize(util.Arrays.asList(following)) + .get(following) shouldBe editedContent.length + 5 + + val pinned = createWorkflow("size_uses_published") + publishPinned(pinned) + edit(pinned, editedContent + " ") + workflowResource + .getSize(util.Arrays.asList(pinned)) + .get(pinned) shouldBe publishedContent.length + } } From dbcf2ec3964d75f22a033406f85066b25bc46c04 Mon Sep 17 00:00:00 2001 From: Yang Zhang Date: Sat, 22 Aug 2026 14:57:53 -0700 Subject: [PATCH 3/6] feat(workflow): match search and listings against the copy on show Search and the listings it feeds were reading the author's live columns, which for a pinned workflow is the one copy the public cannot open. A draft would turn up in a public search under a title nobody has seen, and the card would advertise a name the detail page does not show. Each filter is now applied to whichever copy the caller may see: `onVisibleCopy` builds the same filter twice -- over the live columns for rows the caller was granted access to, over the frozen ones for rows they reach only because the workflow is public -- and ORs the two. A disjunction over bare columns rather than a CASE, so each side stays eligible for its own fulltext index. Unpinned public rows fall back to the live columns, so a following workflow searches exactly as it does now. Listings carry two more things from the same query: the frozen name and description to show a viewer without granted access, and whether the copy on show is behind the author's working copy. `constructWhereClause` takes `includePublic` for this; the other builders accept and ignore it. Co-Authored-By: Claude Opus 5 --- .../dashboard/SearchQueryBuilder.scala | 14 +- .../dashboard/UnifiedResourceSchema.scala | 15 +- .../VersionedResourceSearchQueryBuilder.scala | 3 +- .../WorkflowSearchQueryBuilder.scala | 125 +++++++- .../resource/dashboard/hub/HubResource.scala | 19 +- .../workflow/WorkflowPublishService.scala | 49 ++- .../user/workflow/WorkflowResource.scala | 5 +- .../dashboard/UnifiedResourceSchemaSpec.scala | 33 +- .../WorkflowSearchQueryBuilderSpec.scala | 70 ++++- .../user/workflow/WorkflowPublishSpec.scala | 292 +++++++++++++++++- 10 files changed, 586 insertions(+), 39 deletions(-) diff --git a/amber/src/main/scala/org/apache/texera/web/resource/dashboard/SearchQueryBuilder.scala b/amber/src/main/scala/org/apache/texera/web/resource/dashboard/SearchQueryBuilder.scala index c132500625a..76c6e7d9763 100644 --- a/amber/src/main/scala/org/apache/texera/web/resource/dashboard/SearchQueryBuilder.scala +++ b/amber/src/main/scala/org/apache/texera/web/resource/dashboard/SearchQueryBuilder.scala @@ -58,7 +58,17 @@ trait SearchQueryBuilder { includePublic: Boolean = false ): TableLike[_] - protected def constructWhereClause(uid: Integer, params: SearchQueryParams): Condition + /** + * @param includePublic whether public resources the user has not been granted access to are in + * scope. Builders whose content differs between the public and the private + * view (workflows, whose public copy is a pinned version) need this to know + * which copy a filter may match against. + */ + protected def constructWhereClause( + uid: Integer, + params: SearchQueryParams, + includePublic: Boolean = false + ): Condition protected def getGroupByFields: Seq[GroupField] = Seq.empty @@ -78,7 +88,7 @@ trait SearchQueryBuilder { val query: SelectGroupByStep[Record] = context .selectDistinct(mappedResourceSchema.allFields: _*) .from(constructFromClause(uid, params, includePublic)) - .where(constructWhereClause(uid, params)) + .where(constructWhereClause(uid, params, includePublic)) val groupByFields = getGroupByFields if (groupByFields.nonEmpty) { query.groupBy(groupByFields: _*) diff --git a/amber/src/main/scala/org/apache/texera/web/resource/dashboard/UnifiedResourceSchema.scala b/amber/src/main/scala/org/apache/texera/web/resource/dashboard/UnifiedResourceSchema.scala index 40171c8f11e..591e152abc6 100644 --- a/amber/src/main/scala/org/apache/texera/web/resource/dashboard/UnifiedResourceSchema.scala +++ b/amber/src/main/scala/org/apache/texera/web/resource/dashboard/UnifiedResourceSchema.scala @@ -80,7 +80,12 @@ object UnifiedResourceSchema { // mark the row and route it accordingly. workflowDefaultView: Field[DefaultViewEnum] = DSL.cast(null, classOf[DefaultViewEnum]), modelFramework: Field[String] = DSL.cast(null, classOf[String]), - modelFormat: Field[String] = DSL.cast(null, classOf[String]) + modelFormat: Field[String] = DSL.cast(null, classOf[String]), + workflowHasUnpublishedChanges: Field[java.lang.Boolean] = + DSL.cast(null, classOf[java.lang.Boolean]), + workflowPublishedName: Field[String] = DSL.cast(null, classOf[String]), + workflowPublishedDescription: Field[String] = DSL.cast(null, classOf[String]), + viewerHasGrantedAccess: Field[java.lang.Boolean] = DSL.cast(null, classOf[java.lang.Boolean]) ): UnifiedResourceSchema = { new UnifiedResourceSchema( Seq( @@ -110,7 +115,13 @@ object UnifiedResourceSchema { workflowCoverImage -> workflowCoverImage.as("workflow_cover_image"), workflowDefaultView -> workflowDefaultView.as("workflow_default_view"), modelFramework -> modelFramework.as("model_framework"), - modelFormat -> modelFormat.as("model_format") + modelFormat -> modelFormat.as("model_format"), + workflowHasUnpublishedChanges -> workflowHasUnpublishedChanges + .as("workflow_has_unpublished_changes"), + workflowPublishedName -> workflowPublishedName.as("workflow_published_name"), + workflowPublishedDescription -> workflowPublishedDescription + .as("workflow_published_description"), + viewerHasGrantedAccess -> viewerHasGrantedAccess.as("viewer_has_granted_access") ) ) } diff --git a/amber/src/main/scala/org/apache/texera/web/resource/dashboard/VersionedResourceSearchQueryBuilder.scala b/amber/src/main/scala/org/apache/texera/web/resource/dashboard/VersionedResourceSearchQueryBuilder.scala index aa60f11c8e4..4b693152a1c 100644 --- a/amber/src/main/scala/org/apache/texera/web/resource/dashboard/VersionedResourceSearchQueryBuilder.scala +++ b/amber/src/main/scala/org/apache/texera/web/resource/dashboard/VersionedResourceSearchQueryBuilder.scala @@ -110,7 +110,8 @@ abstract class VersionedResourceSearchQueryBuilder[Rec <: Record, P]( override protected def constructWhereClause( uid: Integer, - params: DashboardResource.SearchQueryParams + params: DashboardResource.SearchQueryParams, + includePublic: Boolean ): Condition = { val splitKeywords = params.keywords.asScala .flatMap(_.split("[+\\-()<>~*@\"]")) diff --git a/amber/src/main/scala/org/apache/texera/web/resource/dashboard/WorkflowSearchQueryBuilder.scala b/amber/src/main/scala/org/apache/texera/web/resource/dashboard/WorkflowSearchQueryBuilder.scala index 2e08a98d579..8f8e59eaa29 100644 --- a/amber/src/main/scala/org/apache/texera/web/resource/dashboard/WorkflowSearchQueryBuilder.scala +++ b/amber/src/main/scala/org/apache/texera/web/resource/dashboard/WorkflowSearchQueryBuilder.scala @@ -23,9 +23,10 @@ import org.apache.texera.dao.jooq.generated.Tables._ import org.apache.texera.dao.jooq.generated.tables.pojos.Workflow import org.apache.texera.web.resource.dashboard.DashboardResource.DashboardClickableFileEntry import org.apache.texera.web.resource.dashboard.FulltextSearchQueryUtils._ +import org.apache.texera.web.resource.dashboard.user.workflow.WorkflowPublishService import org.apache.texera.web.resource.dashboard.user.workflow.WorkflowResource.DashboardWorkflow import org.jooq.impl.DSL -import org.jooq.{Condition, GroupField, Record, TableLike} +import org.jooq.{Condition, Field, GroupField, Record, TableLike} import scala.jdk.CollectionConverters.CollectionHasAsScala import org.apache.texera.dao.jooq.generated.enums.{DefaultViewEnum, PrivilegeEnum} @@ -53,7 +54,25 @@ object WorkflowSearchQueryBuilder extends SearchQueryBuilder { ownerId = WORKFLOW_OF_USER.UID, userName = USER.NAME, workflowCoverImage = DSL.max(WORKFLOW_COVER_IMAGE.IMAGE).as("workflow_cover_image"), - workflowDefaultView = WORKFLOW.DEFAULT_VIEW.as("workflow_default_view") + workflowDefaultView = WORKFLOW.DEFAULT_VIEW.as("workflow_default_view"), + // The isNotNull guard because isDistinctFrom would read a NULL pin as different content, and + // every unpinned public workflow would report drift. Aggregated to stay out of the GROUP BY, + // which would otherwise group the search by two TEXT columns. + workflowHasUnpublishedChanges = DSL + .boolOr( + WORKFLOW.PUBLISHED_CONTENT.isNotNull + .and(WorkflowPublishService.pinDiffersFromWorkingCopy) + ) + .as("workflow_has_unpublished_changes"), + // What a viewer without granted access is shown instead of the author's live metadata. NULL + // while following, which the reader treats as "show the live values". + workflowPublishedName = DSL.max(WORKFLOW.PUBLISHED_NAME).as("workflow_published_name"), + workflowPublishedDescription = + DSL.max(WORKFLOW.PUBLISHED_DESCRIPTION).as("workflow_published_description"), + // The access join is already restricted to the caller, so this needs no user id of its own. + viewerHasGrantedAccess = DSL + .boolOr(WORKFLOW_USER_ACCESS.UID.isNotNull) + .as("viewer_has_granted_access") ) } @@ -88,9 +107,58 @@ object WorkflowSearchQueryBuilder extends SearchQueryBuilder { baseQuery.where(condition) } + /** Rows the user was granted access to, as opposed to rows they see only because it is public. */ + private def grantedAccessCondition(uid: Integer): Condition = + if (uid == null) DSL.falseCondition() + else WORKFLOW_USER_ACCESS.UID.eq(uid) + + /** + * The three searchable columns carrying one copy. They travel together because a pin freezes them + * together: a filter over content alone would match a pinned workflow on a title no public viewer + * has seen. + */ + private case class WorkflowCopy( + name: Field[String], + description: Field[String], + content: Field[String] + ) + + private val workingCopy = + WorkflowCopy(WORKFLOW.NAME, WORKFLOW.DESCRIPTION, WORKFLOW.CONTENT) + private val pinnedCopy = + WorkflowCopy( + WORKFLOW.PUBLISHED_NAME, + WORKFLOW.PUBLISHED_DESCRIPTION, + WORKFLOW.PUBLISHED_CONTENT + ) + + /** + * Applies a filter to whichever copy the user is allowed to see: one search can return both their + * own workflows and public ones, and a pinned public one must not turn up on keywords that exist + * only behind the pin. A disjunction of guarded filters over bare columns rather than a CASE, so + * each side stays eligible for its own PGroonga index. + */ + private def onVisibleCopy( + uid: Integer, + includePublic: Boolean + )(build: WorkflowCopy => Condition): Condition = { + val onWorkingCopy = build(workingCopy).and(grantedAccessCondition(uid)) + val onPublicCopy = WORKFLOW.IS_PUBLIC + .eq(true) + .and( + // Following leaves the pinned columns NULL, and the public copy is then the working one. + build(pinnedCopy) + .or(WORKFLOW.PUBLISHED_CONTENT.isNull.and(build(workingCopy))) + ) + if (uid == null) onPublicCopy + else if (includePublic) onWorkingCopy.or(onPublicCopy) + else onWorkingCopy + } + override protected def constructWhereClause( uid: Integer, - params: DashboardResource.SearchQueryParams + params: DashboardResource.SearchQueryParams, + includePublic: Boolean ): Condition = { val splitKeywords = params.keywords.asScala .flatMap(_.split("[+\\-()<>~*@\"]")) @@ -114,13 +182,23 @@ object WorkflowSearchQueryBuilder extends SearchQueryBuilder { // Apply owner filter .and(getContainsFilter(params.owners, USER.EMAIL)) // Apply operators filter - .and(getOperatorsFilter(params.operators, WORKFLOW.CONTENT)) + .and( + if (params.operators.isEmpty) DSL.noCondition() + else + onVisibleCopy(uid, includePublic)(copy => + getOperatorsFilter(params.operators, copy.content) + ) + ) // Apply fulltext search filter .and( - getFullTextSearchFilter( - splitKeywords, - List(WORKFLOW.NAME, WORKFLOW.DESCRIPTION, WORKFLOW.CONTENT) - ) + if (splitKeywords.isEmpty) DSL.noCondition() + else + onVisibleCopy(uid, includePublic)(copy => + getFullTextSearchFilter( + splitKeywords, + List(copy.name, copy.description, copy.content) + ) + ) ) } @@ -141,20 +219,35 @@ object WorkflowSearchQueryBuilder extends SearchQueryBuilder { uid: Integer, record: Record ): DashboardResource.DashboardClickableFileEntry = { + val workflow = record.into(WORKFLOW).into(classOf[Workflow]) + // The select lists specific columns, so the POJO built from the record does not carry this one. + // Without it the listing forgets the default-view preference on every refresh. + workflow.setDefaultView(record.get("workflow_default_view", classOf[DefaultViewEnum])) + + // A viewer here only because the workflow is public sees the pinned name and description, the + // same copy the detail page serves -- otherwise a listing would advertise a title that opening it + // does not show. Both are NULL while following, which leaves the live values in place. + // Unknown counts as not granted: the reverse is the leak. + val granted = Option(record.get("viewer_has_granted_access", classOf[java.lang.Boolean])) + .exists(_.booleanValue()) + if (!granted) { + Option(record.get("workflow_published_name", classOf[String])).foreach(workflow.setName) + Option(record.get("workflow_published_description", classOf[String])) + .foreach(workflow.setDescription) + } + val dw = DashboardWorkflow( record.into(WORKFLOW_OF_USER).getUid == uid, Option(record.get(WORKFLOW_USER_ACCESS.PRIVILEGE, classOf[PrivilegeEnum])) .map(_.toString) .getOrElse(PrivilegeEnum.NONE.toString), - record.into(USER).getName, { - // The select lists specific columns, so the POJO built from the record does not carry - // this one. Without it the listing forgets the default-view preference on every refresh. - val w = record.into(WORKFLOW).into(classOf[Workflow]) - w.setDefaultView(record.get("workflow_default_view", classOf[DefaultViewEnum])) - w - }, + record.into(USER).getName, + workflow, record.into(USER).getUid, - Option(record.get("workflow_cover_image", classOf[String])) + Option(record.get("workflow_cover_image", classOf[String])), + // Null for the resource types that do not define the column at all. + Option(record.get("workflow_has_unpublished_changes", classOf[java.lang.Boolean])) + .exists(_.booleanValue()) ) DashboardClickableFileEntry(SearchQueryBuilder.WORKFLOW_RESOURCE_TYPE, workflow = Some(dw)) } diff --git a/amber/src/main/scala/org/apache/texera/web/resource/dashboard/hub/HubResource.scala b/amber/src/main/scala/org/apache/texera/web/resource/dashboard/hub/HubResource.scala index 42ae99d9ce3..6f8c9880589 100644 --- a/amber/src/main/scala/org/apache/texera/web/resource/dashboard/hub/HubResource.scala +++ b/amber/src/main/scala/org/apache/texera/web/resource/dashboard/hub/HubResource.scala @@ -30,6 +30,7 @@ import org.apache.texera.web.resource.dashboard.VersionedResourceTables import org.apache.texera.web.resource.dashboard.hub.ActionType.{Clone, Like, Unlike, View} import org.apache.texera.web.resource.dashboard.hub.EntityTables._ import org.apache.texera.web.resource.dashboard.hub.HubResource._ +import org.apache.texera.web.resource.dashboard.user.workflow.WorkflowPublishService import org.apache.texera.web.resource.dashboard.user.workflow.WorkflowResource.{ DashboardWorkflow, baseWorkflowSelect, @@ -273,7 +274,23 @@ object HubResource { ) .fetch() - mapWorkflowEntries(records, uid) + // The hub is the public shelf, so everything on it is listed as the public sees it: while a pin + // is in place, under the name and description frozen with it rather than the author's live ones + // -- for the author too, who is looking at the shelf and not at their own dashboard. + val entries = mapWorkflowEntries(records, uid) + val pinned = WorkflowPublishService.pinnedListingsOf(entries.map(_.workflow.getWid)) + entries.map { entry => + pinned.get(entry.workflow.getWid) match { + case None => entry + case Some(listing) => + entry.workflow.setName(listing.name) + entry.workflow.setDescription(listing.description) + // Carried like the search listing carries it: a card advertising the pinned copy has to + // open that copy, and without this flag the author's own card would open their editor and + // show them something else. + entry.copy(hasUnpublishedChanges = listing.hasUnpublishedChanges) + } + } } /** 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 e7306ad9b38..25b8ad029fa 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 @@ -26,9 +26,10 @@ import org.apache.texera.dao.jooq.generated.Tables.WORKFLOW 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 -import org.jooq.DSLContext +import org.jooq.{Condition, DSLContext} import javax.ws.rs.NotFoundException +import scala.jdk.CollectionConverters.CollectionHasAsScala import scala.util.Try /** @@ -230,6 +231,52 @@ object WorkflowPublishService extends LazyLogging { workflow.getDefaultView ) + /** + * [[differs]] as a condition, for the listings that ask about many workflows at once: the same + * fields, so a card and the share dialog can never disagree about whether edits are held back. + * Only meaningful on a row that is pinned -- while following, every frozen column is NULL and + * `isDistinctFrom` would read that as drift. + */ + val pinDiffersFromWorkingCopy: Condition = + WORKFLOW.PUBLISHED_CONTENT + .isDistinctFrom(WORKFLOW.CONTENT) + .or(WORKFLOW.PUBLISHED_NAME.isDistinctFrom(WORKFLOW.NAME)) + .or(WORKFLOW.PUBLISHED_DESCRIPTION.isDistinctFrom(WORKFLOW.DESCRIPTION)) + .or(WORKFLOW.PUBLISHED_DEFAULT_VIEW.isDistinctFrom(WORKFLOW.DEFAULT_VIEW)) + + /** + * What a listing needs about one pinned workflow: the frozen name and description it must show + * instead of the author's live ones, and whether those live ones have moved on. + */ + case class PinnedListing(name: String, description: String, hasUnpublishedChanges: Boolean) + + /** + * The pinned listings among `wids`, keyed by wid. A workflow that follows the author's latest is + * simply absent, which leaves its live values in place and its drift flag false. + * + * Drift is decided in SQL here rather than by [[differs]], because a listing asks about many + * workflows at once and none of their contents are worth shipping back to compare in memory. + */ + def pinnedListingsOf(wids: Seq[Integer]): Map[Integer, PinnedListing] = + if (wids.isEmpty) Map() + else { + val drifted = pinDiffersFromWorkingCopy + context + .select(WORKFLOW.WID, WORKFLOW.PUBLISHED_NAME, WORKFLOW.PUBLISHED_DESCRIPTION, drifted) + .from(WORKFLOW) + .where(WORKFLOW.WID.in(wids: _*).and(WORKFLOW.PUBLISHED_CONTENT.isNotNull)) + .fetch() + .asScala + .map(row => + row.get(WORKFLOW.WID) -> PinnedListing( + row.get(WORKFLOW.PUBLISHED_NAME), + row.get(WORKFLOW.PUBLISHED_DESCRIPTION), + row.get(drifted) + ) + ) + .toMap + } + /** As [[publicCopyOf]], for callers holding only a wid. 404s unless the workflow is public. */ def publicCopyOf(wid: Integer): PublicCopy = { val workflow = requireWorkflow(wid) 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 6337715293c..8d48d70405f 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 @@ -122,7 +122,10 @@ object WorkflowResource { ownerName: String, workflow: Workflow, ownerId: Integer, - coverImage: Option[String] + coverImage: Option[String], + // Behind the author's working copy? Listings use it to open the published preview instead of + // the editor -- for the author too, since that is what the entry is showing. + hasUnpublishedChanges: Boolean = false ) case class WorkflowWithPrivilege( diff --git a/amber/src/test/scala/org/apache/texera/web/resource/dashboard/UnifiedResourceSchemaSpec.scala b/amber/src/test/scala/org/apache/texera/web/resource/dashboard/UnifiedResourceSchemaSpec.scala index 27856595794..7d545d469e9 100644 --- a/amber/src/test/scala/org/apache/texera/web/resource/dashboard/UnifiedResourceSchemaSpec.scala +++ b/amber/src/test/scala/org/apache/texera/web/resource/dashboard/UnifiedResourceSchemaSpec.scala @@ -79,9 +79,10 @@ class UnifiedResourceSchemaSpec extends AnyFlatSpec with Matchers { // Sentinels for the two slots that have no convenient distinct table // column of the right type; every other slot uses a real generated column so - // that all 23 originals render differently from one another. + // that all 27 originals render differently from one another. private val sentinelResourceType: Field[String] = JDSL.inline("s-resource-type") private val sentinelStoragePath: Field[String] = JDSL.inline("s-storage-path") + private val sentinelGranted: Field[java.lang.Boolean] = JDSL.inline(java.lang.Boolean.TRUE) private val sentinelSchema: UnifiedResourceSchema = UnifiedResourceSchema( resourceType = sentinelResourceType, @@ -106,7 +107,11 @@ class UnifiedResourceSchemaSpec extends AnyFlatSpec with Matchers { workflowCoverImage = WORKFLOW_COVER_IMAGE.IMAGE, workflowDefaultView = WORKFLOW.DEFAULT_VIEW, modelFramework = MODEL.FRAMEWORK, - modelFormat = MODEL.FORMAT + modelFormat = MODEL.FORMAT, + workflowHasUnpublishedChanges = WORKFLOW.IS_PUBLIC, + workflowPublishedName = WORKFLOW.PUBLISHED_NAME, + workflowPublishedDescription = WORKFLOW.PUBLISHED_DESCRIPTION, + viewerHasGrantedAccess = sentinelGranted ) // Expected projection, in order: alias -> the original it must be built from. @@ -133,13 +138,17 @@ class UnifiedResourceSchemaSpec extends AnyFlatSpec with Matchers { "workflow_cover_image" -> WORKFLOW_COVER_IMAGE.IMAGE, "workflow_default_view" -> WORKFLOW.DEFAULT_VIEW, "model_framework" -> MODEL.FRAMEWORK, - "model_format" -> MODEL.FORMAT + "model_format" -> MODEL.FORMAT, + "workflow_has_unpublished_changes" -> WORKFLOW.IS_PUBLIC, + "workflow_published_name" -> WORKFLOW.PUBLISHED_NAME, + "workflow_published_description" -> WORKFLOW.PUBLISHED_DESCRIPTION, + "viewer_has_granted_access" -> sentinelGranted ) // -- apply(): the projection ------------------------------------------------ - "apply" should "expose all 23 slots as aliases, in the order the UNION ALL depends on" in { - sentinelSchema.allFields should have size 23 + "apply" should "expose all 27 slots as aliases, in the order the UNION ALL depends on" in { + sentinelSchema.allFields should have size 27 sentinelSchema.allFields.map(_.getName) shouldBe expectedProjection.map(_._1) } @@ -159,7 +168,7 @@ class UnifiedResourceSchemaSpec extends AnyFlatSpec with Matchers { // about datasets still union with one that does: the column count and // types have to line up. val defaults = UnifiedResourceSchema() - defaults.allFields should have size 23 + defaults.allFields should have size 27 val rendered = ctx.renderInlined(JDSL.select(defaults.allFields: _*)) rendered should include("'' as \"resourceType\"") rendered should include("cast(null as timestamp) as \"resourceCreationTime\"") @@ -194,12 +203,12 @@ class UnifiedResourceSchemaSpec extends AnyFlatSpec with Matchers { } it should "collapse the all-defaults projection down to one alias per distinct default" in { - // 23 slots, but only seven structurally distinct default expressions, so the + // 27 slots, but only seven structurally distinct default expressions, so the // de-dup collapses the map to seven entries. Worth pinning because it is // surprising, and because it is what makes the keep-first rule observable at - // all: allFields stays at 23 while the translation map does not. + // all: allFields stays at 27 while the translation map does not. val defaults = UnifiedResourceSchema() - defaults.allFields should have size 23 + defaults.allFields should have size 27 translatedAliases(defaults) shouldBe Seq( "resourceType", // DSL.inline("") "resourceCreationTime", // cast(null as timestamp) @@ -211,7 +220,7 @@ class UnifiedResourceSchemaSpec extends AnyFlatSpec with Matchers { ) } - it should "keep every distinct original when the caller supplies 23 distinct Fields" in { + it should "keep every distinct original when the caller supplies 27 distinct Fields" in { // Nothing to collapse here, which is the control case for the two tests // above: the shrinkage they observe comes from duplicate originals only. translatedAliases(sentinelSchema) shouldBe expectedProjection.map(_._1) @@ -219,7 +228,7 @@ class UnifiedResourceSchemaSpec extends AnyFlatSpec with Matchers { it should "drop exactly the duplicated slots of the production workflow projection" in { val workflowSchema = WorkflowSearchQueryBuilder.mappedResourceSchema - workflowSchema.allFields should have size 23 + workflowSchema.allFields should have size 27 val aliases = translatedAliases(workflowSchema) // `uid` duplicates ownerId (WORKFLOW_OF_USER.UID); the rest are slots the // builder left at their default, and the defaults collide by type. @@ -238,7 +247,7 @@ class UnifiedResourceSchemaSpec extends AnyFlatSpec with Matchers { "jOOQ Field equality" should "be structural, which is what makes the de-dup collapse anything" in { // If jOOQ ever switched to identity equality, translatedFieldSet would keep - // all 23 slots and translateRecord would start reading duplicated columns — + // all 27 slots and translateRecord would start reading duplicated columns — // the tests above would flip, and this one says why. JDSL.cast(null, classOf[Integer]) shouldBe JDSL.cast(null, classOf[Integer]) JDSL.inline("") shouldBe JDSL.inline("") diff --git a/amber/src/test/scala/org/apache/texera/web/resource/dashboard/WorkflowSearchQueryBuilderSpec.scala b/amber/src/test/scala/org/apache/texera/web/resource/dashboard/WorkflowSearchQueryBuilderSpec.scala index f0d8fc63d9a..1870eb9e9bb 100644 --- a/amber/src/test/scala/org/apache/texera/web/resource/dashboard/WorkflowSearchQueryBuilderSpec.scala +++ b/amber/src/test/scala/org/apache/texera/web/resource/dashboard/WorkflowSearchQueryBuilderSpec.scala @@ -62,6 +62,13 @@ class WorkflowSearchQueryBuilderSpec extends AnyFlatSpec with Matchers { // The select lists default_view under its own alias (not carried by the WORKFLOW POJO), // and toEntryImpl reads it back by that alias — the record has to carry the column. private val defaultViewField = WORKFLOW.DEFAULT_VIEW.as("workflow_default_view") + private val driftField = + JDSL.field(JDSL.name("workflow_has_unpublished_changes"), classOf[java.lang.Boolean]) + private val publishedNameField = JDSL.field(JDSL.name("workflow_published_name"), classOf[String]) + private val publishedDescriptionField = + JDSL.field(JDSL.name("workflow_published_description"), classOf[String]) + private val grantedField = + JDSL.field(JDSL.name("viewer_has_granted_access"), classOf[java.lang.Boolean]) private val ownerUid: Integer = Integer.valueOf(42) private val viewerUid: Integer = Integer.valueOf(43) @@ -79,7 +86,13 @@ class WorkflowSearchQueryBuilderSpec extends AnyFlatSpec with Matchers { uidValue: Integer = ownerUid, privilege: PrivilegeEnum = PrivilegeEnum.WRITE, cover: String = "cover-b64", - defaultView: DefaultViewEnum = DefaultViewEnum.CANVAS + defaultView: DefaultViewEnum = DefaultViewEnum.CANVAS, + hasUnpublishedChanges: java.lang.Boolean = null, + // The rows these tests describe belong to a viewer who was granted access, which is what + // leaves the author's own name and description in place. + grantedAccess: java.lang.Boolean = java.lang.Boolean.TRUE, + publishedName: String = null, + publishedDescription: String = null ): Record = { val record = ctx.newRecord( WORKFLOW.WID, @@ -89,7 +102,11 @@ class WorkflowSearchQueryBuilderSpec extends AnyFlatSpec with Matchers { WORKFLOW_USER_ACCESS.PRIVILEGE, USER.NAME, coverField, - defaultViewField + defaultViewField, + driftField, + publishedNameField, + publishedDescriptionField, + grantedField ) record.set(WORKFLOW.WID, wid) record.set(WORKFLOW.NAME, "wf-name") @@ -99,12 +116,61 @@ class WorkflowSearchQueryBuilderSpec extends AnyFlatSpec with Matchers { record.set(USER.NAME, "owner-name") record.set(coverField, cover) record.set(defaultViewField, defaultView) + record.set(driftField, hasUnpublishedChanges) + record.set(publishedNameField, publishedName) + record.set(publishedDescriptionField, publishedDescription) + record.set(grantedField, grantedAccess) record } private def workflowOf(record: Record, uid: Integer): DashboardWorkflow = WorkflowSearchQueryBuilder.toEntryImpl(uid, record).workflow.get + // -- what a viewer without granted access is shown -------------------------- + + "toEntryImpl" should "serve the published name and description to a viewer without granted access" in { + // Publishing pins what the public sees, and a listing is where the public meets a workflow, so + // the listing has to serve the pinned copy too rather than the author's live metadata. + val record = translatedRecord( + grantedAccess = java.lang.Boolean.FALSE, + publishedName = "as-published", + publishedDescription = "described-as-published" + ) + + val workflow = workflowOf(record, viewerUid).workflow + + workflow.getName shouldBe "as-published" + workflow.getDescription shouldBe "described-as-published" + } + + it should "leave the author's live name and description for a viewer who was granted access" in { + val record = translatedRecord( + grantedAccess = java.lang.Boolean.TRUE, + publishedName = "as-published", + publishedDescription = "described-as-published" + ) + + val workflow = workflowOf(record, ownerUid).workflow + + workflow.getName shouldBe "wf-name" + workflow.getDescription shouldBe "wf-description" + } + + it should "treat an unknown access answer as public, which is the safe direction" in { + // Showing the published copy to someone who turns out to have access is harmless; the reverse + // would hand the author's live metadata to the public. + val record = translatedRecord(grantedAccess = null, publishedName = "as-published") + + workflowOf(record, viewerUid).workflow.getName shouldBe "as-published" + } + + it should "keep the author's name when a public workflow has nothing pinned" in { + // Nothing to substitute, so the row is left as it is rather than blanked. + val record = translatedRecord(grantedAccess = java.lang.Boolean.FALSE, publishedName = null) + + workflowOf(record, viewerUid).workflow.getName shouldBe "wf-name" + } + // -- privilege fallback ----------------------------------------------------- "toEntryImpl" should "fall back to NONE when the workflow privilege is NULL" in { 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 dbf3c5eec59..515db075c03 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 @@ -29,7 +29,10 @@ import org.apache.texera.dao.jooq.generated.tables.daos.{ WorkflowUserAccessDao } import org.apache.texera.dao.jooq.generated.tables.pojos.{User, Workflow, WorkflowUserAccess} +import org.apache.texera.web.resource.dashboard.DashboardResource.SearchQueryParams +import org.apache.texera.web.resource.dashboard.hub.HubResource import org.apache.texera.web.resource.dashboard.user.workflow.WorkflowResource.WorkflowIDs +import org.apache.texera.web.resource.dashboard.{DashboardResource, FulltextSearchQueryUtils} import org.scalatest.BeforeAndAfterAll import org.scalatest.flatspec.AnyFlatSpec import org.scalatest.matchers.should.Matchers @@ -45,7 +48,7 @@ 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. + * read paths that decide which of the two a caller is served, listings and search among them. */ class WorkflowPublishSpec extends AnyFlatSpec @@ -85,6 +88,7 @@ class WorkflowPublishSpec override protected def beforeAll(): Unit = { initializeDBAndReplaceDSLContext() + FulltextSearchQueryUtils.usePgroonga = false val userDao = new UserDao(getDSLContext.configuration()) userDao.insert(owner) userDao.insert(stranger) @@ -168,6 +172,40 @@ class WorkflowPublishSpec ) .asInstanceOf[HttpServletRequest] + private def keywords(values: String*): util.ArrayList[String] = { + val list = new util.ArrayList[String]() + values.foreach(list.add) + list + } + + private def keywordIds(values: Integer*): util.ArrayList[Integer] = { + val list = new util.ArrayList[Integer]() + values.foreach(list.add) + list + } + + /** The wids a search returns, which is all any of these tests asks of one. */ + private def searchWids(user: SessionUser, params: SearchQueryParams): List[Integer] = + DashboardResource + .searchAllResources(user, params, includePublic = true) + .results + .flatMap(_.workflow.map(_.workflow.getWid)) + + /** One workflow's listing row, as the given user would see it in a dashboard or on the Hub. */ + private def listingOf(user: SessionUser, wid: Integer) = + DashboardResource + .searchAllResources( + user, + SearchQueryParams(workflowIDs = keywordIds(wid)), + includePublic = true + ) + .results + .flatMap(_.workflow) + .head + + /** Nobody: the hub as an unauthenticated visitor reads it. */ + private def anonymous: SessionUser = new SessionUser(new User()) + private def statusOf(wid: Integer): WorkflowPublishService.PublishStatus = workflowResource.getPublishStatus(wid, ownerSession) @@ -913,4 +951,256 @@ class WorkflowPublishSpec .getSize(util.Arrays.asList(pinned)) .get(pinned) shouldBe publishedContent.length } + + behavior of "hub listings" + + it should "list a pinned workflow on the hub under the name and description it froze" in { + // The hub is the public shelf: everything on it is listed as the public sees it, the author + // included. A pin that this listing did not honour would put the author's live title on that + // shelf, and would leave it disagreeing with the hub's own search about what a workflow is + // called. + val wid = createWorkflow("hub_listing_shows_public_copy") + publishPinned(wid) + relabel(wid, "renamed_after_pinning", "described_after_pinning") + + for (viewer <- Seq(stranger.getUid, owner.getUid)) { + val listed = HubResource.fetchDashboardWorkflowsByWids(Seq(wid), viewer).head.workflow + listed.getName shouldBe "hub_listing_shows_public_copy" + listed.getDescription shouldBe "a workflow" + } + } + + it should "list a following workflow on the hub under the author's latest name and description" in { + val wid = createWorkflow("hub_listing_follows_latest") + workflowResource.makePublic(wid, ownerSession) + relabel(wid, "renamed_while_following", "described_while_following") + + val listed = HubResource.fetchDashboardWorkflowsByWids(Seq(wid), owner.getUid).head.workflow + listed.getName shouldBe "renamed_while_following" + listed.getDescription shouldBe "described_while_following" + } + + it should "tell the hub listing when the copy on show is behind the author's working copy" in { + // The card advertises the pinned copy, so clicking it has to open that copy. Without this flag + // the author's own card would take them to their editor and show them something else -- the + // same signal the search listing carries, on the query the hub builds for itself. + val wid = createWorkflow("hub_listing_reports_drift") + publishPinned(wid) + + def listedDrift(): Boolean = + HubResource.fetchDashboardWorkflowsByWids(Seq(wid), owner.getUid).head.hasUnpublishedChanges + + listedDrift() shouldBe false + edit(wid, editedContent) + listedDrift() shouldBe true + workflowResource.pinLatest(wid, ownerSession) + listedDrift() shouldBe false + } + + it should "clone the author's latest name and description while nothing is pinned" in { + val wid = createWorkflow("clone_follows_latest_metadata") + workflowResource.makePublic(wid, ownerSession) + relabel(wid, "renamed_before_clone", "described_before_clone") + + val cloned = + workflowDao.fetchOneByWid(workflowResource.cloneWorkflow(wid, strangerSession, fakeRequest())) + + cloned.getName shouldBe "renamed_before_clone_clone" + cloned.getDescription shouldBe "described_before_clone" + } + + behavior of "search" + + it should "not match a public workflow on anything that exists only in unpublished edits" in { + // Both halves of what public search indexes -- the words in the graph, and the operators in it -- + // have to stop at the pinned copy, or searching would surface drafts nobody can open. + val wid = createWorkflow("search_ignores_drafts") + publishPinned(wid) + edit( + wid, + """{"operators":[{"operatorType":"SecretDraftOperator"}],"note":"supersecretdraftword"}""" + ) + + val byKeyword = + searchWids(anonymous, SearchQueryParams(keywords = keywords("supersecretdraftword"))) + val byOperator = + searchWids(anonymous, SearchQueryParams(operators = keywords("SecretDraftOperator"))) + + byKeyword should not contain wid + byOperator should not contain wid + } + + it should "match a public workflow on keywords in its published copy" in { + val wid = createWorkflow("search_finds_published") + publishPinned(wid) + edit(wid, editedContent) + + searchWids( + anonymous, + SearchQueryParams(keywords = keywords("content_as_published")) + ) should contain(wid) + } + + it should "match an unpinned public workflow on the author's latest" in { + // Following means the public copy is the working copy, so search must reach it through the same + // public path that a pinned workflow reaches its frozen copy through. + val wid = createWorkflow("search_finds_unpinned_latest") + workflowResource.makePublic(wid, ownerSession) + edit(wid, """{"operators":[],"note":"unpinnedsearchword"}""") + + searchWids( + anonymous, + SearchQueryParams(keywords = keywords("unpinnedsearchword")) + ) should contain(wid) + } + + it should "still match the author's own workflow on their unpublished edits" in { + val wid = createWorkflow("search_finds_own_draft") + publishPinned(wid) + edit(wid, """{"operators":[],"note":"myowndraftword"}""") + + searchWids( + ownerSession, + SearchQueryParams(keywords = keywords("myowndraftword")) + ) should contain(wid) + } + + it should "match a pinned workflow on the name and description the public can see" in { + // The graph was already matched against the pinned copy; the title and description are on public + // show just as much, so matching them against the author's live values gets it wrong in both + // directions at once -- findable by a title nobody has seen, unfindable by the one on screen. + val wid = createWorkflow("aapublicnamesearch") + relabel(wid, "aapublicnamesearch", "aapublicdescriptionsearch") + publishPinned(wid) + relabel(wid, "zzsecretnamesearch", "zzsecretdescriptionsearch") + + def anonymousHits(word: String): List[Integer] = + searchWids(anonymous, SearchQueryParams(keywords = keywords(word))) + + anonymousHits("aapublicnamesearch") should contain(wid) + anonymousHits("aapublicdescriptionsearch") should contain(wid) + anonymousHits("zzsecretnamesearch") should not contain wid + anonymousHits("zzsecretdescriptionsearch") should not contain wid + } + + it should "match an unpinned public workflow on its live name and description" in { + val wid = createWorkflow("bbfollowingnamesearch") + workflowResource.makePublic(wid, ownerSession) + relabel(wid, "bbrenamedwhilefollowing", "a workflow") + + searchWids( + anonymous, + SearchQueryParams(keywords = keywords("bbrenamedwhilefollowing")) + ) should contain(wid) + } + + it should "still match the author's own workflow on a name only they can see" in { + val wid = createWorkflow("ccownnamesearch") + publishPinned(wid) + relabel(wid, "ccprivaterenamesearch", "a workflow") + + searchWids( + ownerSession, + SearchQueryParams(keywords = keywords("ccprivaterenamesearch")) + ) should contain(wid) + } + + it should "show a public viewer the published name and description in a listing" in { + // The listing is where people find and click a workflow, so a title that keeps following the + // author defeats the freeze exactly as an unfrozen graph would: a report about a title could be + // answered by quietly editing the title. + val wid = createWorkflow("listing_name_before") + publishPinned(wid) + relabel(wid, "listing_name_after", "described after publishing") + + def listedAs(session: SessionUser): (String, String) = { + val entry = listingOf(session, wid).workflow + (entry.getName, entry.getDescription) + } + + // The author is not a public viewer of their own workflow. + listedAs(ownerSession) shouldBe ("listing_name_after", "described after publishing") + // A stranger reaches it only because it is public. + listedAs(strangerSession) shouldBe ("listing_name_before", "a workflow") + // ...and the listing now agrees with what opening it shows. + workflowResource.retrievePublicWorkflow(wid).name shouldBe "listing_name_before" + } + + it should "show a public viewer the author's live name while nothing is pinned" in { + val wid = createWorkflow("listing_unpinned_before") + workflowResource.makePublic(wid, ownerSession) + relabel(wid, "listing_unpinned_after", "a workflow") + + listingOf(strangerSession, wid).workflow.getName shouldBe "listing_unpinned_after" + } + + it should "show a collaborator the author's live name in a listing" in { + val wid = createWorkflow("listing_for_collaborator") + publishPinned(wid) + grantAccess(wid, PrivilegeEnum.READ) + + try { + relabel(wid, "renamed_after_sharing", "a workflow") + listingOf(strangerSession, wid).workflow.getName shouldBe "renamed_after_sharing" + } finally revokeAccess(wid) + } + + it should "leave a private workflow's own listing untouched" in { + val wid = createWorkflow("listing_private") + relabel(wid, "private_renamed", "a workflow") + + listingOf(ownerSession, wid).workflow.getName shouldBe "private_renamed" + } + + it should "tell listings when the copy on show is behind the author's working copy" in { + // What sends the author to the published preview instead of their editor when they click their + // own workflow in the hub: the entry is advertising the pinned version, not what they are editing. + val wid = createWorkflow("listing_reports_drift") + publishPinned(wid) + + def drifted(): Boolean = listingOf(ownerSession, wid).hasUnpublishedChanges + + drifted() shouldBe false + edit(wid, editedContent) + drifted() shouldBe true + workflowResource.pinLatest(wid, ownerSession) + drifted() shouldBe false + } + + it should "report the same drift to a listing as to the share dialog" in { + // Three places answer "is the public behind?": the share dialog in Scala, the search projection + // and the hub listing in SQL. A rename or a change of view exercises the fields most easily left + // out of one of them, and a card that disagrees with the dialog is a card the author distrusts. + val wid = createWorkflow("listing_matches_dialog") + publishPinned(wid) + + relabel(wid, "renamed_after_pinning", "a workflow") + listingOf(ownerSession, wid).hasUnpublishedChanges shouldBe statusOf(wid).hasUnpublishedChanges + listingOf(ownerSession, wid).hasUnpublishedChanges shouldBe true + + workflowResource.pinLatest(wid, ownerSession) + getDSLContext + .update(WORKFLOW) + .set(WORKFLOW.DEFAULT_VIEW, DefaultViewEnum.FORM) + .where(WORKFLOW.WID.eq(wid)) + .execute() + + listingOf(ownerSession, wid).hasUnpublishedChanges shouldBe statusOf(wid).hasUnpublishedChanges + listingOf(ownerSession, wid).hasUnpublishedChanges shouldBe true + } + + it should "never report drift for a workflow with nothing frozen" in { + // Drift is "what the public sees is not what you have", so it can only be true of a workflow that + // has a frozen copy at all. Private and public-but-following both have none. + def driftOf(wid: Integer): Boolean = listingOf(ownerSession, wid).hasUnpublishedChanges + + val priv = createWorkflow("listing_ignores_private") + edit(priv, editedContent) + driftOf(priv) shouldBe false + + val following = createWorkflow("listing_ignores_unpinned") + workflowResource.makePublic(following, ownerSession) + edit(following, editedContent) + driftOf(following) shouldBe false + } } From 127a900dce998a31052989176cc934a65843dfa6 Mon Sep 17 00:00:00 2001 From: Yang Zhang Date: Sat, 22 Aug 2026 14:59:57 -0700 Subject: [PATCH 4/6] feat(workflow): mark the pinned version in the revision history The author can pin a version, and their working copy then moves on. This gives them a way back to what the public is seeing: pinning leaves an anchor in the revision history they already use, and the panel marks it as the one currently public, so restoring the published version is the restore they already know. The anchor is a version row whose delta is the identity patch, so replaying it returns exactly what was pinned however many edits pile up after. An existing row cannot stand in: a version row replays to the content as it was *before* the change it records. Pinning content that is already the pinned one reuses its anchor rather than adding a twin, and the read that decides takes a row lock so two pins racing cannot both insert. Two consequences the anchor forces: - It must not start the version panel's aggregation window. It lands seconds after the save it freezes, and the panel folds close-together versions into the newest, which would hide the author's own save behind a row they never made. - The revision history is now readable only with granted access, or while nothing is pinned. Replaying a version folds deltas back from the author's current content, so listing versions of a pinned workflow would hand a public viewer the very edits the pin is holding back. `publish-status` carries the pinned version's date, read from the version row so the dialog and the panel print one value rather than two clocks'. Co-Authored-By: Claude Opus 5 --- .../workflow/WorkflowPublishService.scala | 63 +++++++-- .../user/workflow/WorkflowResource.scala | 6 +- .../workflow/WorkflowVersionResource.scala | 48 ++++--- .../user/workflow/WorkflowPublishSpec.scala | 122 +++++++++++++++++- .../WorkflowVersionResourceSpec.scala | 32 ++++- 5 files changed, 235 insertions(+), 36 deletions(-) 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 25b8ad029fa..58e24838d50 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.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 ) @@ -108,12 +114,14 @@ object WorkflowPublishService extends LazyLogging { * before it -- which would leave them looking at "you have unpublished changes" the instant * after they pinned. * - * @return how many rows it matched, so a missing workflow is distinguishable from a done one. + * `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.IS_PUBLIC, java.lang.Boolean.TRUE) + .set(WORKFLOW.PUBLISHED_VERSION_ID, versionId) .set(WORKFLOW.PUBLISHED_CONTENT, WORKFLOW.CONTENT) .set(WORKFLOW.PUBLISHED_NAME, WORKFLOW.NAME) .set(WORKFLOW.PUBLISHED_DESCRIPTION, WORKFLOW.DESCRIPTION) @@ -126,14 +134,12 @@ 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. - * * @return how many rows it matched, so a missing workflow is distinguishable from a done one. */ 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]) @@ -145,8 +151,30 @@ object WorkflowPublishService extends LazyLogging { /** Pins the current content as the public copy. Moving a pin forward is the same operation. */ def pinLatest(wid: Integer): PublishStatus = { - if (writePin(wid) == 0) { - throw new NotFoundException(s"Workflow $wid not found") + 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. + 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") + } + + // 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) @@ -175,6 +203,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) @@ -182,6 +226,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 8d48d70405f..c920cffd328 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 @@ -1011,14 +1011,16 @@ class WorkflowResource extends LazyLogging { .fetchOne() // 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 e84dde72fee..e3770561d64 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 @@ -141,11 +141,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 } @@ -209,39 +213,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 @@ -340,7 +350,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 ) } @@ -374,7 +387,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 515db075c03..6b44f77b729 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) } @@ -1203,4 +1212,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) } } From 8e673842273e7d440643284631965f3ff411d479 Mon Sep 17 00:00:00 2001 From: Yang Zhang Date: Sat, 22 Aug 2026 15:01:50 -0700 Subject: [PATCH 5/6] feat(frontend): choose between following and pinning in the share dialog The share dialog said whether a workflow was public and nothing about which copy the public was getting. With pinning that is now a choice, so the dialog carries it: a two-option control -- Follow latest, or Pinned -- under the Public tile, plus a line naming the pinned version by the date it went public. Follow latest is the default and is exactly what publishing does today. Pinning freezes the version the author has now; while a pin is in place and their working copy has moved on, the panel says so and offers Update to current, which is the only way those edits reach the public. The panel re-reads its state when a save lands rather than on a timer: `WorkflowPersistService` now emits on persist, rename and re-describe, which is what makes "your edits are not public yet" appear as soon as the autosave lands rather than the next time the dialog is opened. Publishing returns the state it produced, so the panel does not have to ask again. Co-Authored-By: Claude Opus 5 --- .../user/workflow/WorkflowResource.scala | 4 + common/config/src/main/resources/gui.conf | 5 + .../texera/common/config/GuiConfig.scala | 2 + .../texera/common/config/GuiConfigSpec.scala | 5 + .../service/resource/ConfigResource.scala | 1 + .../common/service/gui-config.service.mock.ts | 1 + .../workflow-persist.service.spec.ts | 63 +++ .../workflow-persist.service.ts | 50 ++- frontend/src/app/common/type/gui-config.ts | 1 + .../share-access/share-access.component.html | 63 +++ .../share-access/share-access.component.scss | 156 ++++++- .../share-access.component.spec.ts | 398 +++++++++++++++++- .../share-access/share-access.component.ts | 136 +++++- 13 files changed, 854 insertions(+), 31 deletions(-) diff --git a/amber/src/main/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowResource.scala b/amber/src/main/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowResource.scala index c920cffd328..91342b2a946 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 @@ -788,6 +788,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}") diff --git a/common/config/src/main/resources/gui.conf b/common/config/src/main/resources/gui.conf index 7cb7fff6555..6b6ef9994bc 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 = false + 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 d5295024dc8..26079eb240f 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..5b7f7ca12c6 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,11 @@ class GuiConfigSpec extends AnyFlatSpec with Matchers { ifUnset("GUI_WORKFLOW_WORKSPACE_FORM_VIEW_ENABLED")( GuiConfig.guiWorkflowWorkspaceFormViewEnabled shouldBe true ) + // Version pinning stays off until the last PR of the series lands, so a half-built feature is + // never reachable from the dialog on a deployed instance. + ifUnset("GUI_WORKFLOW_WORKSPACE_VERSION_PINNING_ENABLED")( + GuiConfig.guiWorkflowWorkspaceVersionPinningEnabled shouldBe false + ) 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 5bdba890c20..48f2921142c 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 @@ -91,6 +91,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 c4441540cbe..9bfdd13b08a 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 a35c9ec106e..3c745fea8fb 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, @@ -92,6 +93,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 66d267a673d..217b327bc8c 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 @@ -52,6 +52,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", }) @@ -71,6 +82,9 @@ export class WorkflowPersistService { */ private readonly persistQueue = new Subject<{ send: Observable; result: 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 @@ -117,7 +131,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. @@ -126,6 +141,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 @@ -218,6 +238,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); @@ -236,6 +258,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); @@ -248,6 +272,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); @@ -256,6 +284,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 27dff5d0848..c85d566bfc9 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/share-access/share-access.component.html b/frontend/src/app/dashboard/component/user/share-access/share-access.component.html index 8128f333633..bc77db1cea3 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,69 @@ + +
+ + + + +
+ + + + 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.
+ + +
+
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 202465b25f8..42e9645a98a 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"; interface SetupOptions { @@ -42,6 +43,7 @@ interface SetupOptions { id?: number; inWorkspace?: boolean; currentEmail?: string | undefined; + versionPinningEnabled?: boolean; } describe("ShareAccessComponent", () => { @@ -56,7 +58,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; }; @@ -64,7 +66,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. @@ -72,6 +77,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. @@ -81,7 +88,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({ @@ -103,6 +118,7 @@ describe("ShareAccessComponent", () => { { provide: WorkflowPersistService, useValue: workflowPersistSpy }, { provide: DatasetService, useValue: datasetServiceSpy }, { provide: WorkflowActionService, useValue: workflowActionSpy }, + { provide: GuiConfigService, useValue: { env: { versionPinningEnabled } } }, ], }); fixture = TestBed.createComponent(ShareAccessComponent); @@ -110,9 +126,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() }; @@ -135,12 +176,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 } })), @@ -149,7 +197,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 } { @@ -1081,4 +1133,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("Restore it any time from the version panel"); + }); + + 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 d5ebb4c5957..0de0ef57547 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,21 +206,40 @@ 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) + // 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(name => { - this.owner = name; + .subscribe(([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(); }); - // Stays null for kinds that cannot be published, which is what hides the publish buttons. - this.descriptor - ?.isPublic?.(this.id) - .pipe(untilDestroyed(this)) - .subscribe(isPublic => (this.isPublic = isPublic)); + 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 { @@ -336,7 +435,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", @@ -400,6 +503,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) => { From 2357f7454e91017ba445d9f4e3fb6c17d109f32a Mon Sep 17 00:00:00 2001 From: Yang Zhang Date: Sat, 22 Aug 2026 15:03:44 -0700 Subject: [PATCH 6/6] feat(frontend): mark the public version and open what a listing advertises Two places outside the share dialog have to agree with the pin. The versions panel marks the version the Hub is serving, as a quiet second line under its timestamp. That row is also how an author who has moved on gets the published copy back: it is already in the panel, so restoring it is the restore they already know. The panel re-reads when the share dialog closes, since that is where the pin can have moved and the panel is covered until then. Hub entries open what they advertise. A hub card shows the public copy, so an author whose working copy has moved on behind a pin now lands on the published preview -- what they just saw, and where Clone gives them a copy of it -- rather than in their editor showing something else. Their own listings are untouched: there the entry is their workflow, and it opens the editor as before. Co-Authored-By: Claude Opus 5 --- common/config/src/main/resources/gui.conf | 2 +- .../texera/common/config/GuiConfigSpec.scala | 5 +-- .../card-item/card-item.component.spec.ts | 15 +++++++ .../card-item/card-item.component.ts | 2 +- .../list-item/list-item.component.spec.ts | 29 ++++++++++++++ .../user/list-item/list-item.component.ts | 2 +- .../share-access/share-access.component.html | 5 ++- .../share-access.component.spec.ts | 2 +- .../resource-registry.service.ts | 13 +++++- .../workflow-version.service.ts | 17 +++++++- .../type/dashboard-workflow.interface.ts | 2 + .../dashboard/type/workflow-version-entry.ts | 2 + .../versions-list.component.html | 12 ++++++ .../versions-list.component.scss | 19 +++++++++ .../versions-list.component.spec.ts | 40 +++++++++++++++++++ .../versions-list/versions-list.component.ts | 11 +++++ .../component/menu/menu.component.spec.ts | 11 +++++ .../component/menu/menu.component.ts | 4 ++ 18 files changed, 182 insertions(+), 11 deletions(-) diff --git a/common/config/src/main/resources/gui.conf b/common/config/src/main/resources/gui.conf index 6b6ef9994bc..93f7f175d97 100644 --- a/common/config/src/main/resources/gui.conf +++ b/common/config/src/main/resources/gui.conf @@ -97,7 +97,7 @@ gui { # 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 = false + 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 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 5b7f7ca12c6..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,10 +66,9 @@ class GuiConfigSpec extends AnyFlatSpec with Matchers { ifUnset("GUI_WORKFLOW_WORKSPACE_FORM_VIEW_ENABLED")( GuiConfig.guiWorkflowWorkspaceFormViewEnabled shouldBe true ) - // Version pinning stays off until the last PR of the series lands, so a half-built feature is - // never reachable from the dialog on a deployed instance. + // Version pinning is on by default now that the whole feature has landed. ifUnset("GUI_WORKFLOW_WORKSPACE_VERSION_PINNING_ENABLED")( - GuiConfig.guiWorkflowWorkspaceVersionPinningEnabled shouldBe false + GuiConfig.guiWorkflowWorkspaceVersionPinningEnabled shouldBe true ) ifUnset("GUI_WORKFLOW_WORKSPACE_PRODUCTION_SHARED_EDITING_SERVER")( GuiConfig.guiWorkflowWorkspaceProductionSharedEditingServer shouldBe false 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 dd243bc9c80..971bd36cf39 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 @@ -217,6 +217,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 ce4a2310ef5..c6d3f761dd8 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 @@ -300,6 +300,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 bc77db1cea3..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 @@ -113,7 +113,10 @@ -
Frozen as you keep editing. Restore it any time from the version panel.
+
+ 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.spec.ts b/frontend/src/app/dashboard/component/user/share-access/share-access.component.spec.ts index 42e9645a98a..71b410ed70d 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 @@ -1384,7 +1384,7 @@ describe("ShareAccessComponent", () => { ); setupComponent({ type: "workflow", id: 3, ...asOwner }); - expect(publishLineText()).toContain("Restore it any time from the version panel"); + expect(publishLineText()).toContain("Currently public"); }); it("shows nothing and asks nothing while the feature is off", () => { 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 40ef1fbc863..cb9c491b1ce 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 @@ -100,8 +100,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 []; @@ -110,6 +114,11 @@ export class ResourceRegistryService { return [descriptor.privateRoute, String(entry.id)]; } const reachableByViewer = currentUid !== undefined && 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 3e15706cf0f..3e35c135e8e 100644 --- a/frontend/src/app/workspace/component/menu/menu.component.spec.ts +++ b/frontend/src/app/workspace/component/menu/menu.component.spec.ts @@ -797,6 +797,17 @@ describe("MenuComponent", () => { expect(navigateSpy).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. + 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 3a46e987f33..d3048aed7d8 100644 --- a/frontend/src/app/workspace/component/menu/menu.component.ts +++ b/frontend/src/app/workspace/component/menu/menu.component.ts @@ -350,7 +350,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(); }); }