-
Notifications
You must be signed in to change notification settings - Fork 284
Risk Calculation Errors - Alert Refinement #868
Conversation
- Updated Strings - Now directs to FAQ for ENErrors .unsupported, .internal, and .rateLimited
/* General - Links */ | ||
|
||
"General_moreInfo_URL" = "https://www.coronawarn.app/de/faq/"; | ||
|
||
"General_moreInfo_URL_EN5" = "https://www.coronawarn.app/de/faq/#ENError5"; | ||
|
||
"General_moreInfo_URL_EN11" = "https://www.coronawarn.app/de/faq/#ENError11"; | ||
|
||
"General_moreInfo_URL_EN13" = "https://www.coronawarn.app/de/faq/#ENError13"; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I shifted these around as they are used in several places (not just ExposureSubmission) anymore.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rewrote the Exposure Submission error handlings to use these links, removed the old traces of the links.
default: return nil | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This extension seems like it could go into a separate file, but I wasn't sure if it's worth it as it's only used here.
It is not private as we'd like to unit test it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved this to a separate extension, and added the ExposureSubmission stuff into it as well. Check Error+FAQUrl.swift
for this change.
static let moreInfoURL = AppStrings.Links.appFaq | ||
static let moreInfoURLEN5 = AppStrings.Links.appFaqENError5 | ||
static let moreInfoURLEN11 = AppStrings.Links.appFaqENError11 | ||
static let moreInfoURLEN13 = AppStrings.Links.appFaqENError13 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left these be in order to not change too many unrelated files in this PR, but we can certainly adjust the references where they're used.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am going to adjust these in the Exposure Submission flow to get rid of the indirection.
Hi @kaluju , |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new error message looks fine.
PLEASE DO NOT MERGE THIS. I want to do adjustments. |
I have merged the exposure submissions errors & the errors introduced in this pull request to the best of my knowledge, in order to remove duplication and have a more pleasant code base. Thanks to @johannesrohwer for helping me check for consistencies. Please check the changes again, if you'd be so kind. |
Checklist
Description
Previously,
UIAlertController
s triggered by the app because of a failed exposure detection (when anExposureDetection.DidEndPrematurelyReason
is thrown) would have a simple OK button and nothing else. This PR adds some more details, as well as links severalENError
s to their corresponding FAQ entries:ENError(.unsupported)
-> FAQENError(.internal)
-> FAQENError(.rateLimited)
-> FAQAlert before:
Alert after: