This is an automated email from the ASF dual-hosted git repository. github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-8125-11e3b969a70063d17bfd670b1da85dba100e4339 in repository https://gitbox.apache.org/repos/asf/texera.git
commit b28faee36ff27bca7d47acb13de27c99828f6981 Author: yangzhang75 <[email protected]> AuthorDate: Tue Sep 1 00:43:31 2026 +0000 feat(workflow): persist is_form_view and add Form View toggle endpoints (#8125) ### What changes were proposed in this PR? Let the backend remember, per workflow, which view it opens in by default (canvas or form), and expose setting it. Backend + DB only; no UI or form logic. The Form View is available for every workflow, so this column is a default-entry preference, not a capability gate. - **Schema**: new column `workflow.default_view VARCHAR NOT NULL DEFAULT 'CANVAS'` (values `CANVAS`, `FORM`) in `sql/updates/44.sql`, `changelog.xml`, `texera_ddl.sql`. - **Write**: `PUT /set-default-view/{wid}` with a JSON body `{"view": "CANVAS" | "FORM"}`, guarded by write access and validated (any other value is a 400). It updates only that column, never rewriting content or bumping the modified time. A plain save preserves the stored value; a duplicate and a hub-clone both inherit it. - **Read**: `UnifiedResourceSchema` and `WorkflowSearchQueryBuilder` include the value in listing/search; `WorkflowVersionResource.cloneVersion` carries the source workflow's current value onto the clone; `retrieveWorkflow` / `retrievePublicWorkflow` report it. Naming and migration notes: the value is a `default_view` enum (canvas or form) rather than an on/off flag, and the Form View's actual definition still lives in `content.formBinding`; only this preference is denormalized into a column. The migration is `44.sql` because `43.sql` is reserved for a sibling PR and `42.sql` was the latest on main. ### Any related issues, documentation, discussions? Closes #8014. Part of the Form View parent issue #8011; builds on the feature flag from #8013. ### How was this PR tested? `WorkflowResourceSpec` covers the set-default-view endpoint: the form/canvas switch, rejection without write access, rejection of an invalid view value, survival across a plain save, inheritance by both `duplicateWorkflow` and `cloneWorkflow`, reporting by both retrieve paths, and `formBinding` preserved when switching back to canvas. Together with the version-clone, listing/search and unified-schema specs, the four affected specs run 127 green. Compiled and run against an ephemeral Postgres so jOOQ regenerated the `default_view` column. ### Was this PR authored or co-authored using generative AI tooling? Co-authored with Claude Code. Co-authored-by: Claude Opus 4.8 <[email protected]> --- .../resource/dashboard/UnifiedResourceSchema.scala | 10 +- .../dashboard/WorkflowSearchQueryBuilder.scala | 14 +- .../web/resource/dashboard/hub/HubResource.scala | 3 +- .../dashboard/user/workflow/WorkflowResource.scala | 96 +++++++-- .../user/workflow/WorkflowVersionResource.scala | 4 +- .../dashboard/UnifiedResourceSchemaSpec.scala | 31 +-- .../dashboard/WorkflowSearchQueryBuilderSpec.scala | 25 ++- .../dashboard/file/WorkflowResourceSpec.scala | 220 ++++++++++++++++++++- .../workflow/WorkflowVersionResourceSpec.scala | 35 +++- sql/changelog.xml | 5 + sql/texera_ddl.sql | 7 +- sql/updates/44.sql | 39 ++++ 12 files changed, 445 insertions(+), 44 deletions(-) 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 cfe3f381a4..7ffa2982d0 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 @@ -20,7 +20,7 @@ package org.apache.texera.web.resource.dashboard import org.apache.texera.dao.SqlServer -import org.apache.texera.dao.jooq.generated.enums.PrivilegeEnum +import org.apache.texera.dao.jooq.generated.enums.{DefaultViewEnum, PrivilegeEnum} import org.apache.texera.web.resource.dashboard.UnifiedResourceSchema.context import org.jooq.impl.DSL import org.jooq.{Field, Record} @@ -79,7 +79,10 @@ object UnifiedResourceSchema { DSL.cast(null, classOf[java.lang.Boolean]), versionedResourceUserAccess: Field[PrivilegeEnum] = DSL.castNull(classOf[PrivilegeEnum]), versionedResourceCoverImage: Field[String] = DSL.cast(null, classOf[String]), - workflowCoverImage: Field[String] = DSL.cast(null, classOf[String]) + workflowCoverImage: Field[String] = DSL.cast(null, classOf[String]), + // Workflow-only: which view the workflow opens in by default, so the listing can + // mark the row and route it accordingly. + workflowDefaultView: Field[DefaultViewEnum] = DSL.cast(null, classOf[DefaultViewEnum]) ): UnifiedResourceSchema = { new UnifiedResourceSchema( Seq( @@ -110,7 +113,8 @@ object UnifiedResourceSchema { .as("user_versioned_resource_access"), versionedResourceCoverImage -> versionedResourceCoverImage .as("versioned_resource_cover_image"), - workflowCoverImage -> workflowCoverImage.as("workflow_cover_image") + workflowCoverImage -> workflowCoverImage.as("workflow_cover_image"), + workflowDefaultView -> workflowDefaultView.as("workflow_default_view") ) ) } 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 b44ccaf30c..d668e73f12 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 @@ -29,7 +29,7 @@ import org.jooq.impl.DSL.groupConcatDistinct import org.jooq.{Condition, GroupField, Record, TableLike} import scala.jdk.CollectionConverters.CollectionHasAsScala -import org.apache.texera.dao.jooq.generated.enums.PrivilegeEnum +import org.apache.texera.dao.jooq.generated.enums.{DefaultViewEnum, PrivilegeEnum} object WorkflowSearchQueryBuilder extends SearchQueryBuilder { @@ -54,7 +54,8 @@ object WorkflowSearchQueryBuilder extends SearchQueryBuilder { ownerId = WORKFLOW_OF_USER.UID, userName = USER.NAME, projectsOfWorkflow = groupConcatDistinct(WORKFLOW_OF_PROJECT.PID), - workflowCoverImage = DSL.max(WORKFLOW_COVER_IMAGE.IMAGE).as("workflow_cover_image") + workflowCoverImage = DSL.max(WORKFLOW_COVER_IMAGE.IMAGE).as("workflow_cover_image"), + workflowDefaultView = WORKFLOW.DEFAULT_VIEW.as("workflow_default_view") ) } @@ -156,8 +157,13 @@ object WorkflowSearchQueryBuilder extends SearchQueryBuilder { Option(record.get(WORKFLOW_USER_ACCESS.PRIVILEGE, classOf[PrivilegeEnum])) .map(_.toString) .getOrElse(PrivilegeEnum.NONE.toString), - record.into(USER).getName, - record.into(WORKFLOW).into(classOf[Workflow]), + 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 + }, if (record.get(pidField) == null) { List[Integer]() } else { 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 0dd3cfc1b6..424eb9af85 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 @@ -267,7 +267,8 @@ object HubResource { WORKFLOW.LAST_MODIFIED_TIME, WORKFLOW_USER_ACCESS.PRIVILEGE, WORKFLOW_OF_USER.UID, - USER.NAME + USER.NAME, + WORKFLOW.DEFAULT_VIEW ) .fetch() 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 315418516d..d2c7d62731 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 @@ -28,7 +28,7 @@ import org.apache.texera.amber.core.virtualidentity.ExecutionIdentity import org.apache.texera.auth.SessionUser import org.apache.texera.dao.SqlServer import org.apache.texera.dao.jooq.generated.Tables._ -import org.apache.texera.dao.jooq.generated.enums.PrivilegeEnum +import org.apache.texera.dao.jooq.generated.enums.{DefaultViewEnum, PrivilegeEnum} import org.apache.texera.dao.jooq.generated.tables.daos.{ WorkflowDao, WorkflowOfProjectDao, @@ -43,7 +43,7 @@ import org.apache.texera.web.resource.dashboard.hub.HubResource.recordCloneActio import org.apache.texera.web.resource.dashboard.user.workflow.WorkflowAccessResource.hasReadAccess import org.apache.texera.web.resource.dashboard.user.workflow.WorkflowResource._ import org.jooq.impl.DSL.{groupConcatDistinct, noCondition, max} -import org.jooq.{Condition, DSLContext, Record10, Result, SelectOnConditionStep} +import org.jooq.{Condition, DSLContext, Record11, Result, SelectOnConditionStep} import java.sql.Timestamp import java.util @@ -84,6 +84,9 @@ object WorkflowResource { /** JSON body/response for a workflow's cover image data URL. */ case class CoverImageRequest(image: String) + /** JSON body for setting which view a workflow opens in by default (CANVAS or FORM). */ + case class DefaultViewRequest(view: String) + def getWorkflowName(wid: Integer): String = { val workflow = workflowDao.fetchOneByWid(wid) if (workflow == null) { @@ -138,7 +141,9 @@ object WorkflowResource { creationTime: Timestamp, lastModifiedTime: Timestamp, isPublished: Boolean, - readonly: Boolean + readonly: Boolean, + // Which view this workflow opens in by default (CANVAS or FORM); both load through this endpoint. + defaultView: DefaultViewEnum ) case class WorkflowIDs(wids: List[Integer], pid: Option[Integer]) @@ -193,7 +198,7 @@ object WorkflowResource { } } - def baseWorkflowSelect(): SelectOnConditionStep[Record10[ + def baseWorkflowSelect(): SelectOnConditionStep[Record11[ Integer, String, String, @@ -203,7 +208,8 @@ object WorkflowResource { Integer, String, String, - String + String, + DefaultViewEnum ]] = { context .select( @@ -216,7 +222,8 @@ object WorkflowResource { WORKFLOW_OF_USER.UID, USER.NAME, groupConcatDistinct(WORKFLOW_OF_PROJECT.PID).as("projects"), - max(WORKFLOW_COVER_IMAGE.IMAGE).as("cover_image") + max(WORKFLOW_COVER_IMAGE.IMAGE).as("cover_image"), + WORKFLOW.DEFAULT_VIEW ) .from(WORKFLOW) .leftJoin(WORKFLOW_USER_ACCESS) @@ -232,7 +239,7 @@ object WorkflowResource { } def mapWorkflowEntries( - workflowEntries: Result[Record10[ + workflowEntries: Result[Record11[ Integer, String, String, @@ -242,7 +249,8 @@ object WorkflowResource { Integer, String, String, - String + String, + DefaultViewEnum ]], uid: Integer ): List[DashboardWorkflow] = { @@ -386,7 +394,8 @@ class WorkflowResource extends LazyLogging { WORKFLOW.LAST_MODIFIED_TIME, WORKFLOW_USER_ACCESS.PRIVILEGE, WORKFLOW_OF_USER.UID, - USER.NAME + USER.NAME, + WORKFLOW.DEFAULT_VIEW ) .fetch() mapWorkflowEntries(workflowEntries, user.getUid) @@ -417,7 +426,8 @@ class WorkflowResource extends LazyLogging { workflow.getCreationTime, workflow.getLastModifiedTime, workflow.getIsPublic, - !WorkflowAccessResource.hasWriteAccess(wid, user.getUid) + !WorkflowAccessResource.hasWriteAccess(wid, user.getUid), + workflow.getDefaultView ) } else { throw new ForbiddenException("No sufficient access privilege.") @@ -439,9 +449,10 @@ class WorkflowResource extends LazyLogging { @Path("/persist") def persistWorkflow(workflow: Workflow, @Auth sessionUser: SessionUser): Workflow = { val user = sessionUser.getUser + if (workflowOfUserExists(workflow.getWid, user.getUid)) { WorkflowVersionResource.insertVersion(workflow, insertingNewWorkflow = false) - workflowDao.update(workflow) + saveWorkflowFields(workflow) } else { if (!WorkflowAccessResource.hasReadAccess(workflow.getWid, user.getUid)) { // Check if this workflow exists in the database @@ -458,7 +469,7 @@ class WorkflowResource extends LazyLogging { } else if (WorkflowAccessResource.hasWriteAccess(workflow.getWid, user.getUid)) { WorkflowVersionResource.insertVersion(workflow, insertingNewWorkflow = false) // not owner but has write access - workflowDao.update(workflow) + saveWorkflowFields(workflow) } else { // not owner and no write access -> rejected throw new ForbiddenException("No sufficient access privilege.") @@ -469,6 +480,23 @@ class WorkflowResource extends LazyLogging { workflowDao.fetchOneByWid(wid) } + /** + * Persists a plain save by updating only the fields the client sends + * (name/description/content/is_public). It deliberately leaves `default_view` untouched -- + * that column is owned by /set-default-view alone -- so a save can never clobber a + * concurrent change. Timestamps are likewise not rewritten here. + */ + private def saveWorkflowFields(workflow: Workflow): Unit = { + context + .update(WORKFLOW) + .set(WORKFLOW.NAME, workflow.getName) + .set(WORKFLOW.DESCRIPTION, workflow.getDescription) + .set(WORKFLOW.CONTENT, workflow.getContent) + .set(WORKFLOW.IS_PUBLIC, workflow.getIsPublic) + .where(WORKFLOW.WID.eq(workflow.getWid)) + .execute() + } + /** * This method duplicates the target workflow, the new workflow name is appended with `_copy` * @@ -507,7 +535,9 @@ class WorkflowResource extends LazyLogging { assignNewOperatorIds(oldWorkflow.getContent), null, null, - false + false, + // the default view is part of the workflow, so a copy keeps it + oldWorkflow.getDefaultView ), sessionUser ) @@ -557,7 +587,9 @@ class WorkflowResource extends LazyLogging { assignNewOperatorIds(oldWorkflow.getContent), null, null, - false + false, + // a biologist's path is hub -> clone -> use, so the clone must stay usable + oldWorkflow.getDefaultView ), sessionUser ) @@ -726,6 +758,39 @@ class WorkflowResource extends LazyLogging { workflowDao.update(workflow) } + /** + * Set which view a workflow opens in by default (CANVAS or FORM). Only this preference lives + * in a column; the form's definition travels in workflow.content under `formBinding`, so + * switching the default never touches it. + */ + @PUT + @Consumes(Array(MediaType.APPLICATION_JSON)) + @RolesAllowed(Array("REGULAR", "ADMIN")) + @Path("/set-default-view/{wid}") + def setDefaultView( + @PathParam("wid") wid: Integer, + request: DefaultViewRequest, + @Auth user: SessionUser + ): Unit = { + if (!WorkflowAccessResource.hasWriteAccess(wid, user.getUid)) { + throw new ForbiddenException(s"You do not have permission to modify workflow $wid") + } + // lookupLiteral returns null for an unknown literal and for a null/missing body value, + // so both an out-of-range string and an empty request map to a 400 rather than a 500. + val view = DefaultViewEnum.lookupLiteral(request.view) + if (view == null) { + throw new BadRequestException(s"default_view must be CANVAS or FORM, got: ${request.view}") + } + // Update only this column. The preference is deliberately independent of content, so a change + // must not rewrite the whole row -- doing so would touch content (and could clobber a + // concurrent save) and bump the last-modified time for a mere preference change. + context + .update(WORKFLOW) + .set(WORKFLOW.DEFAULT_VIEW, view) + .where(WORKFLOW.WID.eq(wid)) + .execute() + } + /** Returns the workflow's cover image; 404 if none set. */ @GET @RolesAllowed(Array("REGULAR", "ADMIN")) @@ -848,7 +913,8 @@ class WorkflowResource extends LazyLogging { workflow.getCreationTime, workflow.getLastModifiedTime, workflow.getIsPublic, - readonly = true + readonly = true, + defaultView = workflow.getDefaultView ) } 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 e0664b7c1d..bae03efbd7 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 @@ -435,7 +435,9 @@ class WorkflowVersionResource { assignNewOperatorIds(workflowVersion.getContent), null, null, - false + false, + // carry the workflow's current default-view preference onto the clone + workflowVersion.getDefaultView ), sessionUser ) 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 52890b4bf0..9334fc5b0d 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,7 +79,7 @@ class UnifiedResourceSchemaSpec extends AnyFlatSpec with Matchers { // Sentinels for the three slots that have no convenient distinct table // column of the right type; every other slot uses a real generated column so - // that all 24 originals render differently from one another. + // that all 25 originals render differently from one another. private val sentinelResourceType: Field[String] = JDSL.inline("s-resource-type") private val sentinelProjects: Field[String] = JDSL.inline("s-projects") private val sentinelStoragePath: Field[String] = JDSL.inline("s-storage-path") @@ -108,7 +108,8 @@ class UnifiedResourceSchemaSpec extends AnyFlatSpec with Matchers { isVersionedResourceDownloadable = DATASET.IS_DOWNLOADABLE, versionedResourceUserAccess = DATASET_USER_ACCESS.PRIVILEGE, versionedResourceCoverImage = DATASET.COVER_IMAGE, - workflowCoverImage = WORKFLOW_COVER_IMAGE.IMAGE + workflowCoverImage = WORKFLOW_COVER_IMAGE.IMAGE, + workflowDefaultView = WORKFLOW.DEFAULT_VIEW ) // Expected projection, in order: alias -> the original it must be built from. @@ -136,13 +137,14 @@ class UnifiedResourceSchemaSpec extends AnyFlatSpec with Matchers { "is_versioned_resource_downloadable" -> DATASET.IS_DOWNLOADABLE, "user_versioned_resource_access" -> DATASET_USER_ACCESS.PRIVILEGE, "versioned_resource_cover_image" -> DATASET.COVER_IMAGE, - "workflow_cover_image" -> WORKFLOW_COVER_IMAGE.IMAGE + "workflow_cover_image" -> WORKFLOW_COVER_IMAGE.IMAGE, + "workflow_default_view" -> WORKFLOW.DEFAULT_VIEW ) // -- apply(): the projection ------------------------------------------------ - "apply" should "expose all 24 slots as aliases, in the order the UNION ALL depends on" in { - sentinelSchema.allFields should have size 24 + "apply" should "expose all 25 slots as aliases, in the order the UNION ALL depends on" in { + sentinelSchema.allFields should have size 25 sentinelSchema.allFields.map(_.getName) shouldBe expectedProjection.map(_._1) } @@ -162,7 +164,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 24 + defaults.allFields should have size 25 val rendered = ctx.renderInlined(JDSL.select(defaults.allFields: _*)) rendered should include("'' as \"resourceType\"") rendered should include("cast(null as timestamp) as \"resourceCreationTime\"") @@ -197,23 +199,24 @@ class UnifiedResourceSchemaSpec extends AnyFlatSpec with Matchers { } it should "collapse the all-defaults projection down to one alias per distinct default" in { - // 24 slots, but only six structurally distinct default expressions, so the - // de-dup collapses the map to six entries. Worth pinning because it is + // 25 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 24 while the translation map does not. + // all: allFields stays at 25 while the translation map does not. val defaults = UnifiedResourceSchema() - defaults.allFields should have size 24 + defaults.allFields should have size 25 translatedAliases(defaults) shouldBe Seq( "resourceType", // DSL.inline("") "resourceCreationTime", // cast(null as timestamp) "resourceOwnerId", // cast(null as int) "workflow_privilege", // cast(null as privilege_enum) "dataset_storage_path", // cast(null as varchar) - "is_versioned_resource_public" // cast(null as boolean) + "is_versioned_resource_public", // cast(null as boolean) + "workflow_default_view" // cast(null as default_view_enum) ) } - it should "keep every distinct original when the caller supplies 24 distinct Fields" in { + it should "keep every distinct original when the caller supplies 25 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) @@ -221,7 +224,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 24 + workflowSchema.allFields should have size 25 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. @@ -241,7 +244,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 24 slots and translateRecord would start reading duplicated columns — + // all 25 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 59dd8d9e19..53c2884067 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 @@ -20,7 +20,7 @@ package org.apache.texera.web.resource.dashboard import org.apache.texera.dao.jooq.generated.Tables._ -import org.apache.texera.dao.jooq.generated.enums.PrivilegeEnum +import org.apache.texera.dao.jooq.generated.enums.{DefaultViewEnum, PrivilegeEnum} import org.apache.texera.web.resource.dashboard.user.workflow.WorkflowResource.DashboardWorkflow import org.jooq.impl.{DSL => JDSL} import org.jooq.{Record, SQLDialect} @@ -68,6 +68,9 @@ class WorkflowSearchQueryBuilderSpec extends AnyFlatSpec with Matchers { // QueryParts structurally. The first test pins that assumption. private val pidField = JDSL.groupConcatDistinct(WORKFLOW_OF_PROJECT.PID) private val coverField = JDSL.max(WORKFLOW_COVER_IMAGE.IMAGE).as("workflow_cover_image") + // 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 ownerUid: Integer = Integer.valueOf(42) private val viewerUid: Integer = Integer.valueOf(43) @@ -85,7 +88,8 @@ class WorkflowSearchQueryBuilderSpec extends AnyFlatSpec with Matchers { uidValue: Integer = ownerUid, privilege: PrivilegeEnum = PrivilegeEnum.WRITE, projects: String = "3,1,2", - cover: String = "cover-b64" + cover: String = "cover-b64", + defaultView: DefaultViewEnum = DefaultViewEnum.CANVAS ): Record = { val record = ctx.newRecord( WORKFLOW.WID, @@ -95,7 +99,8 @@ class WorkflowSearchQueryBuilderSpec extends AnyFlatSpec with Matchers { WORKFLOW_USER_ACCESS.PRIVILEGE, USER.NAME, pidField, - coverField + coverField, + defaultViewField ) record.set(WORKFLOW.WID, wid) record.set(WORKFLOW.NAME, "wf-name") @@ -105,6 +110,7 @@ class WorkflowSearchQueryBuilderSpec extends AnyFlatSpec with Matchers { record.set(USER.NAME, "owner-name") record.set(pidField, projects) record.set(coverField, cover) + record.set(defaultViewField, defaultView) record } @@ -201,6 +207,19 @@ class WorkflowSearchQueryBuilderSpec extends AnyFlatSpec with Matchers { workflowOf(translatedRecord(cover = null), ownerUid).coverImage shouldBe None } + it should "carry the default view off its own aliased column" in { + // The listing's select projects default_view separately (the WORKFLOW POJO the + // record maps into does not carry it), so toEntryImpl must read it back by alias. + workflowOf( + translatedRecord(defaultView = DefaultViewEnum.FORM), + ownerUid + ).workflow.getDefaultView shouldBe DefaultViewEnum.FORM + workflowOf( + translatedRecord(defaultView = DefaultViewEnum.CANVAS), + ownerUid + ).workflow.getDefaultView shouldBe DefaultViewEnum.CANVAS + } + it should "tag the entry as a workflow and leave the other payload slots empty" in { val entry = WorkflowSearchQueryBuilder.toEntryImpl(ownerUid, translatedRecord()) entry.resourceType shouldBe "workflow" diff --git a/amber/src/test/scala/org/apache/texera/web/resource/dashboard/file/WorkflowResourceSpec.scala b/amber/src/test/scala/org/apache/texera/web/resource/dashboard/file/WorkflowResourceSpec.scala index c2852fc654..1fd9c406f3 100644 --- a/amber/src/test/scala/org/apache/texera/web/resource/dashboard/file/WorkflowResourceSpec.scala +++ b/amber/src/test/scala/org/apache/texera/web/resource/dashboard/file/WorkflowResourceSpec.scala @@ -29,7 +29,7 @@ import org.apache.texera.dao.jooq.generated.Tables.{ WORKFLOW_USER_CLONES, WORKFLOW_VERSION } -import org.apache.texera.dao.jooq.generated.enums.{PrivilegeEnum, UserRoleEnum} +import org.apache.texera.dao.jooq.generated.enums.{DefaultViewEnum, PrivilegeEnum, UserRoleEnum} import org.apache.texera.dao.jooq.generated.tables.daos.{UserDao, WorkflowUserAccessDao} import org.apache.texera.dao.jooq.generated.tables.pojos.{ Project, @@ -43,6 +43,7 @@ import org.apache.texera.web.resource.dashboard.user.project.ProjectResource import org.apache.texera.web.resource.dashboard.user.workflow.WorkflowResource import org.apache.texera.web.resource.dashboard.user.workflow.WorkflowResource.{ DashboardWorkflow, + DefaultViewRequest, WorkflowIDs, WorkflowWithPrivilege } @@ -1300,4 +1301,221 @@ class WorkflowResourceSpec assert(workflowNamesOf(sessionUser1).isEmpty) } + // --------------------------------------------------------------------------- + // Form View: the per-workflow default view (canvas or form). + // --------------------------------------------------------------------------- + + // duplicateWorkflow runs assignNewOperatorIds over the content, which requires a + // real `operators` array, so the toy content used elsewhere in this spec won't do. + private val contentWithOperators = + """{"operators":[{"operatorID":"Limit-operator-1","operatorType":"Limit"}],""" + + """"operatorPositions":{},"links":[],"commentBoxes":[],"settings":{}}""" + + /** Persist a fresh workflow owned by user 1 and return its wid. */ + private def persistFreshWorkflow( + name: String, + content: String = contentWithOperators + ): Integer = { + val workflow = new Workflow() + workflow.setName(name) + workflow.setContent(content) + workflowResource.persistWorkflow(workflow, sessionUser1) + workflow.getWid + } + + private def defaultView(wid: Integer): DefaultViewEnum = + getDSLContext + .select(WORKFLOW.DEFAULT_VIEW) + .from(WORKFLOW) + .where(WORKFLOW.WID.eq(wid)) + .fetchOne() + .value1() + + private def contentOf(wid: Integer): String = + getDSLContext + .select(WORKFLOW.CONTENT) + .from(WORKFLOW) + .where(WORKFLOW.WID.eq(wid)) + .fetchOne() + .value1() + + private def lastModifiedOf(wid: Integer): Timestamp = + getDSLContext + .select(WORKFLOW.LAST_MODIFIED_TIME) + .from(WORKFLOW) + .where(WORKFLOW.WID.eq(wid)) + .fetchOne() + .value1() + + "/set-default-view API" should "switch the default view to form and back to canvas" in { + val wid = persistFreshWorkflow("param_toggle") + assert(defaultView(wid) == DefaultViewEnum.CANVAS, "a new workflow must default to the canvas") + + workflowResource.setDefaultView(wid, DefaultViewRequest("FORM"), sessionUser1) + assert(defaultView(wid) == DefaultViewEnum.FORM) + + workflowResource.setDefaultView(wid, DefaultViewRequest("CANVAS"), sessionUser1) + assert(defaultView(wid) == DefaultViewEnum.CANVAS) + } + + it should "reject a user without write access" in { + val wid = persistFreshWorkflow("param_no_access") + + assertThrows[ForbiddenException] { + workflowResource.setDefaultView(wid, DefaultViewRequest("FORM"), sessionUser2) + } + assert(defaultView(wid) == DefaultViewEnum.CANVAS) + } + + it should "reject an invalid or missing view value" in { + val wid = persistFreshWorkflow("param_invalid") + + assertThrows[BadRequestException] { + workflowResource.setDefaultView(wid, DefaultViewRequest("SIDEBAR"), sessionUser1) + } + // A missing/null body value must be a 400, not a 500 (lookupLiteral returns null, not NPE). + assertThrows[BadRequestException] { + workflowResource.setDefaultView(wid, DefaultViewRequest(null), sessionUser1) + } + assert(defaultView(wid) == DefaultViewEnum.CANVAS) + } + + // A plain save (persistWorkflow) only writes the fields the client sends -- name, + // description, content, is_public -- and never `default_view`, so saving the canvas must + // not reset the default view. The edit payload mirrors what the frontend sends. + it should "survive a subsequent save of the workflow" in { + val wid = persistFreshWorkflow("param_survives_save") + workflowResource.setDefaultView(wid, DefaultViewRequest("FORM"), sessionUser1) + + val edit = new Workflow() + edit.setWid(wid) + edit.setName("param_survives_save_edited") + edit.setContent("{\"operators\":[],\"links\":[]}") + edit.setIsPublic(false) + workflowResource.persistWorkflow(edit, sessionUser1) + + assert( + defaultView(wid) == DefaultViewEnum.FORM, + "saving the canvas must not reset the default view" + ) + } + + // A biologist's path is hub -> clone -> use, so a copy has to stay usable. + it should "be inherited by a duplicated workflow" in { + val wid = persistFreshWorkflow("param_source") + workflowResource.setDefaultView(wid, DefaultViewRequest("FORM"), sessionUser1) + + val copies = workflowResource.duplicateWorkflow(WorkflowIDs(List(wid), None), sessionUser1) + + assert(copies.length == 1) + assert( + defaultView(copies.head.workflow.getWid) == DefaultViewEnum.FORM, + "the copy must keep the preference" + ) + } + + // The hub's clone button goes through cloneWorkflow (not duplicateWorkflow); a cloned + // form-default workflow must stay form-default so the copy opens straight into its form. + it should "be inherited by a workflow cloned through cloneWorkflow" in { + val wid = + seedWorkflow(sessionUser1, "clone-formview-src", "d", contentWithOperator).workflow.getWid + workflowResource.makePublic(wid, sessionUser1) + workflowResource.setDefaultView(wid, DefaultViewRequest("FORM"), sessionUser1) + + val newWid = workflowResource.cloneWorkflow(wid, sessionUser2, cloneRequest) + + assert(defaultView(newWid) == DefaultViewEnum.FORM, "the clone must keep the default view") + } + + // Both views load a workflow through this endpoint, and the client needs the default view + // to know which one to open first. Leaving the value out of the payload left the client + // guessing, so it is worth pinning down. + it should "be reported by the endpoint both views load through" in { + val wid = persistFreshWorkflow("param_retrieve") + assert( + workflowResource.retrieveWorkflow(wid, sessionUser1).defaultView == DefaultViewEnum.CANVAS + ) + + workflowResource.setDefaultView(wid, DefaultViewRequest("FORM"), sessionUser1) + + assert(workflowResource.retrieveWorkflow(wid, sessionUser1).defaultView == DefaultViewEnum.FORM) + } + + it should "leave a duplicate of a plain workflow defaulting to the canvas" in { + val wid = persistFreshWorkflow("plain_source") + + val copies = workflowResource.duplicateWorkflow(WorkflowIDs(List(wid), None), sessionUser1) + + assert(copies.length == 1) + assert(defaultView(copies.head.workflow.getWid) == DefaultViewEnum.CANVAS) + } + + // Setting the preference updates only its own column, so a mere change must not bump the + // workflow's last-modified time (which would reorder the dashboard's "recent" listing). + it should "not change last_modified_time when the default view is set" in { + val wid = persistFreshWorkflow("param_mtime") + val before = lastModifiedOf(wid) + + workflowResource.setDefaultView(wid, DefaultViewRequest("FORM"), sessionUser1) + assert( + lastModifiedOf(wid) == before, + "setting the default view must not bump last_modified_time" + ) + + workflowResource.setDefaultView(wid, DefaultViewRequest("CANVAS"), sessionUser1) + assert(lastModifiedOf(wid) == before, "setting it back must not bump last_modified_time") + } + + // The dashboard listing (GET /workflow/list) selects specific columns, so it has to include + // default_view explicitly or every listed workflow would report the POJO default (null). + it should "be reported by the workflow listing endpoint" in { + val wid = persistFreshWorkflow("param_list") + workflowResource.setDefaultView(wid, DefaultViewRequest("FORM"), sessionUser1) + + val listed = + workflowResource.retrieveWorkflowsBySessionUser(sessionUser1).find(_.workflow.getWid == wid) + + assert(listed.isDefined) + assert( + listed.get.workflow.getDefaultView == DefaultViewEnum.FORM, + "the listing must carry the default view" + ) + } + + // The hub loads a public workflow through retrievePublicWorkflow, and a clone opens + // straight into the form only when that response says the source defaults to the form. + it should "be reported by retrievePublicWorkflow for a public workflow" in { + val workflow = new Workflow() + workflow.setName("param_public_retrieve") + workflow.setContent(contentWithOperators) + workflow.setIsPublic(true) + workflowResource.persistWorkflow(workflow, sessionUser1) + val wid = workflow.getWid + + assert(workflowResource.retrievePublicWorkflow(wid).defaultView == DefaultViewEnum.CANVAS) + + workflowResource.setDefaultView(wid, DefaultViewRequest("FORM"), sessionUser1) + + assert(workflowResource.retrievePublicWorkflow(wid).defaultView == DefaultViewEnum.FORM) + } + + // Switching the default back to canvas only changes the preference; the author's setup lives + // in content under `formBinding` and must survive so switching back to form restores it. + it should "keep the form definition in content when the default view is set back to canvas" in { + val withBinding = + """{"operators":[{"operatorID":"Limit-operator-1","operatorType":"Limit"}],""" + + """"operatorPositions":{},"links":[],"commentBoxes":[],"settings":{},""" + + """"formBinding":{"exposed":["Limit-operator-1"]}}""" + val wid = persistFreshWorkflow("param_keep_def", withBinding) + workflowResource.setDefaultView(wid, DefaultViewRequest("FORM"), sessionUser1) + + workflowResource.setDefaultView(wid, DefaultViewRequest("CANVAS"), sessionUser1) + + assert(defaultView(wid) == DefaultViewEnum.CANVAS) + assert( + contentOf(wid).contains("formBinding"), + "switching back to canvas must not erase the form definition" + ) + } + } 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 15a3bb6654..91b0b1c1be 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 @@ -23,7 +23,7 @@ import org.apache.texera.amber.util.JSONUtils.objectMapper import org.apache.texera.auth.SessionUser import org.apache.texera.dao.MockTexeraDB import org.apache.texera.dao.jooq.generated.Tables -import org.apache.texera.dao.jooq.generated.enums.PrivilegeEnum +import org.apache.texera.dao.jooq.generated.enums.{DefaultViewEnum, PrivilegeEnum} import org.apache.texera.dao.jooq.generated.tables.daos.{ UserDao, WorkflowDao, @@ -455,6 +455,39 @@ class WorkflowVersionResourceSpec cloned.getName should include("_copy") } + it should "inherit the source workflow's default view" in { + val workflowContent = + """{"operators":[{"operatorID":"CSVFileScan-operator-a","operatorType":"CSVFileScan"}],"links":[]}""" + testWorkflow.setContent(workflowContent) + testWorkflow.setDefaultView(DefaultViewEnum.FORM) + workflowDao.update(testWorkflow) + val version = WorkflowVersionResource.insertNewVersion(testWorkflowWid, "[]") + + val newWid = resource.cloneVersion( + version.getVid, + session(owner), + Map("displayedVersionId" -> 1).asJava + ) + + workflowDao.fetchOneByWid(newWid).getDefaultView shouldBe DefaultViewEnum.FORM + } + + it should "leave the clone defaulting to canvas when the source does" in { + val workflowContent = + """{"operators":[{"operatorID":"CSVFileScan-operator-a","operatorType":"CSVFileScan"}],"links":[]}""" + testWorkflow.setContent(workflowContent) + workflowDao.update(testWorkflow) + val version = WorkflowVersionResource.insertNewVersion(testWorkflowWid, "[]") + + val newWid = resource.cloneVersion( + version.getVid, + session(owner), + Map("displayedVersionId" -> 1).asJava + ) + + workflowDao.fetchOneByWid(newWid).getDefaultView shouldBe DefaultViewEnum.CANVAS + } + // ─── version-importance helpers (pure JSON/timestamp logic) ──────────────── private val isSnapshotImportant = PrivateMethod[Boolean](Symbol("isSnapshotImportant")) diff --git a/sql/changelog.xml b/sql/changelog.xml index b57ec95ca4..8427de8170 100644 --- a/sql/changelog.xml +++ b/sql/changelog.xml @@ -124,6 +124,11 @@ <sqlFile path="sql/updates/42.sql"/> </changeSet> + <!-- Form View: per-workflow default view (canvas or form) --> + <changeSet id="44" author="yangzhang75"> + <sqlFile path="sql/updates/44.sql"/> + </changeSet> + <!-- example changeSet <changeSet id="1" author="author"> <sqlFile path="sql/updates/1.sql"/> diff --git a/sql/texera_ddl.sql b/sql/texera_ddl.sql index acd4eef817..e4efa5277b 100644 --- a/sql/texera_ddl.sql +++ b/sql/texera_ddl.sql @@ -93,6 +93,7 @@ DROP TYPE IF EXISTS user_role_enum CASCADE; DROP TYPE IF EXISTS privilege_enum CASCADE; DROP TYPE IF EXISTS action_enum CASCADE; DROP TYPE IF EXISTS provider_type_enum CASCADE; +DROP TYPE IF EXISTS default_view_enum CASCADE; CREATE TYPE user_role_enum AS ENUM ('INACTIVE', 'RESTRICTED', 'REGULAR', 'ADMIN'); CREATE TYPE action_enum AS ENUM ('like', 'unlike', 'view', 'clone'); @@ -100,6 +101,7 @@ CREATE TYPE privilege_enum AS ENUM ('NONE', 'READ', 'WRITE'); CREATE TYPE workflow_computing_unit_type_enum AS ENUM ('local', 'kubernetes'); CREATE TYPE provider_type_enum AS ENUM ('LOCAL', 'GOOGLE'); CREATE TYPE user_warehouse_flavor_enum AS ENUM ('local', 'aws'); +CREATE TYPE default_view_enum AS ENUM ('CANVAS', 'FORM'); -- ============================================ -- 5. Create tables @@ -166,7 +168,10 @@ CREATE TABLE IF NOT EXISTS workflow content TEXT NOT NULL, creation_time TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP, last_modified_time TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP, - is_public BOOLEAN NOT NULL DEFAULT false + is_public BOOLEAN NOT NULL DEFAULT false, + -- Which view the workflow opens in by default (CANVAS or FORM); the form's definition + -- lives in workflow.content (`formBinding`). + default_view default_view_enum NOT NULL DEFAULT 'CANVAS' ); -- workflow_of_user diff --git a/sql/updates/44.sql b/sql/updates/44.sql new file mode 100644 index 0000000000..4be3264432 --- /dev/null +++ b/sql/updates/44.sql @@ -0,0 +1,39 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +\c texera_db + +SET search_path TO texera_db; + +BEGIN; + +-- Which view a workflow opens in by default (CANVAS or FORM). The form's definition lives +-- in workflow.content under `formBinding`; only this preference is denormalized so listing +-- endpoints need not parse every row's content. It never gates availability -- the form is +-- reachable for every workflow. +DO $$ BEGIN + CREATE TYPE default_view_enum AS ENUM ('CANVAS', 'FORM'); +EXCEPTION + WHEN duplicate_object THEN NULL; +END $$; + +ALTER TABLE workflow + ADD COLUMN IF NOT EXISTS default_view default_view_enum NOT NULL DEFAULT 'CANVAS'; + +COMMIT;
