Fix reading form metadata in forms containing multiple meta sections by grzesiek2010 · Pull Request #7141 · getodk/collect

grzesiek2010 · 2026-03-17T09:10:01Z

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

This ensures the correct for metadata is always read from the top-level <meta>, avoiding using other <meta> sections that only contain entity information.

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?

This should only fix the issue. I can't think of any bigger risk here.

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

The form from https://forum.getodk.org/t/entity-submission-draft-shown-with-form-title-rather-than-instance-name-in-collect/57858

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 · 2026-03-17T09:25:22Z

collect_app/src/main/java/org/odk/collect/android/javarosawrapper/JavaRosaFormController.java

-
-        TreeElement trueSubmissionElement;
-        // Determine the information about the submission...
-        SubmissionProfile p = formDef.getSubmissionProfile();


Was this needed here in any way? I can’t really imagine a use case, but maybe I’m missing something.

To make the change here very explicit, Collect previously allowed the "meta block" to be defined anywhere within the primary instance. This PR changes that so that it's only supported if it is a direct child of the primary instance. Is that your understanding @grzesiek2010?

As far as I can see, the spec is actually a little ambiguous on where the meta block should live. All the examples show it as a direct child, but that requirement isn't really stated explicitly. The closes we get to that is probably:

The primary instance also includes a special type of nodes for metadata inside the block. See the Metadata section

"contains" isn't really specific enough to make this change, but I suspect the intention in the spec is that the meta block should be a direct child. I'd like to get second opinions from @lindsay-stevens or @lognaturel.

This PR changes that so that it's only supported if it is a direct child of the primary instance. Is that your understanding @grzesiek2010?

Kind of, I mean yes, and that's why I got rid of findDepthFirst function, but here the question was more about using SubmissionProfile. Whether it was needed for something or not.

As far as I can see, the spec is actually a little ambiguous on where the meta block should live. All the examples show it as a direct child, but that requirement isn't really stated explicitly.

This PR is not the first where we assume the main meta block (with instanceId, instanceName, audit) is the direct child, see:
https://github.com/getodk/collect/blob/master/collect_app/src/main/java/org/odk/collect/android/formmanagement/finalization/EditedFormFinalizationProcessor.kt#L23

Good point! Ok so if there is a problem with that, it will already exist. I'd still like to hear from others (so we can at least make the spec clearer), but this is good to go.

Very interesting, I'd never noticed that ambiguity. I assume the original intent was indeed the direct meta child, that's certainly an important assumption for the whole way the multiple Entity declaration spec works. I think we should make the spec more specific.

I think pyxform is currently doing as expected but I've filed XLSForm/pyxform#831 to improve test coverage. The spec examples show the main meta element being a child of the primary data instance root element (usually data), but it wouldn't hurt to also say that in the text, and perhaps also that any other meta elements (expected for entities) must only have entities in them (though meta isn't currently a reserved name so that might be a stretch).

Great! I can get a PR up for the spec then.

dbemke · 2026-03-18T11:00:54Z

I reproduced the issue using the store version of Collect (2026.1.1) and forms from the forum issue. There's a difference in results when I use test.getodk.cloud and staging.getodk.cloud
(staging on the left; test on the right)

user "instancename"
https://test.getodk.cloud/projects/629/app-users
https://staging.getodk.cloud/projects/215/app-users
Should it be tested with the newest changes on Central (staging) or the test instance or both?

seadowg · 2026-03-18T11:06:25Z

@dbemke it shouldn't matter, but I'd definitely not expect the behaviour you're seeing on test. What are the reproduction steps to get in that state with test.getodk.cloud?

dbemke · 2026-03-18T11:07:57Z

What are the reproduction steps to get in that state with test.getodk.cloud?

Steps to reproduce:

Fill forms (form 1 and form2).
Save it as drafts.

There are links (above) to user "instancename" both on staging and test.

grzesiek2010 · 2026-03-18T11:16:27Z

test.getodk.cloud has xml with the format that allows reproducing the issue:

<instance>
                <data id="formtest2" version="20260314170716">
                    <today/>
                    <page1>
                        <lbl1/>
                        <lbl2/>
                        <meta>
                            <entity dataset="entity_test" create="1" id="">
                                <label/>
                            </entity>
                        </meta>
                    </page1>
                    <meta>
                        <instanceID/>
                        <instanceName/>
                    </meta>
                </data>
            </instance>

but staging.getodk.cloud doesn't:

            <instance>
                <data id="formtest2" version="20260314170716">
                    <today/>
                    <page1>
                        <lbl1/>
                        <lbl2/>
                    </page1>
                    <meta>
                        <entity dataset="entity_test" create="1" id="">
                            <label/>
                        </entity>
                        <instanceID/>
                        <instanceName/>
                    </meta>
                </data>
            </instance>

I don't know what changes these two servers contain, but it's ok to test this PR only with test.getodk.cloud.

seadowg · 2026-03-18T11:21:35Z

@grzesiek2010 @dbemke ah ok my guess is that it's changes to XLSForm that generate a different XForm from the example. @grzesiek2010 could you share the full XForm that should be used for testing just so there's no confusion?

@dbemke it'd be worth double-checking that this is indeed an expected change by downloading the XForm you get from the example .xlsx (from the forum) when uploading to each version of Central, and share them with the Central team on Slack.

grzesiek2010 · 2026-03-18T11:25:37Z

could you share the full XForm that should be used for testing just so there's no confusion?

I've downloaded the one from test.getodk.cloud:
formtest2.xml

WKobus · 2026-03-18T13:45:01Z

Tested with success

Verified on Android 16

Issue Saved forms with entities ignore dynamic instance names defined by the instance_name column in the settings sheet #7138 does not occur
Calculation for instance name inside and outside group
Comparing results between test and staging servers

dbemke · 2026-03-18T13:45:19Z

Tested with success

Verified on Android 10

Fix reading form metadata in forms containing multiple meta sections

grzesiek2010 added 5 commits March 17, 2026 10:05

Add a failing test

e2b0a6e

Use the top-level meta block to read metadata

3431f99

Remove redundant code

cb61789

Improve variable names

34d6ff3

Convert InstanceMetadata to kotlin

03b73ff

grzesiek2010 commented Mar 17, 2026

View reviewed changes

grzesiek2010 marked this pull request as ready for review March 17, 2026 09:42

grzesiek2010 requested a review from seadowg March 17, 2026 09:42

grzesiek2010 added the high priority Should be looked at before other PRs/issues label Mar 17, 2026

seadowg added the needs testing label Mar 17, 2026

seadowg approved these changes Mar 17, 2026

View reviewed changes

lindsay-stevens mentioned this pull request Mar 18, 2026

Add regression tests for the meta element location and usage patterns with settings XLSForm/pyxform#831

Open

seadowg mentioned this pull request Mar 18, 2026

Add clarification around meta block position getodk/xforms-spec#342

Merged

dbemke added behavior verified and removed needs testing labels Mar 18, 2026

seadowg merged commit d825348 into getodk:v2026.1.x Mar 18, 2026
8 checks passed

seadowg added a commit to seadowg/collect that referenced this pull request Mar 19, 2026

Merge pull request getodk#7141 from grzesiek2010/COLLECT-7138_1

e5e6389

Fix reading form metadata in forms containing multiple meta sections

seadowg mentioned this pull request Mar 19, 2026

Merge changes from v2026.1.2 #7146

Merged

Conversation

grzesiek2010 commented Mar 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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

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?

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

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

Before submitting this PR, please make sure you have:

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dbemke commented Mar 18, 2026

Uh oh!

seadowg commented Mar 18, 2026

Uh oh!

dbemke commented Mar 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

grzesiek2010 commented Mar 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

seadowg commented Mar 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

grzesiek2010 commented Mar 18, 2026

Uh oh!

WKobus commented Mar 18, 2026

Uh oh!

dbemke commented Mar 18, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

grzesiek2010 commented Mar 17, 2026 •

edited

Loading

dbemke commented Mar 18, 2026 •

edited

Loading

grzesiek2010 commented Mar 18, 2026 •

edited

Loading

seadowg commented Mar 18, 2026 •

edited

Loading