corona-warn-app / cwa-app-android

Native Android app using the Apple/Google exposure notification API. The CWA development ends on May 31, 2023. You still can warn other users until April 30, 2023. More information:
https://coronawarn.app/en/faq/#ramp_down
Apache License 2.0
2.44k stars 495 forks source link

Regression in date of birth formatting #4014

Closed MikeMcC399 closed 3 years ago

MikeMcC399 commented 3 years ago

Avoid duplicates

Technical details

Describe the bug

PR https://github.com/corona-warn-app/cwa-app-android/pull/3987 has introduced a regression concerning inconsistent formatting regarding dates of birth.

For example, the date of birth in PersonDetailsFragment_cwa.png in release 2.10 should use the same order of month day year according to the English (United States) locale which is used in the date of sample on the same screen.

Instead of showing the date of birth as 1981/03/20, it should be shown as 03/20/1981.


PersonDetailsFragment_cwa from release/2.10.x

PersonDetailsFragment_cwa_2 10_dates


PersonDetailsFragment_cwa from screenshots 2.8 for comparison. In this version the date formats were fixed and did not use the device's locale settings.

PersonDetailsFragment_cwa_2 8_dates

Steps to reproduce the issue

  1. Settings > System > Languages and input > Languages
    set to English (United States)
  2. Settings > System > Date & time
    select "Use locale default"/ disable "Use 24-hour format"
  3. Connect mobile device to Android Studio
  4. In Android Studio Build > Select BuildVariant deviceForTestersDebug
  5. run src/androidTest/java/de/rki/coronawarnapp on emulator
  6. View > Tools Windows > Device File Explorer
  7. Go to /data/data/de.rki.coronawarnapp.test/screenshots
  8. View screenshots and check the formatting of all dates of birth

Expected behavior

Dates of birth should be formatted according to the device's locale setting, except in certificates where a field label indicates formatting using YYYY-MM-DD.


Internal Tracking ID: EXPOSUREAPP-9297

MikeMcC399 commented 3 years ago

@marcauberer This is the new issue, as you requested.

dsarkar commented 3 years ago

@MikeMcC399 Thanks for opening a separate issue: Internal Tracking ID: EXPOSUREAPP-9297.

marcauberer commented 3 years ago

@MikeMcC399 new pr is open. Would be nice, if you could take a look at the screenshots and report back to me if the bug is solved. Thank you!

MikeMcC399 commented 3 years ago

@marcauberer I'll be happy to check the screenshots, however currently on this branch and on release/2.10.x screenshot generation fails at PersonDetailsFragmentTest.kt io.mockk.MockKException: no answer found for: TestCertificate(#41)

I will try to find out if I can go back to a commit in release/2.10.x where it still works.

mtwalli commented 3 years ago

it is already verified by @MikeMcC399 and can be closed

MikeMcC399 commented 3 years ago

Successfully verified in branch release/2.10.x at commit https://github.com/corona-warn-app/cwa-app-android/commit/b2a35fbef27c2c9f2b6870a0592ad031d9e5a10e.

Steps to verify

Pixel 3a API 30 (Android 11) emulator

  1. Settings > System > Languages and input > Languages
    set to English (United States)
  2. Settings > System > Date & time
    select "Use locale default"/ disable "Use 24-hour format"
  3. Connect mobile device to Android Studio
  4. In Android Studio Build > Select BuildVariant deviceForTestersDebug
  5. run PersonDetailsFragmentTest.kt on emulator
  6. View > Tools Windows > Device File Explorer
  7. Go to /data/data/de.rki.coronawarnapp.test/screenshots
  8. View screenshots and check the formatting of all dates of birth in PersonDetailsFragment*.png

image

Now closing, since the regression was only present in an unreleased version and the issue is fixed.

MikeMcC399 commented 3 years ago

@marcauberer PR https://github.com/corona-warn-app/cwa-app-android/pull/4036 did not fix formatting the date-of-birth in certificates for locale English (United Kingdom). At the time that I closed this issue (https://github.com/corona-warn-app/cwa-app-android/issues/4014) I only verified on locale English (United States).

Since PR https://github.com/corona-warn-app/cwa-app-android/pull/4036 has been merged and CWA 2.10.1 has been released I have opened a new issue https://github.com/corona-warn-app/cwa-app-android/issues/4112 to follow up on this regression.

I will not re-open https://github.com/corona-warn-app/cwa-app-android/issues/4014.

dsarkar commented 3 years ago

@MikeMcC399 I forwarded your comment into the new internal ticket.