-
Notifications
You must be signed in to change notification settings - Fork 21
Feature/jsonresults #150
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feature/jsonresults #150
Conversation
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
try { | ||
SmileID.moshi | ||
.newBuilder() | ||
.add(SelfieCaptureResultAdapter.FACTORY) | ||
.build() | ||
val json = | ||
try { | ||
moshi | ||
.adapter(SmartSelfieCaptureResult::class.java) | ||
.toJson(result) | ||
} catch (e: Exception) { | ||
onError(e) | ||
return@onFinished | ||
} | ||
json?.let { js -> | ||
onSuccessJson(js) | ||
.adapter(SmileIDCaptureResult.SmartSelfieCapture::class.java) | ||
.toJson( | ||
SmileIDCaptureResult.SmartSelfieCapture( | ||
selfieFile = res.data.selfieFile.absolutePath, | ||
livenessFiles = | ||
res.data.livenessFiles.map { file -> | ||
file.absolutePath | ||
}, | ||
), | ||
) | ||
} catch (e: Exception) { | ||
onError(e) | ||
return | ||
} |
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.
Suggestion: Add null check for selfieFile to prevent NullPointerException when accessing absolutePath property. The selfieFile could potentially be null in some cases. [possible issue, importance: 8]
try { | |
SmileID.moshi | |
.newBuilder() | |
.add(SelfieCaptureResultAdapter.FACTORY) | |
.build() | |
val json = | |
try { | |
moshi | |
.adapter(SmartSelfieCaptureResult::class.java) | |
.toJson(result) | |
} catch (e: Exception) { | |
onError(e) | |
return@onFinished | |
} | |
json?.let { js -> | |
onSuccessJson(js) | |
.adapter(SmileIDCaptureResult.SmartSelfieCapture::class.java) | |
.toJson( | |
SmileIDCaptureResult.SmartSelfieCapture( | |
selfieFile = res.data.selfieFile.absolutePath, | |
livenessFiles = | |
res.data.livenessFiles.map { file -> | |
file.absolutePath | |
}, | |
), | |
) | |
} catch (e: Exception) { | |
onError(e) | |
return | |
} | |
val json = | |
try { | |
SmileID.moshi | |
.adapter(SmileIDCaptureResult.SmartSelfieCapture::class.java) | |
.toJson( | |
SmileIDCaptureResult.SmartSelfieCapture( | |
selfieFile = res.data.selfieFile?.absolutePath, | |
livenessFiles = | |
res.data.livenessFiles.map { file -> | |
file.absolutePath | |
}, | |
), | |
) | |
} catch (e: Exception) { | |
onError(e) | |
return | |
} |
android/src/main/kotlin/com/smileidentity/flutter/SmileIDDocumentVerification.kt
Show resolved
Hide resolved
|
||
@JsonClass(generateAdapter = true, generator = "sealed:type") | ||
sealed class SmileIDCaptureResult { | ||
abstract val selfieFile: String? |
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.
Should we set the default value for all those parameters to null? (across the whole file)
@@ -105,7 +104,12 @@ internal class SmileIDDocumentCaptureView private constructor( | |||
} | |||
DocumentCaptureScreen( | |||
jobId = jobId, | |||
side = if (isDocumentFrontSide) DocumentCaptureSide.Front else DocumentCaptureSide.Back, | |||
side = |
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 inline statement should be fine? Any reason to change that?
.adapter(SmileIDCaptureResult.DocumentCaptureResult::class.java) | ||
.toJson( | ||
SmileIDCaptureResult.DocumentCaptureResult.DocumentCapture( | ||
documentFrontFile = |
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'd keep the inline statements here as well - its less code and more readable to me?
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.
Left a few comments. Also can we please add a changelog entry now that this is a separate PR?
User description
Story: https://app.shortcut.com/smileid/story/xxx
Summary
A few sentences/bullet points about the changes
Known Issues
Any shortcomings in your work. This may include corner cases not correctly handled or issues related
to but not within the scope of your PR. Design compromises should be discussed here if they were not
already discussed above.
Test Instructions
Concise test instructions on how to verify that your feature works as intended. This should include
changes to the development environment (if applicable) and all commands needed to run your work.
Screenshot
If applicable (e.g. UI changes), add screenshots to help explain your work.
PR Type
Enhancement
Description
Added skipApiSubmission flag for SmartSelfie™ capture
Made consent information optional in verification flows
Added useStrictMode for enhanced selfie capture
Updated JSON result handling for consistency
Changes walkthrough 📝
34 files
Added useStrictMode parameter to SmileIDSmartSelfieCaptureView
Renamed consent parameters for consistency
Renamed consent parameters for consistency
Added skipApiSubmission parameter
Added useStrictMode parameter
Added skipApiSubmission parameter
Updated auto-generated code for JSON handling
Made consentInformation optional and updated constructor
Updated enum mappings and added default consent
Updated result handling with new JSON converters
Updated to use new JSON result format
Updated to use new JSON result format
Updated to use new JSON result format
Added skipApiSubmission parameter and refactored
Refactored to use new result handling
Added skipApiSubmission parameter and refactored
Added new base class for selfie views
Added skipApiSubmission parameter and refactored
Added skipApiSubmission parameter and refactored
Updated auto-generated code for JSON handling
Removed in favor of unified result format
Removed in favor of unified result format
Added new unified result format
Removed in favor of unified result format
Removed in favor of unified result format
Added default consent information handling
Made consent information optional
Made consent information optional
Updated auto-generated code for JSON handling
Added skipApiSubmission parameter
Added skipApiSubmission parameter
Added useStrictMode and refactored
Added skipApiSubmission parameter
Bumped version to 10.3.5
1 files
Updated iOS SDK version to 10.4.2
2 files
Added release notes for version 10.3.5
Minor formatting improvements