feat(app): Update the invalid pin error handling scenario #511

talwinder50 · 2023-07-25T00:08:39Z

Used wallet sdk parsed error api to get the parsed details and category
Based on parsed Category (INVALID_GRANT) a messaging is shown to the user "Oops! Something went wrong!
Try re-entering the pin or scan the new QR code."
For the other categories we will show what ever is been sent in the details by parsed method.
If user has entered wrong pin they can try re-entering or restart the operation as per the messaging.
- If pin was wrong and user re-entered the right pin, user will be redirected to credential preview screen

Things to fix:

Add a loader once the user hit the submit or renter button Add a loader once the user enter the submit or cancel button #509
Make the button submit/ Cancel button fix to the bottom right now they drag further to the Botton when error widget appears Make the button submit/ Cancel button fix to the bottom #510

codecov-commenter · 2023-07-25T00:11:15Z

Codecov Report

Patch coverage: 91.30% and project coverage change: +0.01% 🎉

Comparison is base (be47cf0) 89.47% compared to head (bb01d0c) 89.48%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #511      +/-   ##
==========================================
+ Coverage   89.47%   89.48%   +0.01%     
==========================================
  Files          93       93              
  Lines        3639     3643       +4     
==========================================
+ Hits         3256     3260       +4     
  Misses        251      251              
  Partials      132      132

Files Changed	Coverage Δ
cmd/wallet-sdk-gomobile/wrapper/error.go	`87.09% <91.30%> (+1.91%)`	⬆️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

DRK3 · 2023-07-25T14:52:03Z

demo/app/lib/views/otp.dart

