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

Task/14315 error handling #4895

Merged
merged 57 commits into from
Nov 29, 2022
Merged

Conversation

flxschmidt
Copy link
Contributor

@flxschmidt flxschmidt commented Nov 22, 2022

Description

Implements a API in ExposureSubmissionCoordinator to show alerts regarding SRS Self Report Flow.

Usage

You only have to call showSRSFlowAlert(for:isLoading:), for example by a callback handler of a SRSFlow ViewController.
The method expect a type of an SRSFlowAlert that can be:

  • .preconditionFailed (like app usage time)
  • .consent (like cancel to warn, or confirm to warn)
  • .error (for multiple errors)

Regarding Errors

The .error(error) expects as an associated value, the regarding Error type, to show the regarding SRS Error alert with the specific alert message for that. Errors to give in, have to be conformable to two protocols: SRSErrorAlertProviding and ErrorCodeProviding

The following Error types are now conformable to that:

  • SRSError
  • PPACError
  • OTPError

Example

// Will show the "SUBMISSION_TOO_EARLY" alert message with error code 
// and the time from App Configuration parameter inside
showSRSFlowAlert(for: .error(PPACError.submissionTooEarly), isLoading: isLoading)

Open Points

  • Implementation of .error at the specific code (further story task)
  • cancel consent alert in showCheckinsScreen dismiss handler

Link to Jira

https://jira-ibs.wbs.net.sap/browse/EXPOSUREAPP-14315

Screenshots

SRS Precondition was failed

SRSPrecondition

Some Errors on Screens

Errors

Consent Confirm Warn Others

Updated

Confirm

Consent Cancel Warn Others

Screenshot 2022-11-22 at 20 37 43

Updated

Cancel

@flxschmidt flxschmidt added this to the v3.0.0 milestone Nov 22, 2022
@flxschmidt flxschmidt added the task part of a feature label Nov 22, 2022
@Ein-Tim
Copy link
Contributor

Ein-Tim commented Nov 23, 2022

Some comments on the suggested actions on the pop-ups:

"Sind Sie sicher, dass Sie jetzt andere Personen, mit denen Sie in letzter Zeit Kontakt hatten, warnen wollen?" -> Here I would expect "Andere Warnen" to be in bold (=the suggested actions) and not "Abbrechen".

"Warn-Vorgang abbrechen" -> Here I would expect "Warnen fortsetzten" to be in bold and not "Abbrechen".

@flxschmidt flxschmidt marked this pull request as ready for review November 23, 2022 13:09
@flxschmidt flxschmidt requested review from a team November 23, 2022 13:09
@flxschmidt
Copy link
Contributor Author

Some comments on the suggested actions on the pop-ups:

"Sind Sie sicher, dass Sie jetzt andere Personen, mit denen Sie in letzter Zeit Kontakt hatten, warnen wollen?" -> Here I would expect "Andere Warnen" to be in bold (=the suggested actions) and not "Abbrechen".

"Warn-Vorgang abbrechen" -> Here I would expect "Warnen fortsetzten" to be in bold and not "Abbrechen".

You're right, and I have implemented that. The screenshots are updated as well. Thank you!

@Ein-Tim
Copy link
Contributor

Ein-Tim commented Nov 23, 2022

Thank you very much @flxschmidt!

…error-handling

# Conflicts:
#	src/xcode/ENA/ENA/Resources/Localization/de.lproj/Localizable.strings
#	src/xcode/ENA/ENA/Source/Scenes/ExposureSubmission/ExposureSubmissionCoordinator.swift
#	src/xcode/ENA/ENA/Source/Scenes/ExposureSubmission/ExposureSubmissionCoordinatorModel.swift
#	src/xcode/ENA/ENA/Source/Services/PPAccessControl/Model/SRSError.swift
#	src/xcode/ENA/ENA/Source/View Helpers/AppStrings.swift
Copy link
Contributor

@nickguendling nickguendling left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just some minor comments and I think one finding that might be critical, marked it with a ⚠️ again


alert.preferredAction = confirmAction

viewController.present(alert, animated: true)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couldn't you present on the navigation controller as we do in other places? Then you could get rid of the view controller reference and even get rid of the implicitly unwrapped view controller in this place I think 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That was my first approach, but unfortunately that was not possible, because nav controller is already presenting a VC. Wo don't know which VC is the top most one. I also tried some SOF approaches, but the simplest way is, to give the VC as an associated value to the type.

@naveeddotio naveeddotio merged commit 97e5688 into release/3.0.x Nov 29, 2022
@naveeddotio naveeddotio deleted the task/14315-error-handling branch November 29, 2022 14:50
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
task part of a feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants