Skip to content

Add a missing test for conditional entity creation/update#7101

Open
grzesiek2010 wants to merge 2 commits intogetodk:masterfrom
grzesiek2010:create_or_update_entities_test
Open

Add a missing test for conditional entity creation/update#7101
grzesiek2010 wants to merge 2 commits intogetodk:masterfrom
grzesiek2010:create_or_update_entities_test

Conversation

@grzesiek2010
Copy link
Copy Markdown
Member

@grzesiek2010 grzesiek2010 commented Feb 17, 2026

Why is this the best possible solution? Were any other approaches considered?

A month ago, we had a meeting about testing entities from repeats, and we agreed that there should be an automated test for conditional create/update. I had this in mind and kept putting it off, but now it’s ready. Sorry for the delay.

How does this change affect users? Describe intentional changes to behavior and behavior that could have accidentally been affected by code changes. In other words, what are the regression risks?

It doesn't require testing.

Do we need any specific form for testing your changes? If so, please attach one.

No.

Does this change require updates to documentation? If so, please file an issue here and include the link below.

No.

Before submitting this PR, please make sure you have:

  • added or modified tests for any new or changed behavior
  • run ./gradlew connectedAndroidTest (or ./gradlew testLab) and confirmed all checks still pass
  • added a comment above any new strings describing it for translators
  • added any new strings with date formatting to DateFormatsTest
  • verified that any code or assets from external sources are properly credited in comments and/or in the about file.
  • verified that any new UI elements use theme colors. UI Components Style guidelines

@grzesiek2010 grzesiek2010 requested a review from seadowg February 17, 2026 11:58
bind("/data/people/name").type("string").withAttribute("entities", "saveto", "name"),
bind("/data/people/meta/entity/@create").type("string").calculate("/data/people/new = 'yes'"),
bind("/data/people/meta/entity/@update").type("string").calculate("/data/people/new != 'yes'"),
bind("/data/people/meta/entity/@id").type("string"),
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think to really capture the essence of a create or update form, you need to have a calculation that populates the id in the update case. See the entities sheet in this example.

This relates to the forum conversation about setting the Entity ID in that case. Currently pyxform would output both the setvalue you already have and a calculate. I think we should match that for now.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should match that for now.

You mean to keep both like now: a86447e?

@grzesiek2010 grzesiek2010 force-pushed the create_or_update_entities_test branch from db30b5f to a86447e Compare February 18, 2026 02:37
}

@Test
fun `filling form with create or update in repeats makes entities available`() {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd be tempted to test this at the Collect UI level instead given that we have to set up the full "stack" of entity plugins and that's not really ever done in EntitiesTest. Generally anything that interacts with the "data" layer (EntitiesRepository) is tested at the full app level to make sure everything is glued together. For example, this test uses an EntitiesRespoistory for its initial state, but doesn't check the final state (because that requires more components to be hooked in). Writing this as a full app test would get you a test of the full flow.

Alternatively, we could add some more realistic full flow tests in entities that sets the whole thing up to test flows like this without needing an app level test.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants