Pp 2310 qr code unsupported warning flag#1180
Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces a new remote configuration flag (unsupportedQRCodeWarningEnabled) to change how invalid QR codes are handled in the Capture camera flow, showing an “unsupported payment QR code” alert (with actions) when enabled.
Changes:
- Add
unsupportedQRCodeWarningEnabledtoClientConfiguration(API + JSON test fixtures) and propagate it intoGiniCaptureUserDefaultsStorage. - Update camera QR handling to optionally present an alert for unsupported QR codes and temporarily pause/resume QR detection.
- Add new localized strings (EN/DE) for the alert title and actions.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| CaptureSDK/GiniCaptureSDK/Sources/GiniCaptureSDK/Resources/en.lproj/Localizable.strings | Adds EN localizations for the unsupported-QR alert UI. |
| CaptureSDK/GiniCaptureSDK/Sources/GiniCaptureSDK/Resources/de.lproj/Localizable.strings | Adds DE localizations for the unsupported-QR alert UI. |
| CaptureSDK/GiniCaptureSDK/Sources/GiniCaptureSDK/Core/Storage/GiniCaptureUserDefaultsStorage.swift | Persists the new remote config flag into CaptureSDK user defaults storage. |
| CaptureSDK/GiniCaptureSDK/Sources/GiniCaptureSDK/Core/Screens/Camera/CameraPreviewViewController.swift | Exposes the camera instance so CameraViewController can pause/resume QR detection. |
| CaptureSDK/GiniCaptureSDK/Sources/GiniCaptureSDK/Core/Screens/Camera/Camera/CameraViewController.swift | Implements new invalid-QR behavior: snapshot flag once per session and show alert + pause/resume QR detection. |
| CaptureSDK/GiniCaptureSDK/Sources/GiniCaptureSDK/Core/Screens/Camera/Camera.swift | Adds QR detection pause/resume API and stores metadata output reference to enable toggling detection. |
| BankSDK/GiniBankSDK/Sources/GiniBankSDK/Core/GiniBankNetworkingScreenApiCoordinator.swift | Maps backend configuration into CaptureSDK defaults for the new flag. |
| BankAPILibrary/GiniBankAPILibrary/Tests/GiniBankAPILibraryTests/Resources/clientConfiguration.json | Extends JSON fixture with unsupportedQRCodeWarningEnabled. |
| BankAPILibrary/GiniBankAPILibrary/Tests/GiniBankAPILibraryTests/ClientConfigurationTests.swift | Extends tests to cover the new configuration property. |
| BankAPILibrary/GiniBankAPILibrary/Sources/GiniBankAPILibrary/Documents/Configuration/ClientConfiguration.swift | Adds the new configuration property and initializer parameter. |
Comments suppressed due to low confidence (1)
BankAPILibrary/GiniBankAPILibrary/Sources/GiniBankAPILibrary/Documents/Configuration/ClientConfiguration.swift:36
- The initializer documentation comment doesn’t match the repository’s required Swift doc style for parameter lists (use
- Parameters:and indent each parameter under it). Updating this block also keeps the newunsupportedQRCodeWarningEnabledentry consistent.
/**
Creates a new `ClientConfiguration` instance.
- Parameters:
- clientID: A unique identifier for the client.
…iguration test helper PP-2310
…tupQRScanningOutput PP-2310
…y setter for pause/resume coverage PP-2310
zladzeyka
left a comment
There was a problem hiding this comment.
Thanks @qnaveed87, left a couple of suggestions.
Could please double check what will happen if the popup will be dismissed somehow bu the system, or use will put the app in the background? Will QR code detection be resumed?
| hideQRCodeTask = DispatchWorkItem(block: { | ||
| self.resetQRCodeScanning(isValid: false) | ||
| }) | ||
| DispatchQueue.main.asyncAfter(deadline: .now() + 1.5, execute: hideQRCodeTask!) |
There was a problem hiding this comment.
[Suggestion]: Please handle force unwrap here
| static let cameraTitle = NSLocalizedStringPreferredFormat("ginicapture.navigationbar.camera.title", | ||
| comment: "Camera title") | ||
|
|
||
| static let unsupportedQRAlertTitleKey = "ginicapture.QRscanning.alert.title" |
There was a problem hiding this comment.
[Suggestion]: Wrapping all keys in the constants looks like overhead... was is need for lint?
We used before static let unsupportedQRAlertTitle = NSLocalizedStringPreferredFormat( "ginicapture.QRscanning.alert.title", comment: "Unsupported QR code alert title")
There was a problem hiding this comment.
Yes. If we keep everything in a single line, SwiftLint raises a warning. We also follow the same pattern in PaymentDueHint
| private var isValidIBANDetected: Bool = false | ||
| // Snapshot of the backend flag taken on the first invalid QR scan. | ||
| // Stays fixed for the session so all repeated scans show the same feedback type. | ||
| private var sessionUnsupportedQRCodeWarningEnabled: Bool? |
There was a problem hiding this comment.
[Suggestion]: Looks like one boolean should be enough like
private var sessionUnsupportedQRCodeWarningEnabled = false
There was a problem hiding this comment.
Second bool is for manage the session to have 1 type of alert within same session.
Here is the AC fro the ticket
`Given the user already triggered an invalid QR-code warning earlier in the same SDK session, when the user scans a second (or subsequent) invalid QR-code, the SDK shows the same warning type as the first time.
This holds even if the configuration endpoint response resolves in between and changes the flag value (e.g. unsupportedQRCodeWarningEnabled becomes true).
Example: On first install, no ClientConfiguration is stored, so the default applies. The user scans an invalid QR-code before the endpoint responds and sees the old yellow warning. While they go back, the endpoint resolves with unsupportedQRCodeWarningEnabled = true. The user scans a second invalid QR-code and still sees the old yellow warning, not the new popup. The new popup only appears from the next SDK session onward.`
| // repeated scans in the same session. A separate boolean guards the snapshot | ||
| // because assigning nil to a Bool? still leaves it nil, making a nil-check | ||
| // unreliable as a "has snapshotted" gate. | ||
| if !isWarningFlagSnapshotted { |
There was a problem hiding this comment.
if we keep 1 boolean
| if !isWarningFlagSnapshotted { | |
| if !sessionUnsupportedQRCodeWarningEnabled { | |
| sessionUnsupportedQRCodeWarningEnabled = | |
| GiniCaptureUserDefaultsStorage.unsupportedQRCodeWarningEnabled == true | |
| } | |
| if sessionUnsupportedQRCodeWarningEnabled { | |
| showUnsupportedQRCodeAlert() | |
| } else { |
There was a problem hiding this comment.
Implemented as per the AC - 7
| } | ||
|
|
||
| private func showUnsupportedQRCodeAlert() { | ||
| let generator = UINotificationFeedbackGenerator() |
There was a problem hiding this comment.
| let generator = UINotificationFeedbackGenerator() | |
| guard !isUnsupportedQRAlertPresented else { return } | |
| isUnsupportedQRAlertPresented = true | |
| let generator = UINotificationFeedbackGenerator() |
There was a problem hiding this comment.
Implemented as per the AC - 7
| qrCodeOverLay.configureQrCodeOverlay(withCorrectQrCode: false) | ||
| } | ||
|
|
||
| private func showUnsupportedQRCodeAlert() { |
There was a problem hiding this comment.
[Clarification]: pauseQRDetection() schedules its work asynchronously on sessionQueue, but the QR delegate fires per-frame and fans out via DispatchQueue.main.async for each frame. By the time the pause actually takes effect, several showUnsupportedQRCodeAlert() calls are already queued on the main thread-each one firing sendGiniAnalyticsEventForInvalidQRCode() and playVoiceOverMessage(). UIKit silently swallows the duplicate present() calls so there's no crash, but analytics and voice-over fire N times. Introducing `guard in suggestion below can be a fix.
There was a problem hiding this comment.
I am not able to re produce issue with voice over but added guard so that it wont happen fro any edge case
…owQRCodeFeedback - Added Constants.hideQRCodeDelay PP-2310
|
|
|



Pull Request Description
PP-2310
This PR introduces a new remote configuration flag (unsupportedQRCodeWarningEnabled) to change how invalid QR codes are handled in the Capture camera flow, showing an “unsupported payment QR code” alert (with actions) when enabled.
Notes for Reviewers