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

Fix test result code instead of alias printed in test certificates detail screen (EXPOSUREAPP-7781) #3449

Merged
merged 10 commits into from
Jun 16, 2021

Conversation

jurajkusnier
Copy link
Contributor

  • Should work on WRU and other endpoints
  • May not work on local cli

@jurajkusnier jurajkusnier added bug Something isn't working maintainers Tag pull requests created by maintainers labels Jun 15, 2021
@jurajkusnier jurajkusnier added this to the 2.4.0 milestone Jun 15, 2021
@jurajkusnier jurajkusnier requested a review from a team June 15, 2021 12:00
@BMItr BMItr self-assigned this Jun 15, 2021
@BMItr
Copy link
Contributor

BMItr commented Jun 15, 2021

@jurajkusnier
general question: why did you delete the valueSetRepo from the other viewmodel?
Wasn't it intended to be in both viewmodels?

@jurajkusnier
Copy link
Contributor Author

@jurajkusnier
general question: why did you delete the valueSetRepo from the other viewmodel?
Wasn't it intended to be in both viewmodels?

I moved just the trigger for updating value sets, to be updated earlier (in the certificates screen). Once the values are updated they are stored in shared preferences so it's not necessary to call this trigger from the second screen.

@harambasicluka harambasicluka added the prio PRs to review first. label Jun 16, 2021
@BMItr
Copy link
Contributor

BMItr commented Jun 16, 2021

@jurajkusnier
general question: why did you delete the valueSetRepo from the other viewmodel?
Wasn't it intended to be in both viewmodels?

I moved just the trigger for updating value sets, to be updated earlier (in the certificates screen). Once the values are updated they are stored in shared preferences so it's not necessary to call this trigger from the second screen.

Yes I got it.
Could you please check whether the Viewmodel is called BEFORE the testCertificat && BEFORE the Vaccination Part?
If its not the case we will move the bug around.

@jurajkusnier
Copy link
Contributor Author

@jurajkusnier
general question: why did you delete the valueSetRepo from the other viewmodel?
Wasn't it intended to be in both viewmodels?

I moved just the trigger for updating value sets, to be updated earlier (in the certificates screen). Once the values are updated they are stored in shared preferences so it's not necessary to call this trigger from the second screen.

Yes I got it.
Could you please check whether the Viewmodel is called BEFORE the testCertificat && BEFORE the Vaccination Part?
If its not the case we will move the bug around.

Yes, as we moved Vaccinations to the Certificates tab, I don't see any way how to access Vaccinations without opening the Certificates tab at least once.

BMItr
BMItr previously approved these changes Jun 16, 2021
@jurajkusnier
Copy link
Contributor Author

@jurajkusnier
general question: why did you delete the valueSetRepo from the other viewmodel?
Wasn't it intended to be in both viewmodels?

I moved just the trigger for updating value sets, to be updated earlier (in the certificates screen). Once the values are updated they are stored in shared preferences so it's not necessary to call this trigger from the second screen.

Yes I got it.
Could you please check whether the Viewmodel is called BEFORE the testCertificat && BEFORE the Vaccination Part?
If its not the case we will move the bug around.

Yes, as we moved Vaccinations to the Certificates tab, I don't see any way how to access Vaccinations without opening the Certificates tab at least once.

I reverted changes in VaccinationListViewModel after discussion regarding concerns for potential future changes in screen flow

# Conflicts:
#	Corona-Warn-App/src/main/java/de/rki/coronawarnapp/covidcertificate/test/ui/CertificatesViewModel.kt
@mtwalli mtwalli self-assigned this Jun 16, 2021
@sonarcloud
Copy link

sonarcloud bot commented Jun 16, 2021

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@harambasicluka harambasicluka merged commit dc9a6e7 into release/2.4.x Jun 16, 2021
@harambasicluka harambasicluka deleted the feature/7781-update_value_sets branch June 16, 2021 13:02
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working maintainers Tag pull requests created by maintainers prio PRs to review first.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants