Improve Create Section: add open case condition and dedicated fields by jingcheng16 · Pull Request #1225 · dimagi/Vellum

jingcheng16 · 2026-04-16T14:16:01Z

Product Description

The old Create section required users to manually enter case_name, case_type, and owner_id as generic key-value properties, each with a Property Name, Calculation, and an optional Relevant (update condition) field. We’ve already reconciled case_type with a top-level dropdown. This PR is to:

make case_name and owner_id dedicated single fields that only require dragging and dropping a question reference - removing the need to type property names manually.
when the action is create, user no longer needs to select update action as well to put other case properties, user can add other case properties in "Case Properties To Create"

Before

After

Backward compatibility

Legacy forms that used per-property relevant on case_type, case_name, or owner_id create binds are automatically migrated:

case_type and case_name relevants are promoted to Open Case Condition (combined with and if they differ).
owner_id relevant is absorbed if it matches the promoted condition, otherwise kept as Owner ID Condition.
Legacy forms with both <create> and <update> sections merge update properties into the Create section.

See the design decision document for the full rationale on how relevant fields are handled.

Technical Summary

Ticket: https://dimagi.atlassian.net/browse/SAAS-19457

Why `aec64e3` (XPath validation) is needed

The xPath widget's built-in logic reference tracking (via the logic manager) validates expressions during interactive editing — but it does NOT validate on form load. When a legacy form with an invalid XPath in a promoted relevant is loaded, the logic manager silently fails to track references without producing an error. The validationFunc in aec64e3 catches this by running form.xpath.parse() during mug.validate(), which fires when the mug is displayed. The message text matches the logic manager's message ("Invalid XPath expression." with period) so getMessages deduplicates them during interactive editing.

Feature Flag

VELLUM_SAVE_TO_CASE

Safety Assurance

Safety story

Tested locally multiple times. I also test on production forms that has per-property relevant to make sure them will load as expected so user's workflow won't be disrupted.

Automated test coverage

New test suites:

"dedicated create fields"
"create section backward compatibility"

QA Plan

Load existing production forms with per-property relevant → verify Open Case Condition is populated, no per-property relevant in output
Create new form with Open Case Condition → verify case-level bind in XML
Verify bubbles display correctly for promoted conditions
Verify invalid XPath shows exactly one warning message

Rollback instructions

This PR can be reverted after deploy with no further considerations

Labels & Review

Risk label is set correctly
The set of people pinged as reviewers is appropriate for the level of risk of the change

Add four new dedicated xPath properties to the Create section: - openCaseCondition - caseName - ownerId - ownerIdCondition Each field has standard parse (parseBindElement) and generate (getBindList, dataChildFilter, getCaseSaveData) support.

Per-property relevant on case_type/case_name causes runtime errors on mobile. Promote it to the case-level openCaseCondition both at parse time and generation time so it can never appear as a per-property attribute in the output. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Now that case_name and owner_id have dedicated fields, repurpose createProperty to accept arbitrary property names while rejecting reserved ones (case_type, case_name, owner_id). Extra create properties emit under <update> in the XML since CommCare's <create> block only supports case_type, case_name, and owner_id. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

When a parsed form has both <create> and <update> sections, merge the update properties into createProperty and clear useUpdate so they appear in the Create section. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Add validateXPath helper that uses form.xpath.parse() to catch invalid XPath expressions. Applied to openCaseCondition and ownerIdCondition via validationFunc. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

mergeUpdatePropertiesIntoCreateAfterParse

millerdev

I ran out of time and quickly reviewed the final tests commit, so should spend a bit more time on that, but otherwise looks good.

millerdev · 2026-04-16T20:57:54Z

+        ownerIdRelevant = mug.p._ownerIdRelevant;
+    delete mug.p._caseTypeRelevant;
+    delete mug.p._caseNameRelevant;
+    delete mug.p._ownerIdRelevant;


I don't see this delete mug.p.property pattern elsewhere in Vellum. Have you confirmed that it works? Might be better to use mug.p.set('property'), which deletes 'property' from the mug's properties.

ahhh, sorry, this is confusing. _caseTypeRelevant, _caseNameRelevant, _ownerIdRelevant technically are not spec properties on mug, they're just plain JS property on MugProperties object. mug.p.set('attr') won't work because it deletes from __data. But I can see it is confusing because the pattern is mug.p.attr... I've now moved them to mug._stashedCreateBindRelevants to avoid confusion. 60a076b

millerdev

Responses to all comments can be implemented in a follow-up PR if you agree with them but would prefer merge this PR ASAP.

millerdev · 2026-04-17T12:23:17Z

+            mug.p.createProperty = {
+                case_name: { calculate: "/data/name" },
+            };
+            assert.notEqual(mug.spec.createProperty.validationFunc(mug), "pass");


How hard would it be to check for the specific expected error here rather than "not pass"?

Example:

Vellum/tests/form.js

Line 99 in 5c0c6ed

assert(mug.getErrors().join("").indexOf("untouched property error") !== -1,

There is at least one other assertion in this file that might also benefit from that technique.

With my changes in #1227, now each field carry its own validation, the property level will never return the reserved-name error, it will only return an errorSummary which is an generic string: “One or more properties above have errors. Fix the highlighted fields.” . And I find checking against the generic string is no different than checking pass.

millerdev · 2026-04-17T12:24:45Z

+            assert.deepEqual(_.without(_.keys(mug.p.createProperty), ""), ["p1", "p2"]);
+        });
+
+        it("should preserve user-provided ownerIdCondition even when openCaseCondition has same condition", function () {


Given this behavior, why not always move owner_id relevant to ownerIdCondition rather than merging the two as outlined in the spec?

If case_name + owner_id or case_type + owner_id share the same relevant → move to Open Case Condition. XML changes.

Seems like that could simplify the implementation and maybe not be quite as surprising for users who (for example) accidentally set the both case_name + owner_id relevants to the same expression when they intended to only set owner_id relevant?

Yes it will simplify the implementation. But I believe the intention for user to leave same relevant for both case_name and owner_id or case_type and owner_id is to open the case conditionally and I want the new UI will accurately reflect that.

maybe not be quite as surprising for users who (for example) accidentally set the both case_name + owner_id relevants to the same expression when they intended to only set owner_id relevant?

This is possible. I wonder how well our user remember the value they typed in all mugs...For users who intentionally set both relevant the same expression and intend to make them just open case condition, if we always move owner_id relevant to ownerIdCondition will we surprise user by open case condition and owner id condition be the same?

millerdev · 2026-04-17T12:25:23Z

+    });
+
+    describe("openCaseCondition promotion", function () {
+        it("should preserve user-provided ownerIdCondition even when openCaseCondition has same condition", function () {


How is this test different from the one above on line 635?

it("should preserve user-provided ownerIdCondition even when openCaseCondition has same condition", ...

nice catch. I'll address in a follow up PR.

jingcheng16 and others added 8 commits April 15, 2026 19:16

Refactor: add a helper to add create property node

472b3da

Refactor: Extract promoteStashedCreateBindRelevants and ...

e49a244

mergeUpdatePropertiesIntoCreateAfterParse

Add tests for create section improvements

5c0c6ed

jingcheng16 marked this pull request as ready for review April 16, 2026 14:18

jingcheng16 requested a review from millerdev April 16, 2026 14:18

jingcheng16 added the product/feature-flag label Apr 16, 2026

jingcheng16 mentioned this pull request Apr 16, 2026

[EPIC]GA Save to Case: Base Changes and Coordination #1206

Draft

14 tasks

millerdev reviewed Apr 16, 2026

View reviewed changes

jingcheng16 added 2 commits April 16, 2026 21:40

Move the stashed relevant to mug itself

60a076b

lint

27fdf37

millerdev approved these changes Apr 17, 2026

View reviewed changes

jingcheng16 merged commit a194484 into jc/save-to-case-base Apr 17, 2026
3 checks passed

jingcheng16 deleted the jc/open-case-condition-v2 branch April 17, 2026 13:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve Create Section: add open case condition and dedicated fields#1225

Improve Create Section: add open case condition and dedicated fields#1225
jingcheng16 merged 10 commits intojc/save-to-case-basefrom
jc/open-case-condition-v2

jingcheng16 commented Apr 16, 2026 •

edited

Loading

Uh oh!

millerdev left a comment

Uh oh!

millerdev Apr 16, 2026 •

edited

Loading

Uh oh!

jingcheng16 Apr 17, 2026

Uh oh!

millerdev left a comment

Uh oh!

millerdev Apr 17, 2026

Uh oh!

jingcheng16 Apr 23, 2026

Uh oh!

millerdev Apr 17, 2026

Uh oh!

jingcheng16 Apr 23, 2026

Uh oh!

millerdev Apr 17, 2026

Uh oh!

jingcheng16 Apr 23, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

jingcheng16 commented Apr 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Product Description

Backward compatibility

Technical Summary

Why aec64e3 (XPath validation) is needed

Feature Flag

Safety Assurance

Safety story

Automated test coverage

QA Plan

Rollback instructions

Labels & Review

Uh oh!

millerdev left a comment

Choose a reason for hiding this comment

Uh oh!

millerdev Apr 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jingcheng16 Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

millerdev left a comment

Choose a reason for hiding this comment

Uh oh!

millerdev Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

jingcheng16 Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

millerdev Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

jingcheng16 Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

millerdev Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

jingcheng16 Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

jingcheng16 commented Apr 16, 2026 •

edited

Loading

Why `aec64e3` (XPath validation) is needed

millerdev Apr 16, 2026 •

edited

Loading