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 searching in the list of drafts #5797

Merged
merged 1 commit into from
Oct 25, 2023

Conversation

grzesiek2010
Copy link
Member

@grzesiek2010 grzesiek2010 commented Oct 25, 2023

Closes #5796

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

That method was already updated but only for the case with no search text see https://github.com/getodk/collect/compare/v2023.3.x...grzesiek2010:COLLECT-5796?expand=1#diff-37147e150adcca4396232ebf94609b5ef3b79226becb8b5686c740d78ab8bda5R49 so I did the same for searching.

I haven't added any tests because that method does not have any... generally the whole class does not have tests and is marked as deprecated so I'm torn when it comes to adding tests now or waiting for refactoring and then doing that. @seadowg if you think we definitely need to cover this change with tests I will do that.

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?

Testing searching in the list of drafts should be enough. Nothing else should be affected.

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 checkAll and confirmed all checks still pass OR confirm CircleCI build passes and run ./gradlew connectedDebugAndroidTest locally.
  • added a comment above any new strings describing it for translators
  • 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 added the high priority Should be looked at before other PRs/issues label Oct 25, 2023
@seadowg seadowg linked an issue Oct 25, 2023 that may be closed by this pull request
@seadowg
Copy link
Member

seadowg commented Oct 25, 2023

@seadowg if you think we definitely need to cover this change with tests I will do that.

I'm ok with the idea of backfilling when we rework this screen.

@srujner
Copy link

srujner commented Oct 25, 2023

Tested with Success!

Verified on device with Android 13

Verified cases:

@dbemke
Copy link

dbemke commented Oct 25, 2023

Tested with Success!

Verified on device with Android 10

@grzesiek2010 grzesiek2010 merged commit 52eb023 into getodk:v2023.3.x Oct 25, 2023
6 checks passed
seadowg pushed a commit to seadowg/collect that referenced this pull request Oct 26, 2023
seadowg pushed a commit to seadowg/collect that referenced this pull request Oct 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
behavior verified high priority Should be looked at before other PRs/issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The search does not work in Drafts tab
4 participants