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

Disable StrictMode for FormHiearchyActivity #6456

Merged
merged 4 commits into from
Oct 23, 2024

Conversation

seadowg
Copy link
Member

@seadowg seadowg commented Oct 15, 2024

Closes #6397

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

While working on this, I discovered that we'd accidentally disabled StrictMode protection for DatabaseEntitiesRepository. Fortunately, it doesn't seem like we'd added any violations as I was able to add it back in without problems (other than the expected ones).

I'd initially thought about removing the blanket protection on the whole repository and just adding custom slow calls to some methods, but I realised I'd have to remove protection on pretty much all the get... methods to allow for different kinds of filters in the hierarchy.

Instead, I've gone for an approach where StrictMode is disabled when the FormHierarchyActivity is created and then re-enabled when it is destroyed. This means we get full protection elsewhere, and only the "broken" code will have to change when we fix it.

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?

Shouldn't affect users at all as the crashes/errors were only visible in debug. No need for testing here.

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

@seadowg seadowg requested a review from grzesiek2010 October 15, 2024 09:44
@seadowg seadowg marked this pull request as ready for review October 15, 2024 09:44
protected void onDestroy() {
super.onDestroy();

CollectStrictMode.enable();
Copy link
Member

Choose a reason for hiding this comment

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

Is this a temporary solution? If so should we file an issue to fix it in the future?

Copy link
Member Author

Choose a reason for hiding this comment

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

I've added an issue: #6470

@grzesiek2010 grzesiek2010 merged commit a778f90 into getodk:master Oct 23, 2024
6 checks passed
@seadowg seadowg deleted the strict-mode branch October 23, 2024 16:01
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.

View hierarchy with filtered entity selects crashes debug builds
2 participants