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

Make sure all EN-Erros link to the FAQ page correctly #1998

Closed
2 tasks done
Ein-Tim opened this issue Feb 16, 2021 · 22 comments · Fixed by #2024
Closed
2 tasks done

Make sure all EN-Erros link to the FAQ page correctly #1998

Ein-Tim opened this issue Feb 16, 2021 · 22 comments · Fixed by #2024
Assignees
Labels
bug Something isn't working Fix 1.13 Fix is planned for 1.13 mirrored-to-jira This item is also tracked internally in JIRA

Comments

@Ein-Tim
Copy link
Contributor

Ein-Tim commented Feb 16, 2021

Avoid duplicates

  • Bug is not mentioned in the FAQ
  • Bug is not already reported in another issue

Technical details

  • Device name: -
  • iOS Version: 14.4
  • App Version: 1.12.1

Describe the bug

I have some indication that ENError-Pop ups do not correctly link to the General_moreInfo_URL defined in

"General_moreInfo_URL" = "https://www.coronawarn.app/en/faq/";

Instead, if the user clicks "Mehr Erfahren", nothing happens but the pop-up closes. I can't confirm this myself, since it is not easy to reproduce such errors.
I think that this does not affect, the separately defined, ENErrors 5, 11 and 13.

Steps to reproduce the issue

  1. Get a ENError (but 5, 11 and 13)
  2. Click on "Mehr Erfahren"
  3. Pop-Up closes
  4. Nothing happens

Expected behaviour

The FAQ website should be opened.


Related Issue


Internal Tracking ID: EXPOSUREAPP-5223

@ndegendogo
Copy link
Contributor

@Ein-Tim did you actually see this popup? If yes: do you know if the corresponding risk check was triggered from background or foreground?

@Ein-Tim
Copy link
Contributor Author

Ein-Tim commented Feb 17, 2021

@ndegendogo

I don't experience this issue, I just talked with somebody on Twitter who encountered ENError 14.

But I also noticed back in #1353 that I think that it is (still) not working.

@ndegendogo
Copy link
Contributor

Thanks @Ein-Tim.
I cannot promise for sure, but if I find time I am going to play with this scenario in my simulator ...

@dsarkar
Copy link
Member

dsarkar commented Feb 17, 2021

@ndegendogo @Ein-Tim
Do you have any idea how to trigger one of the ENErrors?

@dsarkar dsarkar added the in review Moderators are investigating how to best proceed with the issue label Feb 17, 2021
@Ein-Tim
Copy link
Contributor Author

Ein-Tim commented Feb 17, 2021

@dsarkar ENError 14 is triggered if a MDM is preventing CWA to use the ENF/the ENF is deactivated by a MDM.

But it's all a bit tricky because some ENErorrs are also hidden from the user, others aren't but the most common (13, 11 and 5) seem to work correctly (but not sure about that).

@dsarkar
Copy link
Member

dsarkar commented Feb 17, 2021

@Ein-Tim Ok. ENError13 ( API was called too often ), I think one could trigger it: At the end of the day, after 4-6 checks, reset app several times (or re-install). So we could check at least that one.

@Ein-Tim
Copy link
Contributor Author

Ein-Tim commented Feb 17, 2021

@dsarkar Yes, I triggered this yesterday and it seemed to work. It's only about the Erros which don't have a own FAQ entry. This should best be tried out by the devs.

@dsarkar
Copy link
Member

dsarkar commented Feb 17, 2021

@Ein-Tim ok

@ndegendogo
Copy link
Contributor

@dsarkar I plan to use the simulator / community build, then cherry-pick my recent PR for #1918, and do a local modification so that the mock returns error 14
😎😎

@ndegendogo
Copy link
Contributor

ndegendogo commented Feb 17, 2021

I can reproduce it with 1.12.1. (simulator / community build, mock locally modified for error behaviour).
ENError 13 forwards to the FAQ, ENError 14 shows no action on "more info".

Root cause is here. Three specific error codes are extended with an URL to the corresponding FAQ, but the default gets no URL.

I guess it should use one of the other URLs here, else the list of URLs to be extended.

@dsarkar

@dsarkar dsarkar added bug Something isn't working mirrored-to-jira This item is also tracked internally in JIRA labels Feb 18, 2021
@dsarkar dsarkar removed the in review Moderators are investigating how to best proceed with the issue label Feb 18, 2021
@dsarkar
Copy link
Member

dsarkar commented Feb 18, 2021

