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..d1144f635fa 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,14 @@ 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]), + workflowPublishedDefaultView: Field[DefaultViewEnum] = + DSL.cast(null, classOf[DefaultViewEnum]), + viewerHasGrantedAccess: Field[java.lang.Boolean] = DSL.cast(null, classOf[java.lang.Boolean]) ): UnifiedResourceSchema = { new UnifiedResourceSchema( Seq( @@ -110,7 +117,15 @@ 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"), + workflowPublishedDefaultView -> workflowPublishedDefaultView + .as("workflow_published_default_view"), + 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..ca0e80fb4ea 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,29 @@ 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 view belongs with the name and the description: a card that opens the frozen copy in + // the author's live view would offer a Form View over a graph that carries no form. + workflowPublishedDefaultView = + DSL.max(WORKFLOW.PUBLISHED_DEFAULT_VIEW).as("workflow_published_default_view"), + // 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 +111,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 +186,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 +223,37 @@ 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) + Option(record.get("workflow_published_default_view", classOf[DefaultViewEnum])) + .foreach(workflow.setDefaultView) + } + 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..7b4aa41c2f2 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,26 @@ 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) + // The view too: a hub card that opened the frozen copy in the author's live view would + // offer a Form View over a graph carrying no form. + entry.workflow.setDefaultView(listing.defaultView) + // 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/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 48ad3c85f3e..61483f25600 100644 --- a/amber/src/main/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowPublishService.scala +++ b/amber/src/main/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowPublishService.scala @@ -22,13 +22,16 @@ 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 -import org.jooq.DSLContext +import org.jooq.{Condition, DSLContext} import javax.ws.rs.{BadRequestException, NotFoundException} +import scala.jdk.CollectionConverters.CollectionHasAsScala + +import java.sql.Timestamp import scala.util.Try /** @@ -53,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 ) @@ -111,11 +118,14 @@ object WorkflowPublishService extends LazyLogging { * two statements, so a `/private` landing between them would otherwise be undone here, putting a * workflow the author had just taken down back on show under a frozen copy. * - * @return how many rows it matched -- zero for a workflow that is missing or not published. + * @return how many rows it matched -- zero for a workflow that is missing or not published. * + * `versionId` names the row in the revision history that replays to this copy, so the history can + * mark which version is the published one. */ - private def writePin(wid: Integer): Int = - context + private def writePin(ctx: DSLContext, wid: Integer, versionId: Integer): Unit = + ctx .update(WORKFLOW) + .set(WORKFLOW.PUBLISHED_VERSION_ID, versionId) .set(WORKFLOW.PUBLISHED_CONTENT, WORKFLOW.CONTENT) .set(WORKFLOW.PUBLISHED_NAME, WORKFLOW.NAME) .set(WORKFLOW.PUBLISHED_DESCRIPTION, WORKFLOW.DESCRIPTION) @@ -128,9 +138,6 @@ object WorkflowPublishService extends LazyLogging { * row only with every frozen column set on a public workflow, or with every one of them NULL, so * clearing them one at a time -- or clearing them after `is_public` -- would be rejected. * - * `published_version_id` is named by that constraint as well but is not touched here, for the - * same reason [[writePin]] does not set it: nothing writes it yet, so it is NULL on every row. - * * Dropping a pin is offered on a published workflow only, so that condition travels in the same * statement: a `/private` landing between the guard and here cannot be answered with "unpinned, * following latest" about a workflow that is now down. Unpublishing carries no such condition -- @@ -141,6 +148,7 @@ object WorkflowPublishService extends LazyLogging { private def clearPin(wid: Integer, alsoUnpublish: Boolean = false): Int = { val cleared = context .update(WORKFLOW) + .set(WORKFLOW.PUBLISHED_VERSION_ID, null.asInstanceOf[Integer]) .set(WORKFLOW.PUBLISHED_CONTENT, null.asInstanceOf[String]) .set(WORKFLOW.PUBLISHED_NAME, null.asInstanceOf[String]) .set(WORKFLOW.PUBLISHED_DESCRIPTION, null.asInstanceOf[String]) @@ -160,9 +168,35 @@ object WorkflowPublishService extends LazyLogging { * matching means one of two things, and the caller is told which. */ def pinLatest(wid: Integer): PublishStatus = { - if (writePin(wid) == 0) { - requireWorkflow(wid) // 404s if it is gone - throw new BadRequestException(s"Workflow $wid is not published") + context.transaction { txConfig => + val ctx = org.jooq.impl.DSL.using(txConfig) + // Locked, not merely read: two pins racing would both read before either wrote, and both + // insert an anchor. The lock is also what holds the publication check below: a `/private` + // arriving after it has to wait for this transaction rather than slipping under it. + val workflow = ctx + .selectFrom(WORKFLOW) + .where(WORKFLOW.WID.eq(wid)) + .forUpdate() + .fetchOneInto(classOf[Workflow]) + if (workflow == null) { + throw new NotFoundException(s"Workflow $wid not found") + } + // Before the anchor, not after: a pin that cannot land must not leave a version row behind. + if (!workflow.getIsPublic) { + throw new BadRequestException(s"Workflow $wid is not published") + } + + // An anchor in the revision history for the copy being pinned: its delta is the identity patch, + // so replaying this row returns what was published however many edits pile up later. An + // existing version row cannot stand in, since one replays to the content as it was *before* + // the change it records. Pinning again unchanged reuses the anchor rather than adding a twin. + val anchorVid = Option(workflow.getPublishedVersionId) + .filter(_ => + Option(workflow.getPublishedContent).exists(sameContent(_, workflow.getContent)) + ) + .getOrElse(WorkflowVersionResource.insertNewVersion(wid, ctx = ctx).getVid) + + writePin(ctx, wid, anchorVid) } logger.info(s"Workflow $wid pinned to its latest content") statusOf(wid) @@ -192,6 +226,22 @@ object WorkflowPublishService extends LazyLogging { logger.info(s"Workflow $wid unpublished") } + /** Read from the version row, so the dialog and the revision panel print one date rather than two. */ + private def pinnedVersionTimeOf(versionId: Integer): Option[Timestamp] = + Option( + context + .select(WORKFLOW_VERSION.CREATION_TIME) + .from(WORKFLOW_VERSION) + .where(WORKFLOW_VERSION.VID.eq(versionId)) + .fetchOneInto(classOf[Timestamp]) + ) + + /** The date a public viewer should see: that of the version on show, not of an edit they cannot. */ + def publicModifiedTime(workflow: Workflow): Timestamp = + Option(workflow.getPublishedVersionId) + .flatMap(pinnedVersionTimeOf) + .getOrElse(workflow.getLastModifiedTime) + /** Whether a version is pinned, and whether it is holding edits back. */ def statusOf(wid: Integer): PublishStatus = { val workflow = requireWorkflow(wid) @@ -199,15 +249,120 @@ 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 + 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)) + ) + } + + /** + * 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 ) + + /** + * [[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, + defaultView: DefaultViewEnum, + 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, + WORKFLOW.PUBLISHED_DEFAULT_VIEW, + 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(WORKFLOW.PUBLISHED_DEFAULT_VIEW), + 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) + 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 bae2fa0047e..8c464404f29 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( @@ -126,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( @@ -144,11 +143,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 +169,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. @@ -381,16 +425,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.") @@ -491,13 +539,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 ) @@ -526,13 +577,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 ) @@ -681,6 +735,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}") @@ -894,15 +952,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( @@ -913,29 +966,29 @@ 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 stored = workflow.into(classOf[Workflow]) + val publicCopy = WorkflowPublishService.publicCopyOf(stored) WorkflowWithPrivilege( - workflow.getName, - workflow.getDescription, + publicCopy.name, + publicCopy.description, workflow.getWid, - workflow.getContent, + 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, - 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 @@ -951,7 +1004,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 41f0378579d..faefabb6393 100644 --- a/amber/src/main/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowVersionResource.scala +++ b/amber/src/main/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowVersionResource.scala @@ -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 @@ -114,11 +127,15 @@ object WorkflowVersionResource { * * @param wid */ - def insertNewVersion(wid: Integer, content: String = "[]"): WorkflowVersion = { + def insertNewVersion( + wid: Integer, + content: String = "[]", + ctx: DSLContext = context + ): WorkflowVersion = { val workflowVersion = new WorkflowVersion() workflowVersion.setContent(content) workflowVersion.setWid(wid) - workflowVersionDao.insert(workflowVersion) + new WorkflowVersionDao(ctx.configuration).insert(workflowVersion) workflowVersion } @@ -182,39 +199,45 @@ object WorkflowVersionResource { * @return */ private def encodeVersionImportance( - currentVersions: List[WorkflowVersion] + currentVersions: List[WorkflowVersion], + publicVersionId: Option[Integer] ): List[VersionEntry] = { var impEncodedVersions: List[VersionEntry] = List() + // A pin's anchor is not a version the author made: it carries the identity patch and appears the + // moment they pin. It is always shown, so they can find and restore what the public has, but it + // must not start the aggregation window -- the save it was pinned from is seconds older, and + // would otherwise be folded into it and disappear from the panel. + def isAnchor(version: WorkflowVersion): Boolean = publicVersionId.contains(version.getVid) + val lastVersion = currentVersions.head - var lastVersionTime = lastVersion.getCreationTime + var lastVersionTime: Option[Timestamp] = + if (isAnchor(lastVersion)) None else Some(lastVersion.getCreationTime) impEncodedVersions = impEncodedVersions :+ VersionEntry( lastVersion.getVid, lastVersion.getCreationTime, lastVersion.getContent, - true - ) // the first (latest) - // version is important even if it is positional + true, + isAnchor(lastVersion) + ) // the first (latest) version is important even if it is positional var versionImportance: Boolean = true for (version <- currentVersions.tail) { - if ( - isWithinTimeLimit( - lastVersionTime, - version.getCreationTime - ) - ) { + if (isAnchor(version)) { + versionImportance = true + } else if (lastVersionTime.exists(isWithinTimeLimit(_, version.getCreationTime))) { versionImportance = false } // try reducing unnecessary check of positional versions // because parsing the Json string is expensive else { - lastVersionTime = version.getCreationTime + lastVersionTime = Some(version.getCreationTime) versionImportance = isVersionImportant(version.getContent) } impEncodedVersions = impEncodedVersions :+ VersionEntry( version.getVid, version.getCreationTime, version.getContent, - versionImportance + versionImportance, + isAnchor(version) ) } impEncodedVersions @@ -313,7 +336,10 @@ object WorkflowVersionResource { vId: Integer, creationTime: Timestamp, content: String, - importance: Boolean + importance: Boolean, + // True for the one version the Hub is serving right now, so the author can tell at a glance + // where the public copy sits relative to what they are editing. + isCurrentlyPublic: Boolean ) } @@ -336,7 +362,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( @@ -347,7 +373,8 @@ class WorkflowVersionResource { .orderBy(WORKFLOW_VERSION.CREATION_TIME.desc()) .fetchInto(classOf[WorkflowVersion]) .asScala - .toList + .toList, + Option(workflowDao.fetchOneByWid(wid)).flatMap(w => Option(w.getPublishedVersionId)) ) } } @@ -371,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/UnifiedResourceSchemaSpec.scala b/amber/src/test/scala/org/apache/texera/web/resource/dashboard/UnifiedResourceSchemaSpec.scala index 27856595794..72c4d9eadea 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 28 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,12 @@ 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, + workflowPublishedDefaultView = WORKFLOW.PUBLISHED_DEFAULT_VIEW, + viewerHasGrantedAccess = sentinelGranted ) // Expected projection, in order: alias -> the original it must be built from. @@ -133,13 +139,18 @@ 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, + "workflow_published_default_view" -> WORKFLOW.PUBLISHED_DEFAULT_VIEW, + "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 28 slots as aliases, in the order the UNION ALL depends on" in { + sentinelSchema.allFields should have size 28 sentinelSchema.allFields.map(_.getName) shouldBe expectedProjection.map(_._1) } @@ -159,7 +170,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 28 val rendered = ctx.renderInlined(JDSL.select(defaults.allFields: _*)) rendered should include("'' as \"resourceType\"") rendered should include("cast(null as timestamp) as \"resourceCreationTime\"") @@ -194,12 +205,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 + // 28 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 28 while the translation map does not. val defaults = UnifiedResourceSchema() - defaults.allFields should have size 23 + defaults.allFields should have size 28 translatedAliases(defaults) shouldBe Seq( "resourceType", // DSL.inline("") "resourceCreationTime", // cast(null as timestamp) @@ -211,7 +222,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 28 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 +230,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 28 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 +249,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 28 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..4b6a8a8937b 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,15 @@ 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 publishedDefaultViewField = + WORKFLOW.PUBLISHED_DEFAULT_VIEW.as("workflow_published_default_view") + 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 +88,14 @@ 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, + publishedDefaultView: DefaultViewEnum = null ): Record = { val record = ctx.newRecord( WORKFLOW.WID, @@ -89,7 +105,12 @@ class WorkflowSearchQueryBuilderSpec extends AnyFlatSpec with Matchers { WORKFLOW_USER_ACCESS.PRIVILEGE, USER.NAME, coverField, - defaultViewField + defaultViewField, + driftField, + publishedNameField, + publishedDescriptionField, + publishedDefaultViewField, + grantedField ) record.set(WORKFLOW.WID, wid) record.set(WORKFLOW.NAME, "wf-name") @@ -99,12 +120,75 @@ 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(publishedDefaultViewField, publishedDefaultView) + 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 "serve the pinned view to a viewer without granted access" in { + // A card that opened the frozen copy in the author's live view would offer a Form View over a + // graph that carries no form; the view travels with the name and the description. + val record = translatedRecord( + grantedAccess = java.lang.Boolean.FALSE, + defaultView = DefaultViewEnum.FORM, + publishedName = "as-published", + publishedDefaultView = DefaultViewEnum.CANVAS + ) + + workflowOf(record, viewerUid).workflow.getDefaultView shouldBe DefaultViewEnum.CANVAS + } + + 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 7a40981fed3..9ba7cbcb0a8 100644 --- a/amber/src/test/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowPublishSpec.scala +++ b/amber/src/test/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowPublishSpec.scala @@ -29,22 +29,27 @@ 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 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, listings and search among them, and + * the anchor a pin leaves in the revision history. */ class WorkflowPublishSpec extends AnyFlatSpec @@ -75,6 +80,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"}""" @@ -83,6 +89,7 @@ class WorkflowPublishSpec override protected def beforeAll(): Unit = { initializeDBAndReplaceDSLContext() + FulltextSearchQueryUtils.usePgroonga = false val userDao = new UserDao(getDSLContext.configuration()) userDao.insert(owner) userDao.insert(stranger) @@ -138,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() } @@ -156,6 +166,51 @@ class WorkflowPublishSpec 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 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) @@ -280,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) } @@ -295,11 +354,14 @@ class WorkflowPublishSpec // The "is it published?" check and the pin are two statements. An unpublish landing between // them used to be undone by the pin's own `is_public = true`, putting a workflow the author had // just taken down back on the Hub -- and pinned, so the copy they removed is what the public - // reads. + // reads. Interleaved on the way in: once the pin has the row's lock, an unpublish waits for it. val wid = createWorkflow("pin_cannot_republish") publishPinned(wid) - interleaving(() => workflowResource.makePrivate(wid, ownerSession)) { + interleaving( + () => workflowResource.makePrivate(wid, ownerSession), + at = _.contains("for update") + ) { a[BadRequestException] should be thrownBy workflowResource.pinLatest(wid, ownerSession) } @@ -616,4 +678,688 @@ 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 + } + + 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 public viewer the view the copy on show opens in" in { + // The Form View's definition rides inside the content, so a card offering a form over a frozen + // canvas would open a form the copy does not carry. The view travels with the name and the + // description, and for the author's own row it stays theirs. + val wid = createWorkflow("listing_freezes_the_view") + publishPinned(wid) + getDSLContext + .update(WORKFLOW) + .set(WORKFLOW.DEFAULT_VIEW, DefaultViewEnum.FORM) + .where(WORKFLOW.WID.eq(wid)) + .execute() + + listingOf(strangerSession, wid).workflow.getDefaultView shouldBe DefaultViewEnum.CANVAS + listingOf(ownerSession, wid).workflow.getDefaultView shouldBe DefaultViewEnum.FORM + } + + 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 + } + + behavior of "the revision history" + + it should "leave an anchor in the revision history the author can identify" in { + val wid = createWorkflow("publish_leaves_anchor") + publishPinned(wid) + + val marked = versionResource + .retrieveVersionsOfWorkflow(wid, ownerSession) + .filter(_.isCurrentlyPublic) + marked should have size 1 + marked.head.vId shouldBe workflowDao.fetchOneByWid(wid).getPublishedVersionId + // Both collapsing rules would otherwise hide it, and a hidden anchor is useless to the author. + marked.head.importance shouldBe true + // The revision panel prints this row's creation time and the share dialog prints the pinned + // version's date. They are one row, so they have to be one value: the same version dated a + // second apart in two panels reads as two versions. + marked.head.creationTime shouldBe statusOf(wid).pinnedVersionTime.get + } + + it should "keep the save a pin was taken from visible in the revision panel" in { + // The anchor lands seconds after the save it freezes, and the panel folds versions that land + // close together into the newest of them. Letting the anchor be that newest one would hide the + // author's own save behind a row they never made. + val wid = createWorkflow("pin_does_not_swallow_the_save") + edit(wid, editedContent) + publishPinned(wid) + + val shown = versionResource + .retrieveVersionsOfWorkflow(wid, ownerSession) + .filter(_.importance) + // The anchor, and the save it was taken from. + shown.count(_.isCurrentlyPublic) shouldBe 1 + shown.size should be >= 2 + } + + it should "move the mark when the pin moves, leaving the old anchor unmarked" in { + val wid = createWorkflow("only_the_live_one_is_marked") + publishPinned(wid) + val first = workflowDao.fetchOneByWid(wid).getPublishedVersionId + edit(wid, editedContent) + workflowResource.pinLatest(wid, ownerSession) + val second = workflowDao.fetchOneByWid(wid).getPublishedVersionId + + second should not be first + versionResource + .retrieveVersionsOfWorkflow(wid, ownerSession) + .filter(_.isCurrentlyPublic) + .map(_.vId) shouldBe List(second) + } + + it should "replay the anchor to exactly what was published, however much is edited after" in { + val wid = createWorkflow("anchor_replays_to_published") + publishPinned(wid) + val anchor = workflowDao.fetchOneByWid(wid).getPublishedVersionId + + edit(wid, editedContent) + edit(wid, """{"operators":[],"note":"later_still"}""") + + versionResource + .retrieveWorkflowVersion(wid, anchor, ownerSession) + .getContent shouldBe publishedContent + } + + it should "not record a second anchor when pinning again without having edited" in { + // Otherwise a repeated click would litter the author's revision panel with rows that all replay + // to the same graph. + val wid = createWorkflow("republish_without_edits") + val before = versionResource.retrieveVersionsOfWorkflow(wid, ownerSession).size + publishPinned(wid) + val anchor = workflowDao.fetchOneByWid(wid).getPublishedVersionId + workflowResource.pinLatest(wid, ownerSession) + workflowResource.pinLatest(wid, ownerSession) + + workflowDao.fetchOneByWid(wid).getPublishedVersionId shouldBe anchor + versionResource.retrieveVersionsOfWorkflow(wid, ownerSession) should have size (before + 1) + } + + it should "keep the version the pin was taken from after the pin is dropped" in { + val wid = createWorkflow("unpin_keeps_history") + publishPinned(wid) + + workflowResource.unpin(wid, ownerSession) + + versionResource.retrieveVersionsOfWorkflow(wid, ownerSession) should not be empty + workflowDao.fetchOneByWid(wid).getPublishedVersionId shouldBe null + } + + it should "date the public view by the version on show" in { + // A public viewer is told when what they are looking at was published, not when the author last + // touched a copy they cannot see. + val wid = createWorkflow("public_view_is_dated_by_the_pin") + publishPinned(wid) + val pinnedAt = statusOf(wid).pinnedVersionTime.get + + edit(wid, editedContent) + + workflowResource.retrievePublicWorkflow(wid).lastModifiedTime shouldBe pinnedAt + } } diff --git a/amber/src/test/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowVersionResourceSpec.scala b/amber/src/test/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowVersionResourceSpec.scala index 91b0b1c1be0..35475ec9ca3 100644 --- a/amber/src/test/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowVersionResourceSpec.scala +++ b/amber/src/test/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowVersionResourceSpec.scala @@ -551,10 +551,40 @@ class WorkflowVersionResourceSpec version(3, 0L, positional), // latest — important regardless of content version(2, TimeUnit.SECONDS.toMillis(1), meaningful), // within the aggregate window version(1, TimeUnit.DAYS.toMillis(30), meaningful) // outside it, judged on content - ) + ), + Option.empty[Integer] ) encoded.map(_.vId) shouldBe List(3, 2, 1) encoded.map(_.importance) shouldBe List(true, false, true) + encoded.map(_.isCurrentlyPublic) shouldBe List(false, false, false) + } + + it should "keep the version on public show visible even when the rules would collapse it" in { + val base = 1_700_000_000_000L + def version(vid: Int, offsetMillis: Long, content: String): WorkflowVersion = { + val v = new WorkflowVersion + v.setVid(vid) + v.setWid(testWorkflowWid) + v.setContent(content) + v.setCreationTime(new Timestamp(base - offsetMillis)) + v + } + + val positional = """[{"op":"replace","path":"/operatorPositions/op-1/x","value":5}]""" + + // Version 2 lands inside the aggregation window and carries a positional-only + // patch, so both collapsing rules would hide it. Being the published one wins. + val encoded = WorkflowVersionResource invokePrivate encodeVersionImportance( + List( + version(3, 0L, positional), + version(2, TimeUnit.SECONDS.toMillis(1), positional), + version(1, TimeUnit.SECONDS.toMillis(2), positional) + ), + Option(Integer.valueOf(2)) + ) + + encoded.map(_.importance) shouldBe List(true, true, false) + encoded.map(_.isCurrentlyPublic) shouldBe List(false, true, false) } } diff --git a/common/config/src/main/resources/gui.conf b/common/config/src/main/resources/gui.conf index 7cb7fff6555..93f7f175d97 100644 --- a/common/config/src/main/resources/gui.conf +++ b/common/config/src/main/resources/gui.conf @@ -95,6 +95,11 @@ gui { form-view-enabled = true form-view-enabled = ${?GUI_WORKFLOW_WORKSPACE_FORM_VIEW_ENABLED} + # whether an author may pin a version as the public copy of a published workflow, instead of + # the public always following their latest content + version-pinning-enabled = true + version-pinning-enabled = ${?GUI_WORKFLOW_WORKSPACE_VERSION_PINNING_ENABLED} + # Whether to connect to local or production shared editing server. Set to true if you have # reverse proxy set up for y-websocket. production-shared-editing-server = false diff --git a/common/config/src/main/scala/org/apache/texera/common/config/GuiConfig.scala b/common/config/src/main/scala/org/apache/texera/common/config/GuiConfig.scala index 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..6279d6d3ccb 100644 --- a/common/config/src/test/scala/org/apache/texera/common/config/GuiConfigSpec.scala +++ b/common/config/src/test/scala/org/apache/texera/common/config/GuiConfigSpec.scala @@ -66,6 +66,10 @@ class GuiConfigSpec extends AnyFlatSpec with Matchers { ifUnset("GUI_WORKFLOW_WORKSPACE_FORM_VIEW_ENABLED")( GuiConfig.guiWorkflowWorkspaceFormViewEnabled shouldBe true ) + // Version pinning is on by default now that the whole feature has landed. + ifUnset("GUI_WORKFLOW_WORKSPACE_VERSION_PINNING_ENABLED")( + GuiConfig.guiWorkflowWorkspaceVersionPinningEnabled shouldBe true + ) ifUnset("GUI_WORKFLOW_WORKSPACE_PRODUCTION_SHARED_EDITING_SERVER")( GuiConfig.guiWorkflowWorkspaceProductionSharedEditingServer shouldBe false ) diff --git a/config-service/src/main/scala/org/apache/texera/service/resource/ConfigResource.scala b/config-service/src/main/scala/org/apache/texera/service/resource/ConfigResource.scala index d682fe8d026..219b82b87c9 100644 --- a/config-service/src/main/scala/org/apache/texera/service/resource/ConfigResource.scala +++ b/config-service/src/main/scala/org/apache/texera/service/resource/ConfigResource.scala @@ -92,6 +92,7 @@ class ConfigResource { "asyncRenderingEnabled" -> GuiConfig.guiWorkflowWorkspaceAsyncRenderingEnabled, "timetravelEnabled" -> GuiConfig.guiWorkflowWorkspaceTimetravelEnabled, "formViewEnabled" -> GuiConfig.guiWorkflowWorkspaceFormViewEnabled, + "versionPinningEnabled" -> GuiConfig.guiWorkflowWorkspaceVersionPinningEnabled, "productionSharedEditingServer" -> GuiConfig.guiWorkflowWorkspaceProductionSharedEditingServer, "defaultExecutionMode" -> GuiConfig.guiWorkflowWorkspaceDefaultExecutionMode, "workflowEmailNotificationEnabled" -> GuiConfig.guiWorkflowWorkspaceWorkflowEmailNotificationEnabled, diff --git a/frontend/src/app/common/service/gui-config.service.mock.ts b/frontend/src/app/common/service/gui-config.service.mock.ts index 8591fc727a9..2a3f43b02b2 100644 --- a/frontend/src/app/common/service/gui-config.service.mock.ts +++ b/frontend/src/app/common/service/gui-config.service.mock.ts @@ -43,6 +43,7 @@ export class MockGuiConfigService { asyncRenderingEnabled: false, timetravelEnabled: false, formViewEnabled: false, + versionPinningEnabled: false, productionSharedEditingServer: false, pythonLanguageServerPort: "3000", defaultDataTransferBatchSize: 100, diff --git a/frontend/src/app/common/service/workflow-persist/workflow-persist.service.spec.ts b/frontend/src/app/common/service/workflow-persist/workflow-persist.service.spec.ts index 7773458b5f1..c5246cb57f1 100644 --- a/frontend/src/app/common/service/workflow-persist/workflow-persist.service.spec.ts +++ b/frontend/src/app/common/service/workflow-persist/workflow-persist.service.spec.ts @@ -21,6 +21,7 @@ import { TestBed } from "@angular/core/testing"; import { HttpClientTestingModule, HttpTestingController } from "@angular/common/http/testing"; import { WorkflowPersistService, + WorkflowPublishStatus, WORKFLOW_BASE_URL, WORKFLOW_ID_URL, WORKFLOW_OWNER_URL, @@ -99,6 +100,68 @@ describe("WorkflowPersistService", () => { httpTestingController.expectOne(request => request.method === "POST"); }); + it("should publish without pinning anything", () => { + service.updateWorkflowIsPublished(7, true).subscribe(); + const request = httpTestingController.expectOne(req => req.url.endsWith("workflow/public/7")); + expect(request.request.method).toEqual("PUT"); + }); + + it("should pin the latest version through the pin endpoint", () => { + let received: WorkflowPublishStatus | undefined; + service.pinLatestVersion(7).subscribe(status => (received = status)); + + const request = httpTestingController.expectOne(req => req.url.endsWith("workflow/pin/7")); + expect(request.request.method).toEqual("POST"); + request.flush({ + isPublished: true, + isPinned: true, + pinnedVersionTime: 1700000000000, + hasUnpublishedChanges: false, + }); + + expect(received?.pinnedVersionTime).toEqual(1700000000000); + expect(received?.hasUnpublishedChanges).toBe(false); + }); + + it("should drop the pin through the same path with DELETE", () => { + let received: WorkflowPublishStatus | undefined; + service.unpinVersion(7).subscribe(status => (received = status)); + + const request = httpTestingController.expectOne(req => req.url.endsWith("workflow/pin/7")); + expect(request.request.method).toEqual("DELETE"); + request.flush({ isPublished: true, isPinned: false, hasUnpublishedChanges: false }); + + expect(received?.isPinned).toBe(false); + }); + + it("should announce a save once it lands, not when it is sent", () => { + // Panels that describe the saved copy re-read on this. Announcing on the request rather than the + // response would have them re-read the state the save was about to replace. + let announced = 0; + service.getWorkflowPersistedStream().subscribe(() => announced++); + + const workflow = { wid: 7, name: "n", content: { operators: [], links: [] } } as unknown as Workflow; + service.persistWorkflow(workflow).subscribe(); + const request = httpTestingController.expectOne(req => req.url.endsWith("workflow/persist")); + expect(announced).toBe(0); + + request.flush({ wid: 7, name: "n", content: '{"operators":[]}' }); + + expect(announced).toBe(1); + }); + + it("should report unpublished changes from the publish status endpoint", () => { + let received: WorkflowPublishStatus | undefined; + service.getPublishStatus(7).subscribe(status => (received = status)); + + const request = httpTestingController.expectOne(req => req.url.endsWith("workflow/publish-status/7")); + expect(request.request.method).toEqual("GET"); + request.flush({ isPublished: true, isPinned: true, hasUnpublishedChanges: true }); + + expect(received?.isPinned).toBe(true); + expect(received?.hasUnpublishedChanges).toBe(true); + }); + it("should check if workflow content and name returned correctly", () => { service .createWorkflow(jsonCast(testContent), "testname") diff --git a/frontend/src/app/common/service/workflow-persist/workflow-persist.service.ts b/frontend/src/app/common/service/workflow-persist/workflow-persist.service.ts index fa61b112008..59e81ffb503 100644 --- a/frontend/src/app/common/service/workflow-persist/workflow-persist.service.ts +++ b/frontend/src/app/common/service/workflow-persist/workflow-persist.service.ts @@ -53,6 +53,17 @@ export const WORKFLOW_SET_DEFAULT_VIEW_URL = WORKFLOW_BASE_URL + "/set-default-v export const DEFAULT_WORKFLOW_NAME = "Untitled workflow"; +/** A published workflow follows the author's latest until they pin a version. */ +export interface WorkflowPublishStatus { + isPublished: boolean; + /** Whether a version is pinned. False means the public follows the author's latest. */ + isPinned: boolean; + /** When the pinned version was created, which is how the dialog names it. */ + pinnedVersionTime?: number; + /** Whether a pin is holding edits back. Always false while following. */ + hasUnpublishedChanges: boolean; +} + @Injectable({ providedIn: "root", }) @@ -85,6 +96,9 @@ export class WorkflowPersistService { private pendingSaves = 0; private readonly savesDrained = new Subject(); + /** Fires when a save lands: the write, not the keystroke, is what changes the saved copy. */ + private workflowPersisted = new Subject(); + constructor( private http: HttpClient, private notificationService: NotificationService, @@ -172,7 +186,8 @@ export class WorkflowPersistService { }) .pipe( filter((updatedWorkflow: Workflow) => updatedWorkflow != null), - map(WorkflowUtilService.parseWorkflowInfo) + map(WorkflowUtilService.parseWorkflowInfo), + tap(() => this.workflowPersisted.next()) ); // Replayed, so a caller that subscribes after the queue has already relayed the outcome (a // save that was quick, or a synchronous test double) still receives it. @@ -182,6 +197,11 @@ export class WorkflowPersistService { return result.asObservable(); } + /** Emits when a save lands, so panels describing the saved copy can re-read it. */ + public getWorkflowPersistedStream(): Observable { + return this.workflowPersisted.asObservable(); + } + /** * creates a workflow and insert it to backend database and return its information * @param newWorkflowName @@ -274,6 +294,8 @@ export class WorkflowPersistService { name: name, }) .pipe( + // A pin freezes the name too, so a rename is a save like any other. + tap(() => this.workflowPersisted.next()), catchError((error: unknown) => { // @ts-ignore this.notificationService.error(error.error.message); @@ -292,6 +314,8 @@ export class WorkflowPersistService { description: description, }) .pipe( + // Frozen by a pin like the name and the canvas are. + tap(() => this.workflowPersisted.next()), catchError((error: unknown) => { // @ts-ignore this.notificationService.error(error.error.message); @@ -304,6 +328,10 @@ export class WorkflowPersistService { return this.http.get(`${AppSettings.getApiEndpoint()}/${WORKFLOW_BASE_URL}/type/${wid}`, { responseType: "text" }); } + /** + * Publishes the workflow, or unpublishes it. A published workflow follows the author's latest + * content until a version is pinned; see {@link pinLatestVersion}. + */ public updateWorkflowIsPublished(wid: number, isPublished: boolean): Observable { if (isPublished) { return this.http.put(`${AppSettings.getApiEndpoint()}/${WORKFLOW_BASE_URL}/public/${wid}`, null); @@ -312,6 +340,26 @@ export class WorkflowPersistService { } } + /** Pins the author's current version as the public copy, so later edits stop reaching the public. */ + public pinLatestVersion(wid: number): Observable { + return this.http.post( + `${AppSettings.getApiEndpoint()}/${WORKFLOW_BASE_URL}/pin/${wid}`, + null + ); + } + + /** Drops the pin, so the public follows the author's latest content again. */ + public unpinVersion(wid: number): Observable { + return this.http.delete(`${AppSettings.getApiEndpoint()}/${WORKFLOW_BASE_URL}/pin/${wid}`); + } + + /** Whether the workflow is published, whether a version is pinned, and whether it holds edits back. */ + public getPublishStatus(wid: number): Observable { + return this.http.get( + `${AppSettings.getApiEndpoint()}/${WORKFLOW_BASE_URL}/publish-status/${wid}` + ); + } + public setWorkflowPersistFlag(flag: boolean): void { this.workflowPersistFlag = flag; } diff --git a/frontend/src/app/common/type/gui-config.ts b/frontend/src/app/common/type/gui-config.ts index 5b8b23db03d..d24c4805bfb 100644 --- a/frontend/src/app/common/type/gui-config.ts +++ b/frontend/src/app/common/type/gui-config.ts @@ -34,6 +34,7 @@ export interface GuiConfig { asyncRenderingEnabled: boolean; timetravelEnabled: boolean; formViewEnabled: boolean; + versionPinningEnabled: boolean; productionSharedEditingServer: boolean; pythonLanguageServerPort: string; defaultDataTransferBatchSize: number; diff --git a/frontend/src/app/dashboard/component/user/list-item/card-item/card-item.component.spec.ts b/frontend/src/app/dashboard/component/user/list-item/card-item/card-item.component.spec.ts index 4aa3be86299..9da4d356dea 100644 --- a/frontend/src/app/dashboard/component/user/list-item/card-item/card-item.component.spec.ts +++ b/frontend/src/app/dashboard/component/user/list-item/card-item/card-item.component.spec.ts @@ -230,6 +230,21 @@ describe("CardItemComponent", () => { expect(component.entryLink).toEqual([HUB_WORKFLOW_RESULT_DETAIL, "7"]); }); + it("should route an owner to the hub detail view when the public copy is behind", () => { + // The hub entry advertises the pinned version, so it must open the preview of that version -- + // where the author can clone it -- rather than the editor holding something else. + component.currentUid = 42; + component.entry = makeWorkflowEntry({ id: 7, accessibleUserIds: [42] }); + (component.entry.workflow as any).hasUnpublishedChanges = true; + component.ngOnChanges({ entry: { currentValue: component.entry } as any }); + expect(component.entryLink).toEqual([HUB_WORKFLOW_RESULT_DETAIL, "7"]); + + // ...but their own listings still open the editor. + component.isPrivateSearch = true; + component.ngOnChanges({ entry: { currentValue: component.entry } as any }); + expect(component.entryLink).toEqual([USER_WORKSPACE, "7"]); + }); + it("should format counts as kilo for values >= 1000", () => { expect(component.formatCount(999)).toBe("999"); expect(component.formatCount(1500)).toBe("1.5k"); diff --git a/frontend/src/app/dashboard/component/user/list-item/card-item/card-item.component.ts b/frontend/src/app/dashboard/component/user/list-item/card-item/card-item.component.ts index a35b4742080..44e657cc935 100644 --- a/frontend/src/app/dashboard/component/user/list-item/card-item/card-item.component.ts +++ b/frontend/src/app/dashboard/component/user/list-item/card-item/card-item.component.ts @@ -235,7 +235,7 @@ export class CardItemComponent implements OnChanges { this.disableDelete = !descriptor.isOwner(this.entry); this.canDownload = descriptor.download !== undefined; this.canShare = descriptor.retrieveOwners !== undefined; - this.entryLink = this.resourceRegistry.entryLink(this.entry, this.currentUid); + this.entryLink = this.resourceRegistry.entryLink(this.entry, this.currentUid, this.isPrivateSearch); // Same landing rule as the list row (default-view-landing.ts): a form-default workflow shows // the Form View icon and opens in its form, so the card and the row never disagree. this.defaultsToForm = defaultsToFormView(this.entry, this.config.env.formViewEnabled); diff --git a/frontend/src/app/dashboard/component/user/list-item/list-item.component.spec.ts b/frontend/src/app/dashboard/component/user/list-item/list-item.component.spec.ts index 997c0996b96..68d95062d95 100644 --- a/frontend/src/app/dashboard/component/user/list-item/list-item.component.spec.ts +++ b/frontend/src/app/dashboard/component/user/list-item/list-item.component.spec.ts @@ -314,6 +314,35 @@ describe("ListItemComponent", () => { expect(component.entryLink).toEqual([HUB_WORKFLOW_RESULT_DETAIL, "101"]); }); + it("routes an owned workflow whose public copy is behind to the hub detail page", () => { + // The hub entry advertises the pinned version, so it must open the preview of that version -- + // where the author can clone it -- rather than the editor holding something else. + component.currentUid = 1; + component.entry = { + id: 102, + type: "workflow", + workflow: { isOwner: true, hasUnpublishedChanges: true }, + accessibleUserIds: [1], + ...baseStats, + } as unknown as DashboardEntry; + component.initializeEntry(); + expect(component.entryLink).toEqual([HUB_WORKFLOW_RESULT_DETAIL, "102"]); + }); + + it("keeps the author's own listings pointing at the editor", () => { + component.currentUid = 1; + component.isPrivateSearch = true; + component.entry = { + id: 103, + type: "workflow", + workflow: { isOwner: true, hasUnpublishedChanges: true }, + accessibleUserIds: [1], + ...baseStats, + } as unknown as DashboardEntry; + component.initializeEntry(); + expect(component.entryLink).toEqual([USER_WORKSPACE, "103"]); + }); + it("routes owned datasets to the user dataset page", () => { component.currentUid = 1; component.entry = { diff --git a/frontend/src/app/dashboard/component/user/list-item/list-item.component.ts b/frontend/src/app/dashboard/component/user/list-item/list-item.component.ts index 5f35b3d2896..35ec29c1fe7 100644 --- a/frontend/src/app/dashboard/component/user/list-item/list-item.component.ts +++ b/frontend/src/app/dashboard/component/user/list-item/list-item.component.ts @@ -151,7 +151,7 @@ export class ListItemComponent implements OnChanges { this.disableDelete = !descriptor.isOwner(this.entry); this.canDownload = descriptor.download !== undefined; this.canShare = descriptor.retrieveOwners !== undefined; - this.entryLink = this.resourceRegistry.entryLink(this.entry, this.currentUid); + this.entryLink = this.resourceRegistry.entryLink(this.entry, this.currentUid, this.isPrivateSearch); if (descriptor.hasSize && typeof this.entry.id === "number") { this.size = this.entry.size; } diff --git a/frontend/src/app/dashboard/component/user/share-access/share-access.component.html b/frontend/src/app/dashboard/component/user/share-access/share-access.component.html index 8128f333633..8fd1b38a242 100644 --- a/frontend/src/app/dashboard/component/user/share-access/share-access.component.html +++ b/frontend/src/app/dashboard/component/user/share-access/share-access.component.html @@ -55,6 +55,72 @@ + +
+ + + + +
+ + + + The public is on an older version. + The public sees your current version, pinned. + The public sees your latest, updated on every save. + +
+ + + +
+
+
+ Pinned to {{ publishStatus.pinnedVersionTime | date: publicationTimeFormat }} +
+
Your later edits stay private
+
+ +
+ +
+ Frozen as you keep editing. Restore it any time from the version panel, where it is marked + Currently public. +
+ + +
+
diff --git a/frontend/src/app/dashboard/component/user/share-access/share-access.component.scss b/frontend/src/app/dashboard/component/user/share-access/share-access.component.scss index 1a74945e159..401747d706b 100644 --- a/frontend/src/app/dashboard/component/user/share-access/share-access.component.scss +++ b/frontend/src/app/dashboard/component/user/share-access/share-access.component.scss @@ -36,25 +36,163 @@ line-height: 1; } -.access-button-group { - display: flex; - flex-direction: row; - justify-content: space-evenly; - align-items: center; -} - .access-button { display: flex; + flex: 1; flex-direction: column; align-items: center; justify-content: space-evenly; height: 150px; - width: 300px; padding: 20px; - margin-bottom: 24px; border-radius: 10px; } +// Laid out with a gap rather than space-evenly so the tiles span the full width of the dialog body. +// space-evenly left them inset, which the full-width publish-state strip below could not line up with. +.access-button-group { + display: flex; + gap: 24px; + align-items: stretch; + margin-bottom: 24px; +} + +.access-button-group:has(+ .publish-anchor) { + margin-bottom: 8px; +} + +// The publish panel. Nested under one root so nothing here reaches the Private/Public tiles, the +// invite form or the access list around it. +.publish-anchor { + margin-bottom: 24px; + + // Sized to the panel's own scale; everything else about the switch is the component's. + // ::ng-deep because these are the control's own nodes: our styles are scoped to this component, + // and its inner DOM belongs to ng-zorro. + .publish-seg ::ng-deep { + font-size: 13.5px; + // Between antd's default and a pill: square enough to sit with the tiles above, round enough to + // echo the card below, whose corners are 12px. + border-radius: 12px; + + .ant-segmented-item, + .ant-segmented-thumb { + border-radius: 9px; + } + + .ant-segmented-item-label { + padding: 3px 16px; + } + + // Quicker than the default: the thumb only confirms a choice the author already made, and a + // slide they have to wait out reads as the app thinking rather than as an answer. + .ant-segmented-thumb { + transition-duration: 0.16s; + } + + // The side in force takes the colour of the state it puts the workflow in -- green while the + // public follows along, amber once a version is held. Without it the only mark is a white tile, + // which is easy to miss at a glance. + .ant-segmented-item-label[aria-selected="true"] { + font-weight: 600; + color: #12a05e; + } + } + + // Amber once a version is held, matching the dot and the sentence below. + &:not([data-state="follow"]) .publish-seg ::ng-deep .ant-segmented-item-label[aria-selected="true"] { + color: #e0951c; + } + + .publish-status { + display: flex; + align-items: center; + gap: 12px; + padding-top: 16px; + } + + .publish-say { + flex: 1; + min-width: 0; + font-size: 14px; + line-height: 1.45; + color: #5f666f; + + b { + font-weight: 580; + color: #1c1f26; + } + } + + // The one state that leaves the author something to decide gets a card: which version is public, + // what it costs them, and the act that ends it -- in that order, left to right. + .publish-card { + display: flex; + align-items: center; + gap: 12px; + margin-top: 12px; + padding: 12px 12px 12px 16px; + background: #f6f7f9; + border: 1px solid #e6e9ee; + border-radius: 12px; + } + + .publish-card-text { + flex: 1; + min-width: 0; + } + + .publish-card-title { + font-size: 13px; + font-weight: 580; + color: #1c1f26; + // Digits of equal width, so a date that ticks does not shuffle the words around it. + font-variant-numeric: tabular-nums; + } + + .publish-card-meta { + margin-top: 2px; + font-size: 11.5px; + line-height: 1.3; + color: #9299a2; + } + + .publish-card-action { + flex: none; + padding: 9px 14px; + border: none; + border-radius: 8px; + background: #ddeafe; + color: #2f74f0; + font: inherit; + font-size: 13px; + font-weight: 580; + white-space: nowrap; + cursor: pointer; + transition: 0.14s; + + &:hover:not([disabled]) { + background: #d0e1fd; + } + + &[disabled] { + cursor: default; + opacity: 0.55; + } + } + + .publish-hint { + margin-top: 12px; + font-size: 12.5px; + line-height: 1.5; + color: #9299a2; + + b { + font-weight: 540; + color: #5f666f; + } + } +} + .button-icon { font-size: 40px; } diff --git a/frontend/src/app/dashboard/component/user/share-access/share-access.component.spec.ts b/frontend/src/app/dashboard/component/user/share-access/share-access.component.spec.ts index ec41db898c2..675baa9d5ef 100644 --- a/frontend/src/app/dashboard/component/user/share-access/share-access.component.spec.ts +++ b/frontend/src/app/dashboard/component/user/share-access/share-access.component.spec.ts @@ -22,7 +22,7 @@ import { HttpClientTestingModule } from "@angular/common/http/testing"; import { NoopAnimationsModule } from "@angular/platform-browser/animations"; import { By } from "@angular/platform-browser"; import { HttpErrorResponse } from "@angular/common/http"; -import { of, throwError } from "rxjs"; +import { Subject, of, throwError } from "rxjs"; import { NZ_MODAL_DATA, NzModalRef, NzModalService } from "ng-zorro-antd/modal"; import { NzMessageService } from "ng-zorro-antd/message"; @@ -35,6 +35,7 @@ import { NotificationService } from "../../../../common/service/notification/not import { DatasetService } from "../../../service/user/dataset/dataset.service"; import { WorkflowPersistService } from "src/app/common/service/workflow-persist/workflow-persist.service"; import { WorkflowActionService } from "src/app/workspace/service/workflow-graph/model/workflow-action.service"; +import { GuiConfigService } from "src/app/common/service/gui-config.service"; import { Privilege } from "../../../type/share-access.interface"; import { ResourceRegistryService } from "../../../service/user/resource-registry/resource-registry.service"; import { ResourceDescriptor } from "../../../type/resource-descriptor"; @@ -47,6 +48,7 @@ interface SetupOptions { currentEmail?: string | undefined; /** What the registry answers for the kind, in place of the shipped descriptor. */ descriptor?: ResourceDescriptor; + versionPinningEnabled?: boolean; } /** @@ -71,7 +73,7 @@ describe("ShareAccessComponent", () => { let messageSpy: { error: ReturnType }; let modalRefSpy: { close: ReturnType }; let modalServiceSpy: { create: ReturnType }; - let workflowPersistSpy: { + let workflowPersistSpy: any /* the publish-status spies vary per test */ & { getWorkflowIsPublished: ReturnType; updateWorkflowIsPublished: ReturnType; }; @@ -79,7 +81,10 @@ describe("ShareAccessComponent", () => { getDataset: ReturnType; updateDatasetPublicity: ReturnType; }; - let workflowActionSpy: { setWorkflowIsPublished: ReturnType }; + let workflowActionSpy: { + setWorkflowIsPublished: ReturnType; + getWorkflow: ReturnType; + }; let userServiceCurrentEmail: string | undefined; // The component reads publicity back after writing it, so the doubles have to hold state: // the workflow endpoint sets it absolutely, the dataset one toggles. @@ -87,6 +92,8 @@ describe("ShareAccessComponent", () => { let datasetPublished: boolean; let capturedModalConfigs: any[]; /** The NzModalRef stubs handed back by modalService.create, in creation order. */ + /** Stands in for the editor's autosave landing, which is what the panel has to follow. */ + let persisted: Subject; let capturedModalRefs: { close: ReturnType }[]; /** * The fixture built by the most recent setupComponent() call, for the template-level tests. @@ -96,7 +103,15 @@ describe("ShareAccessComponent", () => { let fixture: ComponentFixture; function setupComponent(opts: SetupOptions = {}): ShareAccessComponent { - const { type = "workflow", id = 1, inWorkspace = false, currentEmail = "me@example.com" } = opts; + const { + type = "workflow", + id = 1, + inWorkspace = false, + currentEmail = "me@example.com", + // On for the cases that are about the panel; the feature ships behind this flag, and the + // case below covers what the dialog looks like while it is off. + versionPinningEnabled = true, + } = opts; userServiceCurrentEmail = currentEmail; TestBed.configureTestingModule({ @@ -118,6 +133,7 @@ describe("ShareAccessComponent", () => { { provide: WorkflowPersistService, useValue: workflowPersistSpy }, { provide: DatasetService, useValue: datasetServiceSpy }, { provide: WorkflowActionService, useValue: workflowActionSpy }, + { provide: GuiConfigService, useValue: { env: { versionPinningEnabled } } }, ], }); if (opts.descriptor) { @@ -128,9 +144,34 @@ describe("ShareAccessComponent", () => { return fixture.componentInstance; } + /** Everything the publish panel renders, for the assertions about what the author actually reads. */ + function publishLineText(): string { + fixture.detectChanges(); + return (fixture.nativeElement.querySelector(".publish-anchor")?.textContent ?? "").replace(/\s+/g, " ").trim(); + } + + /** + * The two sides of the switch. Which one is highlighted is the control's own business -- it paints + * the selection from an animation frame, which this environment never runs -- so these assert the + * labels and leave the state itself to `publishState`. + */ + function segments(): string[] { + fixture.detectChanges(); + return Array.from(fixture.nativeElement.querySelectorAll(".ant-segmented-item-label")).map((b: any) => + (b.textContent ?? "").trim() + ); + } + + /** The card under the line, which only a pin holding edits back puts on screen. */ + function publishNoteText(): string { + fixture.detectChanges(); + return (fixture.nativeElement.querySelector(".publish-card")?.textContent ?? "").replace(/\s+/g, " ").trim(); + } + beforeEach(() => { TestBed.resetTestingModule(); fixture = undefined as unknown as ComponentFixture; + persisted = new Subject(); capturedModalConfigs = []; capturedModalRefs = []; gmailSpy = { sendEmail: vi.fn() }; @@ -153,12 +194,19 @@ describe("ShareAccessComponent", () => { }; workflowPublished = false; datasetPublished = false; + const followingStatus = { isPublished: true, isPinned: false, hasUnpublishedChanges: false }; workflowPersistSpy = { getWorkflowIsPublished: vi.fn(() => of(workflowPublished ? "Public" : "Private")), updateWorkflowIsPublished: vi.fn((_id: number, next: boolean) => { workflowPublished = next; return of(null); }), + getPublishStatus: vi.fn().mockReturnValue(of(followingStatus)), + getWorkflowPersistedStream: vi.fn().mockReturnValue(persisted.asObservable()), + isWorkflowPersistEnabled: vi.fn().mockReturnValue(true), + persistWorkflow: vi.fn().mockReturnValue(of({ wid: 3 })), + pinLatestVersion: vi.fn().mockReturnValue(of({ ...followingStatus, isPinned: true })), + unpinVersion: vi.fn().mockReturnValue(of(followingStatus)), }; datasetServiceSpy = { getDataset: vi.fn(() => of({ dataset: { isPublic: datasetPublished } })), @@ -167,7 +215,11 @@ describe("ShareAccessComponent", () => { return of(null); }), }; - workflowActionSpy = { setWorkflowIsPublished: vi.fn() }; + workflowActionSpy = { + setWorkflowIsPublished: vi.fn(), + // The publish panel never writes; it only re-reads once a save of the author's own lands. + getWorkflow: vi.fn().mockReturnValue({ wid: 3, name: "w", content: { operators: [], links: [] } }), + }; }); function getFooterButton(config: any, label: string): { onClick: () => void } { @@ -1127,4 +1179,340 @@ describe("ShareAccessComponent", () => { expect(datasetServiceSpy.updateDatasetPublicity).not.toHaveBeenCalled(); }); }); + + describe("publish state line", () => { + // The owner of the workflow, so hasWriteAccess is true. + const asOwner = { currentEmail: "owner@example.com" }; + + it("does not fetch publish status for an unpublished workflow", () => { + workflowPersistSpy.getWorkflowIsPublished.mockReturnValue(of("Private")); + const c = setupComponent({ type: "workflow", id: 3, ...asOwner }); + expect(workflowPersistSpy.getPublishStatus).not.toHaveBeenCalled(); + expect(c.publishStatus).toBeUndefined(); + }); + + it("reports unpublished changes on a pinned workflow", () => { + workflowPersistSpy.getWorkflowIsPublished.mockReturnValue(of("Public")); + workflowPersistSpy.getPublishStatus.mockReturnValue( + of({ isPublished: true, isPinned: true, hasUnpublishedChanges: true }) + ); + const c = setupComponent({ type: "workflow", id: 3, ...asOwner }); + expect(workflowPersistSpy.getPublishStatus).toHaveBeenCalledWith(3); + expect(c.publishStatus?.hasUnpublishedChanges).toBe(true); + }); + + it("does not fetch publish status for a viewer who cannot publish", () => { + workflowPersistSpy.getWorkflowIsPublished.mockReturnValue(of("Public")); + accessServiceSpy.getAccessList.mockReturnValue( + of([{ email: "reader@example.com", name: "R", privilege: Privilege.READ }]) + ); + const c = setupComponent({ type: "workflow", id: 3, currentEmail: "reader@example.com" }); + expect(workflowPersistSpy.getPublishStatus).not.toHaveBeenCalled(); + expect(c.publishStatus).toBeUndefined(); + }); + + it("clears the pending state after moving the pin forward", () => { + workflowPersistSpy.getWorkflowIsPublished.mockReturnValue(of("Public")); + workflowPersistSpy.getPublishStatus.mockReturnValue( + of({ isPublished: true, isPinned: true, hasUnpublishedChanges: true }) + ); + workflowPersistSpy.pinLatestVersion.mockReturnValue( + of({ isPublished: true, isPinned: true, hasUnpublishedChanges: false }) + ); + const c = setupComponent({ type: "workflow", id: 3, ...asOwner }); + + c.choosePublicCopy(true, true); + + expect(workflowPersistSpy.pinLatestVersion).toHaveBeenCalledWith(3); + expect(c.publishStatus?.hasUnpublishedChanges).toBe(false); + expect(c.isPinning).toBe(false); + }); + + it("goes back to following when the pin is dropped", () => { + workflowPersistSpy.getWorkflowIsPublished.mockReturnValue(of("Public")); + workflowPersistSpy.getPublishStatus.mockReturnValue( + of({ isPublished: true, isPinned: true, hasUnpublishedChanges: true }) + ); + workflowPersistSpy.unpinVersion.mockReturnValue( + of({ isPublished: true, isPinned: false, hasUnpublishedChanges: false }) + ); + const c = setupComponent({ type: "workflow", id: 3, ...asOwner }); + + c.choosePublicCopy(false); + + expect(workflowPersistSpy.unpinVersion).toHaveBeenCalledWith(3); + expect(c.publishStatus?.isPinned).toBe(false); + // Nothing is held back once the public follows the latest. + expect(c.publishStatus?.hasUnpublishedChanges).toBe(false); + expect(c.isPinning).toBe(false); + }); + + it("names the pinned version by its date once the author has moved on", () => { + // The date is how the same version is named in the revision panel, which is where the author + // goes to restore it. Naming it any other way would leave them matching two descriptions. + workflowPersistSpy.getWorkflowIsPublished.mockReturnValue(of("Public")); + workflowPersistSpy.getPublishStatus.mockReturnValue( + of({ + isPublished: true, + isPinned: true, + hasUnpublishedChanges: true, + pinnedVersionTime: Date.parse("2026-08-12T23:59:00"), + }) + ); + setupComponent({ type: "workflow", id: 3, ...asOwner }); + + expect(publishLineText()).toContain("Pinned to Aug 12, 23:59:00"); + expect(publishLineText()).toContain("Your later edits stay private"); + }); + + it("says the pinned copy is the current one rather than dating it", () => { + // A date here would invite the author to work out whether it is still what they have; saying + // so is the answer to the question they would be asking. + workflowPersistSpy.getWorkflowIsPublished.mockReturnValue(of("Public")); + workflowPersistSpy.getPublishStatus.mockReturnValue( + of({ + isPublished: true, + isPinned: true, + hasUnpublishedChanges: false, + pinnedVersionTime: Date.parse("2026-08-12T23:59:00"), + }) + ); + setupComponent({ type: "workflow", id: 3, ...asOwner }); + + expect(publishLineText()).toContain("your current version"); + expect(publishLineText()).not.toContain("Aug 12"); + }); + + it("prints the pinned time to the second, as the revision panel does", () => { + // The line names the version the revision panel marks, so the two must print it identically -- + // one format, never varied, is what guarantees that. + workflowPersistSpy.getWorkflowIsPublished.mockReturnValue(of("Public")); + workflowPersistSpy.getPublishStatus.mockReturnValue( + of({ + isPublished: true, + isPinned: true, + hasUnpublishedChanges: true, + pinnedVersionTime: Date.parse("2026-08-12T23:58:51"), + }) + ); + setupComponent({ type: "workflow", id: 3, ...asOwner }); + + expect(publishLineText()).toContain("Aug 12, 23:58:51"); + }); + + it("says the same thing about both states, with only the answer changing", () => { + workflowPersistSpy.getWorkflowIsPublished.mockReturnValue(of("Public")); + workflowPersistSpy.getPublishStatus.mockReturnValue( + of({ isPublished: true, isPinned: false, hasUnpublishedChanges: false }) + ); + setupComponent({ type: "workflow", id: 3, ...asOwner }); + + expect(publishLineText()).toContain("The public sees your latest"); + expect(segments()).toEqual(["Follow latest", "Pinned"]); + }); + + it("drops the line when the workflow is unpublished", () => { + workflowPublished = true; + const c = setupComponent({ type: "workflow", id: 3, ...asOwner }); + expect(c.publishStatus).toBeDefined(); + + c.setPublished(false); + + expect(c.isPublic).toBe(false); + expect(c.publishStatus).toBeUndefined(); + }); + + it("hides the notice when the status request fails", () => { + workflowPersistSpy.getWorkflowIsPublished.mockReturnValue(of("Public")); + workflowPersistSpy.getPublishStatus.mockReturnValue(throwError(() => new Error("boom"))); + const c = setupComponent({ type: "workflow", id: 3, ...asOwner }); + expect(c.publishStatus).toBeUndefined(); + }); + + it("surfaces a failed pin and stops the spinner", () => { + workflowPersistSpy.getWorkflowIsPublished.mockReturnValue(of("Public")); + workflowPersistSpy.getPublishStatus.mockReturnValue( + of({ isPublished: true, isPinned: true, hasUnpublishedChanges: true }) + ); + workflowPersistSpy.pinLatestVersion.mockReturnValue( + throwError(() => new HttpErrorResponse({ error: { message: "nope" }, status: 500 })) + ); + const c = setupComponent({ type: "workflow", id: 3, ...asOwner }); + + c.choosePublicCopy(true, true); + + expect(notificationSpy.error).toHaveBeenCalledWith("nope"); + expect(c.isPinning).toBe(false); + // The pending state stands, so the author can try again. + expect(c.publishStatus?.hasUnpublishedChanges).toBe(true); + }); + + it("never asks for publish status on a dataset", () => { + const c = setupComponent({ type: "dataset", id: 12, ...asOwner }); + expect(workflowPersistSpy.getPublishStatus).not.toHaveBeenCalled(); + expect(c.publishStatus).toBeUndefined(); + }); + + it("fetches the status after publishing from private", () => { + workflowPublished = false; + const c = setupComponent({ type: "workflow", id: 3, ...asOwner }); + expect(workflowPersistSpy.getPublishStatus).not.toHaveBeenCalled(); + + c.setPublished(true); + + expect(workflowPersistSpy.updateWorkflowIsPublished).toHaveBeenCalledWith(3, true); + expect(workflowPersistSpy.getPublishStatus).toHaveBeenCalledWith(3); + expect(c.publishStatus).toBeDefined(); + }); + + it("offers both choices, with the state itself naming the one in force", () => { + workflowPersistSpy.getWorkflowIsPublished.mockReturnValue(of("Public")); + workflowPersistSpy.getPublishStatus.mockReturnValue( + of({ isPublished: true, isPinned: true, hasUnpublishedChanges: false }) + ); + const c = setupComponent({ type: "workflow", id: 3, ...asOwner }); + + expect(segments()).toEqual(["Follow latest", "Pinned"]); + expect(c.publishState).toBe("pinned"); + }); + + it("does nothing when the side already in force is picked again", () => { + // Re-picking "Pinned" would otherwise publish whatever has been edited since -- which is the + // card's job, and the card says what it would publish. + workflowPersistSpy.getWorkflowIsPublished.mockReturnValue(of("Public")); + workflowPersistSpy.getPublishStatus.mockReturnValue( + of({ isPublished: true, isPinned: true, hasUnpublishedChanges: true }) + ); + const c = setupComponent({ type: "workflow", id: 3, ...asOwner }); + + c.choosePublicCopy(true); + + expect(workflowPersistSpy.pinLatestVersion).not.toHaveBeenCalled(); + expect(c.publishStatus?.hasUnpublishedChanges).toBe(true); + }); + + it("does nothing when following is picked while already following", () => { + workflowPersistSpy.getWorkflowIsPublished.mockReturnValue(of("Public")); + workflowPersistSpy.getPublishStatus.mockReturnValue( + of({ isPublished: true, isPinned: false, hasUnpublishedChanges: false }) + ); + const c = setupComponent({ type: "workflow", id: 3, ...asOwner }); + + c.choosePublicCopy(false); + + expect(workflowPersistSpy.unpinVersion).not.toHaveBeenCalled(); + }); + + it("names the pinned version and the way out while it is holding edits back", () => { + // The card is the answer to "my edits are not public": which version is out there, what it + // costs, and the act that ends it. In every other state there is nothing to decide. + workflowPersistSpy.getWorkflowIsPublished.mockReturnValue(of("Public")); + workflowPersistSpy.getPublishStatus.mockReturnValue( + of({ + isPublished: true, + isPinned: true, + hasUnpublishedChanges: true, + pinnedVersionTime: Date.parse("2026-08-12T23:59:00"), + }) + ); + setupComponent({ type: "workflow", id: 3, ...asOwner }); + + expect(publishNoteText()).toContain("Pinned to Aug 12, 23:59:00"); + expect(publishNoteText()).toContain("Your later edits stay private"); + expect(publishNoteText()).toContain("Update to current"); + }); + + it("points at the version panel once the pinned copy is what the author has", () => { + // The recovery path when they do move on, named where they are already looking. + workflowPersistSpy.getWorkflowIsPublished.mockReturnValue(of("Public")); + workflowPersistSpy.getPublishStatus.mockReturnValue( + of({ isPublished: true, isPinned: true, hasUnpublishedChanges: false }) + ); + setupComponent({ type: "workflow", id: 3, ...asOwner }); + + expect(publishLineText()).toContain("Currently public"); + }); + + it("shows nothing and asks nothing while the feature is off", () => { + // The series ships dark: until the last PR turns the flag on, the dialog is exactly what it + // is today, and it does not even ask the server about a state it cannot show. + workflowPersistSpy.getWorkflowIsPublished.mockReturnValue(of("Public")); + workflowPersistSpy.getPublishStatus.mockReturnValue( + of({ isPublished: true, isPinned: true, hasUnpublishedChanges: true }) + ); + + setupComponent({ type: "workflow", id: 3, versionPinningEnabled: false, ...asOwner }); + + expect(publishLineText()).toBe(""); + expect(workflowPersistSpy.getPublishStatus).not.toHaveBeenCalled(); + }); + + it("writes nothing when it opens", () => { + // The dialog reports on the saved copy rather than forcing the canvas to become it: the canvas + // is empty while a workflow loads, and a save nobody asked for could write that emptiness over + // every operator the workflow had. + workflowPersistSpy.getWorkflowIsPublished.mockReturnValue(of("Public")); + workflowPersistSpy.getPublishStatus.mockReturnValue( + of({ isPublished: true, isPinned: true, hasUnpublishedChanges: false }) + ); + setupComponent({ type: "workflow", id: 3, inWorkspace: true, ...asOwner }); + + expect(workflowPersistSpy.persistWorkflow).not.toHaveBeenCalled(); + }); + + it("re-reads the state when a save lands", () => { + // The editor saves on a debounce, so the answer the dialog fetched when it opened describes a + // workflow the author may already have moved past. Nothing else tells it -- the canvas changing + // is not the save landing, and until the save lands the server still answers with the old copy. + workflowPersistSpy.getWorkflowIsPublished.mockReturnValue(of("Public")); + workflowPersistSpy.getPublishStatus.mockReturnValue( + of({ isPublished: true, isPinned: true, hasUnpublishedChanges: false }) + ); + setupComponent({ type: "workflow", id: 3, ...asOwner }); + expect(publishNoteText()).toBe(""); + + workflowPersistSpy.getPublishStatus.mockReturnValue( + of({ isPublished: true, isPinned: true, hasUnpublishedChanges: true }) + ); + persisted.next({ wid: 3 }); + + expect(workflowPersistSpy.getPublishStatus).toHaveBeenCalledTimes(2); + expect(publishNoteText()).toContain("Update to current"); + }); + + it("says nothing extra when the pinned copy is what the author has", () => { + workflowPersistSpy.getWorkflowIsPublished.mockReturnValue(of("Public")); + workflowPersistSpy.getPublishStatus.mockReturnValue( + of({ isPublished: true, isPinned: true, hasUnpublishedChanges: false }) + ); + setupComponent({ type: "workflow", id: 3, ...asOwner }); + + expect(publishNoteText()).toBe(""); + }); + + it("publishes the edits a pin was holding back, from the note", () => { + workflowPersistSpy.getWorkflowIsPublished.mockReturnValue(of("Public")); + workflowPersistSpy.getPublishStatus.mockReturnValue( + of({ isPublished: true, isPinned: true, hasUnpublishedChanges: true }) + ); + workflowPersistSpy.pinLatestVersion.mockReturnValue( + of({ isPublished: true, isPinned: true, hasUnpublishedChanges: false }) + ); + const c = setupComponent({ type: "workflow", id: 3, ...asOwner }); + + fixture.nativeElement.querySelector(".publish-card-action").click(); + + expect(workflowPersistSpy.pinLatestVersion).toHaveBeenCalledWith(3); + expect(c.publishStatus?.hasUnpublishedChanges).toBe(false); + // Answered, so the question goes away. + expect(publishNoteText()).toBe(""); + }); + + it("says nothing about the copies before the status arrives", () => { + workflowPersistSpy.getWorkflowIsPublished.mockReturnValue(of("Private")); + const c = setupComponent({ type: "workflow", id: 3, ...asOwner }); + expect(c.publishStatus).toBeUndefined(); + expect(publishLineText()).toBe(""); + }); + }); }); diff --git a/frontend/src/app/dashboard/component/user/share-access/share-access.component.ts b/frontend/src/app/dashboard/component/user/share-access/share-access.component.ts index b5baf5d9dba..fe631351fed 100644 --- a/frontend/src/app/dashboard/component/user/share-access/share-access.component.ts +++ b/frontend/src/app/dashboard/component/user/share-access/share-access.component.ts @@ -27,13 +27,18 @@ import { GmailService } from "../../../../common/service/gmail/gmail.service"; import { NZ_MODAL_DATA, NzModalRef, NzModalService } from "ng-zorro-antd/modal"; import { NotificationService } from "../../../../common/service/notification/notification.service"; import { HttpErrorResponse } from "@angular/common/http"; -import { catchError, of, switchMap } from "rxjs"; +import { catchError, forkJoin, of, switchMap } from "rxjs"; import { NzMessageService } from "ng-zorro-antd/message"; import { WorkflowActionService } from "src/app/workspace/service/workflow-graph/model/workflow-action.service"; +import { + WorkflowPersistService, + WorkflowPublishStatus, +} from "src/app/common/service/workflow-persist/workflow-persist.service"; +import { GuiConfigService } from "../../../../common/service/gui-config.service"; import { ResourceRegistryService } from "../../../service/user/resource-registry/resource-registry.service"; import { ResourceDescriptor } from "../../../type/resource-descriptor"; import { EntityType } from "../../../../hub/service/hub.service"; -import { NgIf, NgFor } from "@angular/common"; +import { NgIf, NgFor, NgSwitch, NgSwitchCase, NgSwitchDefault, DatePipe } from "@angular/common"; import { NzSpaceCompactItemDirective } from "ng-zorro-antd/space"; import { NzButtonComponent } from "ng-zorro-antd/button"; import { NzWaveDirective } from "ng-zorro-antd/core/wave"; @@ -46,6 +51,8 @@ import { NzInputDirective } from "ng-zorro-antd/input"; import { NzAutocompleteTriggerDirective, NzAutocompleteComponent } from "ng-zorro-antd/auto-complete"; import { NzTagComponent } from "ng-zorro-antd/tag"; import { NzTooltipDirective } from "ng-zorro-antd/tooltip"; +import { NzSegmentedComponent } from "ng-zorro-antd/segmented"; +import { NzBadgeComponent } from "ng-zorro-antd/badge"; @UntilDestroy() @Component({ @@ -73,6 +80,12 @@ import { NzTooltipDirective } from "ng-zorro-antd/tooltip"; NgFor, NzTagComponent, NzTooltipDirective, + NzSegmentedComponent, + NzBadgeComponent, + NgSwitch, + NgSwitchCase, + NgSwitchDefault, + DatePipe, ], }) export class ShareAccessComponent implements OnInit, OnDestroy { @@ -91,6 +104,16 @@ export class ShareAccessComponent implements OnInit, OnDestroy { isPublic: boolean | null = null; /** Undefined for kinds the registry does not carry, i.e. computing units. */ private readonly descriptor: ResourceDescriptor | undefined; + /** Undefined until fetched, and for anyone who cannot publish (the endpoint needs write access). */ + publishStatus?: WorkflowPublishStatus; + isPinning = false; + /** The two sides of the switch; the control only takes strings or numbers, so 1 means pinned. */ + readonly publicCopyOptions = [ + { label: "Follow latest", value: 0 }, + { label: "Pinned", value: 1 }, + ]; + /** To the second, always: the revision panel names the same version that way. */ + readonly publicationTimeFormat = "MMM d, HH:mm:ss"; private shouldRefresh = false; @Output() refresh = new EventEmitter(); @@ -103,7 +126,9 @@ export class ShareAccessComponent implements OnInit, OnDestroy { private message: NzMessageService, private modalService: NzModalService, private workflowActionService: WorkflowActionService, + private workflowPersistService: WorkflowPersistService, private resourceRegistry: ResourceRegistryService, + protected config: GuiConfigService, private modalRef: NzModalRef ) { this.validateForm = this.formBuilder.group({ @@ -114,6 +139,61 @@ export class ShareAccessComponent implements OnInit, OnDestroy { this.descriptor = this.resourceRegistry.find(this.type as EntityType); } + /** Only for a published workflow the user can publish -- the endpoint requires write access. */ + private refreshPublishStatus(): void { + if (!this.config.env.versionPinningEnabled) { + return; + } + if (this.type !== "workflow" || !this.isPublic || !this.hasWriteAccess) { + this.publishStatus = undefined; + return; + } + this.workflowPersistService + .getPublishStatus(this.id) + .pipe(untilDestroyed(this)) + .subscribe({ + next: status => (this.publishStatus = status), + error: () => (this.publishStatus = undefined), + }); + } + + /** Named as a state, so the switch, the sentence and the dot cannot disagree about which is in force. */ + get publishState(): "follow" | "pinned" | "behind" { + if (!this.publishStatus?.isPinned) { + return "follow"; + } + return this.publishStatus.hasUnpublishedChanges ? "behind" : "pinned"; + } + + /** + * Changes what the public sees, leaving the author's working copy alone. Picking the side already + * in force does nothing, so a stray click cannot silently publish edits made since the pin; + * `republish` is how the card asks for exactly that. + */ + public choosePublicCopy(pinned: boolean, republish = false): void { + if (!this.publishStatus || (pinned === this.publishStatus.isPinned && !republish)) { + return; + } + this.isPinning = true; + const change = pinned + ? { request: this.workflowPersistService.pinLatestVersion(this.id), done: "The public now sees this version" } + : { request: this.workflowPersistService.unpinVersion(this.id), done: "The public now follows your latest" }; + change.request + .pipe(untilDestroyed(this)) + .subscribe({ + next: status => { + this.publishStatus = status; + this.notificationService.success(change.done); + }, + error: (error: unknown) => { + if (error instanceof HttpErrorResponse) { + this.notificationService.error(error.error.message); + } + }, + }) + .add(() => (this.isPinning = false)); + } + get hasWriteAccess(): boolean { if (!this.currentEmail) { return false; @@ -126,35 +206,54 @@ export class ShareAccessComponent implements OnInit, OnDestroy { } ngOnInit(): void { - this.accessService - .getAccessList(this.type, this.id) - .pipe(untilDestroyed(this)) - .subscribe(access => (this.accessList = access)); - this.accessService - .getOwner(this.type, this.id) - .pipe(untilDestroyed(this)) - .subscribe(name => { - this.owner = name; - }); - // Stays null for kinds that cannot be published, which is what hides the publish buttons. - // A failed request leaves it null too, and the buttons are equally gone, so say so: without - // this the dialog looked complete while quietly offering one control fewer, and the only way - // to find out was the network tab. - this.descriptor - ?.isPublic?.(this.id) + // Joined rather than subscribed separately because the publish state depends on all three: + // hasWriteAccess is only answerable once the owner and the access list have landed, and the + // panel only applies to a published workflow. + // Stays undefined for kinds that cannot be published, which is what hides the publish buttons. + forkJoin([ + this.accessService.getAccessList(this.type, this.id), + this.accessService.getOwner(this.type, this.id), + this.descriptor?.isPublic?.(this.id) ?? of(undefined), + ]) .pipe(untilDestroyed(this)) .subscribe({ - next: isPublic => (this.isPublic = isPublic), + next: ([accessList, owner, isPublic]) => { + this.accessList = accessList; + this.owner = owner; + if (isPublic !== undefined) { + this.isPublic = isPublic; + } + if (this.type !== "workflow") { + return; + } + // Answered from the saved copy, which the editor writes a few seconds after the last edit. + // Deliberately not forced to save first: the canvas is not always the workflow -- it is + // empty while one loads, and stays empty if the collaborative model never arrives -- so a + // save nobody asked for could write that emptiness over every operator the workflow had. + // The subscription below picks the answer up again as soon as the autosave lands. + this.refreshPublishStatus(); + }, error: () => { - // `ngOnInit` is re-entered as a refresh after an access change, so a value from the - // previous read may still be here. Drop it: keeping it would leave the buttons on - // screen, showing a state nothing has confirmed, while the toast says they are gone. + // A failed read leaves the publish controls hidden, so say so: without this the dialog + // looked complete while quietly offering one control fewer, and the only way to find out + // was the network tab. `ngOnInit` is re-entered as a refresh after an access change, so a + // value from the previous read may still be here -- drop it rather than leave a state + // nothing has confirmed on screen while the toast says it is gone. this.isPublic = null; + this.publishStatus = undefined; this.notificationService.error( `Could not read whether this ${this.type} is public, so that choice is not shown.` ); }, }); + if (this.type === "workflow") { + // The panel describes the saved copy and the editor saves on a debounce, so re-read when a + // save lands -- until then the server would still answer with the copy before it. + this.workflowPersistService + .getWorkflowPersistedStream() + .pipe(untilDestroyed(this)) + .subscribe(() => this.refreshPublishStatus()); + } } ngOnDestroy(): void { @@ -350,7 +449,11 @@ export class ShareAccessComponent implements OnInit, OnDestroy { const cloneWarning = this.descriptor?.affordances?.clonable ? ", along with the right to clone your work" : ""; const modal: NzModalRef = this.modalService.create({ nzTitle: "Notice", - nzContent: `Publishing your ${this.type} would grant all Texera users read access to your ${this.type}${cloneWarning}.`, + nzContent: + `Publishing your ${this.type} would grant all Texera users read access to your ${this.type}${cloneWarning}.` + + (this.type === "workflow" + ? " The public will follow your latest version as you save it, unless you pin one." + : ""), nzFooter: [ { label: "Cancel", @@ -414,6 +517,7 @@ export class ShareAccessComponent implements OnInit, OnDestroy { if (this.inWorkspace) { this.workflowActionService.setWorkflowIsPublished(published ? 1 : 0); } + this.refreshPublishStatus(); this.notificationService.success(`${label} ${published ? "published" : "unpublished"} successfully`); }, error: (error: unknown) => { diff --git a/frontend/src/app/dashboard/service/user/resource-registry/resource-registry.service.ts b/frontend/src/app/dashboard/service/user/resource-registry/resource-registry.service.ts index a65ce89cf34..ba111fc433a 100644 --- a/frontend/src/app/dashboard/service/user/resource-registry/resource-registry.service.ts +++ b/frontend/src/app/dashboard/service/user/resource-registry/resource-registry.service.ts @@ -98,8 +98,12 @@ export class ResourceRegistryService { /** * Where an entry's card links to: the owner-facing page when the viewer can reach it, the hub page * otherwise. An entry with no route, or one not yet persisted, links nowhere. + * + * Access alone is not enough outside the viewer's own listings: a hub entry shows the public copy, + * so an author whose working copy has moved on behind a pin is sent to that copy -- what the entry + * advertised, and where they can clone it -- rather than into their editor showing something else. */ - public entryLink(entry: DashboardEntry, currentUid: number | undefined): string[] { + public entryLink(entry: DashboardEntry, currentUid: number | undefined, isPrivateSearch = false): string[] { const descriptor = this.get(entry.type); if (descriptor.privateRoute === undefined || typeof entry.id !== "number") { return []; @@ -108,6 +112,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 085d61b1ae2..6d37cc16e02 100644 --- a/frontend/src/app/workspace/component/menu/menu.component.spec.ts +++ b/frontend/src/app/workspace/component/menu/menu.component.spec.ts @@ -1137,6 +1137,19 @@ describe("MenuComponent", () => { expect(createSpy).not.toHaveBeenCalled(); }); + + it("tells the versions panel to re-read once the dialog closes", async () => { + // The dialog is where a version gets pinned, and the panel behind it marks the pinned one. + // A real id, because the dialog does not open without one. + component.workflowId = 7; + vi.spyOn(workflowPersistService, "retrieveOwners").mockReturnValue(of([])); + vi.spyOn(modalService, "create").mockReturnValue({ afterClose: of(undefined) } as unknown as NzModalRef); + const announce = vi.spyOn(component.workflowVersionService, "notifyPublicVersionChanged"); + + await component.onClickOpenShareAccess(); + + expect(announce).toHaveBeenCalled(); + }); }); it("onClickCreateNewWorkflow resets the graph and navigates back to root", () => { diff --git a/frontend/src/app/workspace/component/menu/menu.component.ts b/frontend/src/app/workspace/component/menu/menu.component.ts index f04afd8bfd6..850bb6e8f18 100644 --- a/frontend/src/app/workspace/component/menu/menu.component.ts +++ b/frontend/src/app/workspace/component/menu/menu.component.ts @@ -385,7 +385,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(); }); }