-                            errorMessage = err.details!;
+                            var resp =  await WalletSDKPlugin.parseWalletSDKError(localizedErrorMessage: err.details);
+                            if(resp.category == "INVALID_GRANT"){
+                              errorMessage = "Try re-entering the pin or scan the new QR code";


Suggested change

errorMessage = "Try re-entering the pin or scan the new QR code";

errorMessage = "Try re-entering the PIN or scan a new QR code";

DRK3 · 2023-07-25T14:53:45Z

demo/app/ios/Runner/flutterPlugin.swift

+        guard let localizedErrorMessage = arguments["localizedErrorMessage"] as? String else{
+            return  result(FlutterError.init(code: "NATIVE_ERR",
+                                             message: "error while parsing wallet sdk error",
+                                             details: "parameter localizedErrorMessage is missed"))


I think you mean localizedErrorMessage is missing?

DRK3 · 2023-07-25T14:54:24Z

demo/app/ios/Runner/flutterPlugin.swift

+    public func parseWalletSDKError(arguments: Dictionary<String, Any>, result: @escaping FlutterResult){
+        guard let localizedErrorMessage = arguments["localizedErrorMessage"] as? String else{
+            return  result(FlutterError.init(code: "NATIVE_ERR",
+                                             message: "error while parsing wallet sdk error",


Suggested change

message: "error while parsing wallet sdk error",

message: "error while parsing Wallet-SDK error",

DRK3 · 2023-07-25T14:56:00Z

demo/app/ios/Runner/flutterPlugin.swift

+        let parsedError = WalleterrorParse(localizedErrorMessage)
+
+        var parsedErrorResult :[String:Any] = [
+            "category":   parsedError!.category,
+            "details":  parsedError!.details,
+            "code":  parsedError!.code,
+            "traceID":  parsedError!.traceID
+        ]
+        result(parsedErrorResult)
+    }


Can you do the force unwrap once on line 548 instead of on every line from 551-554? Like this:

let parsedError = WalleterrorParse(localizedErrorMessage)!

DRK3 · 2023-07-25T15:00:05Z

demo/app/lib/views/otp.dart

                          if (err is PlatformException &&
                              err.message != null &&
                              err.message!.isNotEmpty) {
-                            errorMessage = err.details!;
+                            var resp =  await WalletSDKPlugin.parseWalletSDKError(localizedErrorMessage: err.details);
+                            if(resp.category == "INVALID_GRANT"){


Suggested change

if(resp.category == "INVALID_GRANT"){

if (resp.category == "INVALID_GRANT"){

DRK3 · 2023-07-25T15:01:25Z

demo/app/lib/views/otp.dart

                          if (err is PlatformException &&
                              err.message != null &&
                              err.message!.isNotEmpty) {
-                            errorMessage = err.details!;
+                            var resp =  await WalletSDKPlugin.parseWalletSDKError(localizedErrorMessage: err.details);
+                            if(resp.category == "INVALID_GRANT"){


There's one more error category you can add a check for: INVALID_TOKEN. In the case of INVALID_TOKEN, however, re-entering the PIN won't help, so in that case the error message should just tell the user to scan a new QR code.

DRK3

Looks good overall - I suggested some recommended UI text changes and another error code we can check for to improve the user experience, however I don't see any major issues.

demo/app/android/app/src/main/kotlin/walletsdk/flutter/app/MainActivity.kt

DRK3 · 2023-07-25T16:22:38Z

demo/app/lib/wallet_sdk/wallet_sdk_mobile.dart

@@ -65,6 +66,12 @@ class WalletSDK extends WalletPlatform {
    }
  }

+  Future<WalletSDKError> parseWalletSDKError({required String localizedErrorMessage}) async {
+    var parsedWalletError = await methodChannel.invokeMethod(
+        'parseWalletSDKError', <String, dynamic>{'localizedErrorMessage': localizedErrorMessage});


Can this map be made more specific?

You mean map of string , string ? Instead of dynamic

Other than that walletsdkerror is the object type we are rendering the object which we get from sdk.

I believe, It cant be more specific than that. I will change that if thats what u mean

Even better: see if it can just be a simple string instead of a map

DRK3 · 2023-07-25T16:22:56Z

demo/app/lib/wallet_sdk/wallet_sdk_model.dart

+  }
+
+  Map<String, dynamic> toJson() {
+    final Map<String, dynamic> data = <String, dynamic>{};


This map looks more generic than it needs to be

DRK3 · 2023-07-25T16:23:02Z

demo/app/lib/wallet_sdk/wallet_sdk_model.dart

+    return data;
+  }
+
+  factory WalletSDKError.fromJson(Map<String, dynamic> json) {


DRK3 · 2023-07-25T19:10:16Z

The commit says "iOS", but I see android code there too. Does this also add support for the Android app, or is that going to be a separate PR?

Signed-off-by: Talwinder kaur <[email protected]>

sonarcloud · 2023-07-25T19:38:14Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
1 Code Smell

No Coverage information
No Duplication information

cla-bot bot added the cla-signed label Jul 25, 2023

talwinder50 changed the title ~~feat(app): Update the invalid pin error handling scenario~~ feat(app): Update the invalid pin error handling scenario iOS Jul 25, 2023

talwinder50 requested review from DRK3, birtony and rolsonquadras July 25, 2023 00:09

rolsonquadras approved these changes Jul 25, 2023

View reviewed changes

DRK3 reviewed Jul 25, 2023

View reviewed changes

talwinder50 force-pushed the invalidpin branch 2 times, most recently from b6a5a91 to 8205b92 Compare July 25, 2023 15:42

talwinder50 requested a review from DRK3 July 25, 2023 15:42

DRK3 reviewed Jul 25, 2023

View reviewed changes

demo/app/android/app/src/main/kotlin/walletsdk/flutter/app/MainActivity.kt Outdated Show resolved Hide resolved

talwinder50 force-pushed the invalidpin branch from 8205b92 to 270b292 Compare July 25, 2023 15:49

talwinder50 requested a review from DRK3 July 25, 2023 15:49

DRK3 reviewed Jul 25, 2023

View reviewed changes

talwinder50 force-pushed the invalidpin branch 2 times, most recently from a3897f6 to bb01d0c Compare July 25, 2023 19:03

talwinder50 requested a review from DRK3 July 25, 2023 19:06

talwinder50 changed the title ~~feat(app): Update the invalid pin error handling scenario iOS~~ feat(app): Update the invalid pin error handling scenario Jul 25, 2023

DRK3 approved these changes Jul 25, 2023

View reviewed changes

feat(app): Update the invalid pin error handling scenario

03f50de

Signed-off-by: Talwinder kaur <[email protected]>

talwinder50 force-pushed the invalidpin branch from bb01d0c to 03f50de Compare July 25, 2023 19:30

talwinder50 merged commit c6d776b into trustbloc:main Jul 26, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(app): Update the invalid pin error handling scenario #511

feat(app): Update the invalid pin error handling scenario #511

talwinder50 commented Jul 25, 2023 •

edited

Loading

codecov-commenter commented Jul 25, 2023 •

edited

Loading

DRK3 Jul 25, 2023 •

edited

Loading

DRK3 Jul 25, 2023

DRK3 Jul 25, 2023

DRK3 Jul 25, 2023

DRK3 Jul 25, 2023

DRK3 Jul 25, 2023

DRK3 left a comment

DRK3 Jul 25, 2023

talwinder50 Jul 25, 2023

DRK3 Jul 25, 2023

DRK3 Jul 25, 2023

DRK3 Jul 25, 2023

DRK3 commented Jul 25, 2023

sonarcloud bot commented Jul 25, 2023

	errorMessage = "Try re-entering the pin or scan the new QR code";
	errorMessage = "Try re-entering the PIN or scan a new QR code";

	message: "error while parsing wallet sdk error",
	message: "error while parsing Wallet-SDK error",

	if(resp.category == "INVALID_GRANT"){
	if (resp.category == "INVALID_GRANT"){

feat(app): Update the invalid pin error handling scenario #511

feat(app): Update the invalid pin error handling scenario #511

Conversation

talwinder50 commented Jul 25, 2023 • edited Loading

codecov-commenter commented Jul 25, 2023 • edited Loading

Codecov Report

DRK3 Jul 25, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DRK3 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DRK3 commented Jul 25, 2023

sonarcloud bot commented Jul 25, 2023

talwinder50 commented Jul 25, 2023 •

edited

Loading

codecov-commenter commented Jul 25, 2023 •

edited

Loading

DRK3 Jul 25, 2023 •

edited

Loading