mengw15 commented on code in PR #8125:
URL: https://github.com/apache/texera/pull/8125#discussion_r3890118720
##########
amber/src/main/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowResource.scala:
##########
@@ -439,6 +442,14 @@ class WorkflowResource extends LazyLogging {
@Path("/persist")
def persistWorkflow(workflow: Workflow, @Auth sessionUser: SessionUser):
Workflow = {
val user = sessionUser.getUser
+
+ // `is_form_view` is owned by /enable-form-view and /disable-form-view
alone; a plain save
+ // sends the whole POJO to workflowDao.update, so without this its default
clears the flag.
+ if (workflow.getWid != null) {
Review Comment:
Not blocking: this read-modify-write still has a small window — an
`enable-form-view` landing between the fetch and `workflowDao.update` gets
overwritten by the save. Since `setFormView` already updates by explicit
column, persist could do the same (update only
name/description/content/is_public), which would remove both the extra fetch
and the window.
##########
sql/changelog.xml:
##########
@@ -124,6 +124,11 @@
<sqlFile path="sql/updates/42.sql"/>
</changeSet>
+ <!-- Form View: per-workflow on/off flag -->
+ <changeSet id="43" author="yangzhang75">
Review Comment:
Heads-up: #7851 (pin series) is still open and also claims `43.sql` —
whichever of the two lands second will need to renumber the file and this
changeSet id.
##########
amber/src/test/scala/org/apache/texera/web/resource/dashboard/UnifiedResourceSchemaSpec.scala:
##########
@@ -197,12 +199,12 @@ 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
+ // 25 slots, but only six structurally distinct default expressions, so the
// de-dup collapses the map to six 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.
Review Comment:
Nit: this line still says 24.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