@ndegendogo @Ein-Tim Thanks for reporting and analyzing!

Internal Tracking ID: EXPOSUREAPP-5223

@ndegendogo
Copy link
Contributor

@dsarkar please ask the devs what is the fastest way to get this fix in.
If they have time to work on it I guess this would be fastest.
Else I could volunteer for a PR; but of course I don't want to spend duplicate effort.

@dsarkar dsarkar added the Fix 1.13 Fix is planned for 1.13 label Feb 18, 2021
@dsarkar
Copy link
Member

dsarkar commented Feb 18, 2021

@ndegendogo They are working already on it as far as I can see in the ticket. However, I will forward your generous offer and suggest the developer contacting you here on this issue if this is applicable and/or practical for the devs.

@dsarkar
Copy link
Member

dsarkar commented Feb 18, 2021

@ndegendogo @Ein-Tim FYI PR https://github.com/corona-warn-app/cwa-app-ios/pull/2024/files

@Ein-Tim
Copy link
Contributor Author

Ein-Tim commented Feb 18, 2021

@dsarkar Thanks!

@ndegendogo I will leave this issue open until Version 1.13 is released, maybe you would like to test this via the emulator again then? Thank you so much for you help here! 👍

@ndegendogo
Copy link
Contributor

Wow - this is fast 🚀 amazing 😀

@dsarkar Please tell the devs!

But then ... I have another question - not sure if I should open a new ticket or can just append here?

Actually, in the same file there is another, very similar extension ExposureSubmissionError and I am wondering if that should be also extended with a default URL.
However, so far I have not analysed its usage (from the name it could be related to submission of DEK keys)

@dsarkar could you please give the developers the hint to double-check that?

(sorry - I should have mentioned it in the first place ... but it was already a bit late yesterday ...)

@heinezen
Copy link
Member

heinezen commented Feb 21, 2021

@ndegendogo

EDIT: This is just an explanatory comment, we will still try to work someting out with the devs

From the perspective of the FAQ it's not necessary to make an FAQ entry for every error type. In general, these errors should be caught and handled internally (e.g. try again in case of submission). Most of them are not meant to be shown to the user. And if they are, the app should show it's own user-friendly error message.

If the error is still shown to a lot of users, we will make an FAQ entry of course. However, chances are we are mirroring the Apple FAQ's if we do this preemptively for every error.


Corona-Warn-App Open Source Team

@ndegendogo
Copy link
Contributor

@heinezen no problem. I have meanwhile analysed the source myself, and as far as I see all is fine.

ExposureSubmissionCoordinator.showErrorAlert adds the 'More Info' button only if the error has an URL, so the user gets the popup with just one button in the 'default' case, which is fine.

This is different with AppDelegate.makeAlertController, which always adds a second button.

(And a button which promises 'More info' but does not fulfill this promise is at least surprising.)

@dsarkar
Copy link
Member

dsarkar commented Feb 23, 2021

@ndegendogo regarding your comment #1998 (comment), I forwarded both, your question and your amazement to the developer.

@freshking
Copy link
Member

@heinezen no problem. I have meanwhile analysed the source myself, and as far as I see all is fine.

ExposureSubmissionCoordinator.showErrorAlert adds the 'More Info' button only if the error has an URL, so the user gets the popup with just one button in the 'default' case, which is fine.

This is different with AppDelegate.makeAlertController, which always adds a second button.

(And a button which promises 'More info' but does not fulfill this promise is at least surprising.)

@ndegendogo This is the correct answer to your question about the ExposureSubmissionError.

@Ein-Tim
Copy link
Contributor Author

Ein-Tim commented Mar 5, 2021

Version 1.13 which fixed this has been released to GitHub and will be available to all users in the next 48h.

See blog post:

DE: https://www.coronawarn.app/de/blog/2021-03-04-corona-warn-app-version-1-13/
EN: https://www.coronawarn.app/en/blog/2021-03-04-corona-warn-app-version-1-13/

See GitHub release:

iOS: https://github.com/corona-warn-app/cwa-app-ios/releases/tag/v1.13.0
Android: https://github.com/corona-warn-app/cwa-app-android/releases/tag/v1.13.2

This issue will now be closed.

@Ein-Tim Ein-Tim closed this as completed Mar 5, 2021
@dsarkar
Copy link
Member

dsarkar commented Mar 5, 2021

@Ein-Tim Thanks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working Fix 1.13 Fix is planned for 1.13 mirrored-to-jira This item is also tracked internally in JIRA
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants