Skip to content
This repository was archived by the owner on Jun 20, 2023. It is now read-only.

Delete country list cache (EXPOSUREAPP-3154) #1431

Merged
merged 21 commits into from
Oct 28, 2020
Merged

Delete country list cache (EXPOSUREAPP-3154) #1431

merged 21 commits into from
Oct 28, 2020

Conversation

@SamuraiKek
Copy link
Contributor

@SamuraiKek SamuraiKek commented Oct 19, 2020 •
edited
Loading

Description

This PR fixes an issue where resetting the app from the in app settings didn't delete the cached country list, resulting in old data being displayed in the onboarding flow without internet access.

Steps to reproduce

  • Make sure you are connected to the internet.
  • From the main screen, go to Exposure Logging.
  • Enter the "Transnational Exposure Logging" screen.
  • Scroll down to the list of countries. At least one country should be displayed.
  • Go back to the main screen.
  • Disable your internet (mobile data + wifi).
  • On the upper right corner click the three dots and go to Settings.
  • Go to "Reset App".
  • Click "Reset App" and when the dialog shows up, click "Reset" once again.
  • On the upcoming onboarding flow, travel to the 3rd page, "How to Enable Exposure Logging".
  • Scroll down the page. No country list should be displayed. Instead, the "Participating Countries" message is displayed that tells you to view the countries in the exposure logging details.

@SamuraiKek SamuraiKek added bug Something isn't working maintainers Tag pull requests created by maintainers labels Oct 19, 2020
@SamuraiKek SamuraiKek added this to the 1.6.0 milestone Oct 19, 2020
@SamuraiKek SamuraiKek requested a review from a team October 19, 2020 14:02
@SamuraiKek SamuraiKek changed the title Fix/3154 delete country list cache Delete country list cache (EXPOSUREAPP-3154) Oct 19, 2020
d4rken
d4rken suggested changes Oct 19, 2020
View reviewed changes
@SamuraiKek SamuraiKek force-pushed the fix/3154-delete-country-list-cache branch from e0463d2 to 329cc51 Compare October 20, 2020 10:28
@vaubaehn
Copy link
Contributor

vaubaehn commented Oct 20, 2020

Would it make sense to have this one also in a potential hotfix 1.5.x? Especially as we still have the issue with outdated pending test results, and more users may find out to reset app data as a workaround to enter a new test, the scenario of this PR here becomes more likely.

@mlenkeit
Copy link
Member

mlenkeit commented Oct 24, 2020

@vaubaehn I think this is not urgent enough to include this in a hotfix for 1.5. Users might not even suspect this list to come from a server, so if they still see if after a reset, it should not confuse them. And even if they knew... CWA would try to download the list right during the onboarding anyway, so as long as there's internet, the list would show anyway. Hope this helps.

@vaubaehn
Copy link
Contributor

vaubaehn commented Oct 26, 2020

@mlenkeit Thanks for your feedback. Without knowing your workflow, I assumed that one could easily be implemented (so no need to wait for next minor version). But I agree, it's not one of the urgent fixes. Btw., looks like priorization of fixes is more focused now. Feels very good. Thanks to you all for your work!

d4rken
d4rken suggested changes Oct 27, 2020
View reviewed changes
Copy link
Member

@d4rken d4rken left a comment

Choose a reason for hiding this comment

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

Tested as far as the crash, need re-eval after that.

harambasicluka
harambasicluka previously approved these changes Oct 27, 2020
View reviewed changes
@SamuraiKek SamuraiKek requested a review from d4rken October 28, 2020 10:19
@sonarqubecloud
Copy link

sonarqubecloud bot commented Oct 28, 2020

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 0 Code Smells

3.4% 3.4% Coverage
0.0% 0.0% Duplication

@d4rken d4rken merged commit e90f5e3 into release/1.6.x Oct 28, 2020
@d4rken d4rken deleted the fix/3154-delete-country-list-cache branch October 28, 2020 15:08
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

2 more reviewers

@d4rken d4rken d4rken approved these changes

@harambasicluka harambasicluka harambasicluka approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

bug Something isn't working maintainers Tag pull requests created by maintainers

Projects

None yet

Milestone

1.6.0

Development

Successfully merging this pull request may close these issues.

7 participants

@SamuraiKek @vaubaehn @mlenkeit @d4rken @harambasicluka @AlexanderAlferov