search

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

Validity of Certificate uses inconsistent date formats #3830

Closed MikeMcC399 closed 3 years ago

MikeMcC399 commented 3 years ago

Technical details

Describe the bug

Formatting of dates in the Certificate Validation function is inconsistent.

In the example below date / times are displayed in two different formats:

with different separators (. and /), reversed day and month, and short and long forms of the year.

Steps to reproduce the issue

  1. Settings > General management > Language and Input > Language and set default language to English (United States)
  2. Open CWA
  3. Tap "Certificates", tap "CONTINUE"
  4. Tap "+ CERTIFICATE" and scan an EU Digital COVID Certificate
  5. Tap "CHECK VALIDITY"
  6. Accept default Country (Germany) and default Date and time
  7. Tap "CHECK"

Compare date formats on input screen and result screen.

Validity of Certificate 1

Validity of Certificate 2

Expected behavior

Dates in the Certificate Validation function should be displayed in a consistent way.

Possible Fix

Use the locale's date formatting to achieve consistency.

Internal Tracking-ID: EXPOSUREAPP-8750

heinezen commented 3 years ago

@MikeMcC399

Thanks for checkin on this. I've made an internal report to the dev team.

Corona-Warn-App Open Source Team

fynngodau commented 3 years ago

Information on the cause of the error

Here the string is formatted:

https://github.com/corona-warn-app/cwa-app-android/blob/a9bc344cef8e4e9d10279a46c88c9c6a70a7af97/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/covidcertificate/validation/ui/validationresult/common/ValidationResultItemCreator.kt#L115-L117

The DateTime.toShortDateTimeFormat extension function is defined as:

https://github.com/corona-warn-app/cwa-app-android/blob/a9bc344cef8e4e9d10279a46c88c9c6a70a7af97/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/util/TimeAndDateExtensions.kt#L142-L145

However, it does not always return a string in the format that it is annotated as, because DateTimeFormat.shortDateTime() is documented as:

Creates a format that outputs a short datetime format. The format will change as you change the locale of the formatter. […]

Whereas the extension function DateTime.toShortDayFormat() uses a "dd.MM.yy" as a fixed pattern.

dsarkar commented 3 years ago

@fynngodau Thanks. I will forward this to the internal ticket.

MikeMcC399 commented 3 years ago

PR https://github.com/corona-warn-app/cwa-app-android/pull/3987 planned for CWA 2.9 harmonizes date display in the app and it changes the date formatting for Certificate Validation.

With locale set to English (United States)

Even though the format on the input screen and the result screen are different, they should not cause any confusion. Importantly, the date format on the result screen is consistent between the two time stamps on that screen.

If PR https://github.com/corona-warn-app/cwa-app-android/pull/3987 is merged in its current state, I would suggest that this issue (https://github.com/corona-warn-app/cwa-app-android/issues/3830 Internal Tracking-ID: EXPOSUREAPP-8750) can be given the Fix 2.9 label as well as issue https://github.com/corona-warn-app/cwa-app-android/issues/3963.

MikeMcC399 commented 3 years ago

PR https://github.com/corona-warn-app/cwa-app-android/pull/3987 is merged into release/2.10.x

This issue (https://github.com/corona-warn-app/cwa-app-android/issues/3830 Internal Tracking-ID: EXPOSUREAPP-8750) is solved by PR #3957 so this issue (#3830) can be given the Fix 2.10 label.

MikeMcC399 commented 3 years ago

@dsarkar Would you please add the fix 2.10 label to this issue?

MikeMcC399 commented 3 years ago

@dsarkar This issue is fixed in the CWA 2.10.1 Android version which was released today. You may like to add the Fix 2.10 label.

Closing as fixed.

dsarkar commented 3 years ago

@MikeMcC399 Thanks! Forwarded to internal ticket.