Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed using raw strings as requiredMsg #731

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Conversation

grzesiek2010
Copy link
Member

@grzesiek2010 grzesiek2010 commented Sep 27, 2023

Closes #730

What has been done to verify that this works as intended?

I've tested the changes manually with ODK Collect and also added automated tests.

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

Nothing to discuss here.

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?

We can focus on testing custom retired messages. The fix is rather safe and isolated so I can't think of any risk in any other area of the project. However we should start testing once we implement changes in Collect and start using the new method for reading required messages.

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

I used the one attached in the issue.

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

No.

@grzesiek2010 grzesiek2010 changed the title Javarosa 730 Fixed using raw strings as requiredMsg Sep 27, 2023
@grzesiek2010 grzesiek2010 marked this pull request as ready for review September 27, 2023 11:55
@@ -185,6 +188,41 @@ public String getAnswerText() {
}
}

public String getRequiredText() {
// look for the text under the requiredMsg bind attribute
String constraintText = form.getMainInstance().resolveReference(index.getReference()).getBindAttributeValue(XFormParser.NAMESPACE_JAVAROSA,
Copy link
Member

Choose a reason for hiding this comment

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

This should be requiredMsgText or something, right? It has nothing to do with constraints?

Copy link
Member Author

Choose a reason for hiding this comment

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

Right, fixed.

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.

Required message doesn't work if it's a raw string
2 participants