-
Notifications
You must be signed in to change notification settings - Fork 4
Metadata: Environment and Device Information #315
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
base: fraud-signals-metadata
Are you sure you want to change the base?
Conversation
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
Generated by 🚫 Danger Swift against a638c10 |
var orientationString: String { | ||
switch UIDevice.current.orientation { | ||
case .portrait, .portraitUpsideDown: | ||
return "Portrait" | ||
case .landscapeLeft, .landscapeRight: | ||
return "Landscape" | ||
case .faceUp: | ||
return "FaceUp" | ||
case .faceDown: | ||
return "FaceDown" | ||
case .unknown: | ||
// Default to UI orientation if device orientation is unknown | ||
let interfaceOrientation = UIApplication.shared.windows.first?.windowScene?.interfaceOrientation | ||
switch interfaceOrientation { | ||
case .portrait, .portraitUpsideDown: | ||
return "Portrait" | ||
case .landscapeLeft, .landscapeRight: | ||
return "Landscape" | ||
case .unknown, .none: | ||
return "Unknown" | ||
@unknown default: | ||
return "Unknown" | ||
} | ||
@unknown default: | ||
return "Unknown" | ||
} | ||
} |
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: The code is using the deprecated UIApplication.shared.windows
property which was deprecated in iOS 15. Use UIApplication.shared.connectedScenes
instead to get the interface orientation. [possible issue, importance: 8]
var orientationString: String { | |
switch UIDevice.current.orientation { | |
case .portrait, .portraitUpsideDown: | |
return "Portrait" | |
case .landscapeLeft, .landscapeRight: | |
return "Landscape" | |
case .faceUp: | |
return "FaceUp" | |
case .faceDown: | |
return "FaceDown" | |
case .unknown: | |
// Default to UI orientation if device orientation is unknown | |
let interfaceOrientation = UIApplication.shared.windows.first?.windowScene?.interfaceOrientation | |
switch interfaceOrientation { | |
case .portrait, .portraitUpsideDown: | |
return "Portrait" | |
case .landscapeLeft, .landscapeRight: | |
return "Landscape" | |
case .unknown, .none: | |
return "Unknown" | |
@unknown default: | |
return "Unknown" | |
} | |
@unknown default: | |
return "Unknown" | |
} | |
} | |
var orientationString: String { | |
switch UIDevice.current.orientation { | |
case .portrait, .portraitUpsideDown: | |
return "Portrait" | |
case .landscapeLeft, .landscapeRight: | |
return "Landscape" | |
case .faceUp: | |
return "FaceUp" | |
case .faceDown: | |
return "FaceDown" | |
case .unknown: | |
// Default to UI orientation if device orientation is unknown | |
if let windowScene = UIApplication.shared.connectedScenes.first as? UIWindowScene { | |
switch windowScene.interfaceOrientation { | |
case .portrait, .portraitUpsideDown: | |
return "Portrait" | |
case .landscapeLeft, .landscapeRight: | |
return "Landscape" | |
case .unknown: | |
return "Unknown" | |
@unknown default: | |
return "Unknown" | |
} | |
} | |
return "Unknown" | |
@unknown default: | |
return "Unknown" | |
} | |
} |
@@ -21,6 +21,34 @@ extension UIDevice { | |||
?? identifier | |||
} | |||
|
|||
var orientationString: 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.
This value is also something that fluctuates over time (ie. depending on when it is recorded). I do wonder if we want to collect this in an array? The question then is how often and when do we want to sample this value? cc @Yeboahmedia
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.
Code changes look fine to me. Tested the flow and the values are added correctly. Left a comment in case we want to collect device orientation as an array.
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.
@tobitech @robin-smileid I think like the orientation the available memory is also something that changes overtime, is that something we want to set on init when we init the metadata or this is something that should be set maybe when we begin a flow or something?
@JNdhlovu We actually record the total memory (RAM) of the device. This shouldn't change over time. We can also think about recording memory changes (available memory) and then that would need to recorded differently. |
return "ARM64" | ||
#elseif arch(x86_64) | ||
return "x86_64" | ||
#elseif arch(i386) | ||
return "i386" | ||
#else | ||
return "Unknown" |
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 keep this all lower cased?
return "ARM64" | |
#elseif arch(x86_64) | |
return "x86_64" | |
#elseif arch(i386) | |
return "i386" | |
#else | |
return "Unknown" | |
return "arm64" | |
#elseif arch(x86_64) | |
return "x86_64" | |
#elseif arch(i386) | |
return "i386" | |
#else | |
return "unknown" |
return "FaceUp" | ||
case .faceDown: | ||
return "FaceDown" | ||
case .unknown: |
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.
And maybe also here lower-case unknown
. Just to keep consistency in case we can't extract 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.
Code changes look fine to me so far. Can we please also add the device orientation recording when we do a document capture? Left some comments in terms of default value unknown
in case we can't extract a value.
User description
Story: https://app.shortcut.com/smileid/story/15152/
Summary
Known Issues
N/A.
Test Instructions
Screenshot
N/A
PR Type
Enhancement
Description
Added device environment metadata collection
Fixed allowNewEnroll parameter type
Implemented extensions for device information
Changes walkthrough 📝
3 files
Fix allowNewEnroll parameter type
Fix allowNewEnroll parameter type
Fix allowNewEnroll parameter type
5 files
Add new metadata keys
Implement collection of environment metadata
Add extension for memory and architecture info
Add device orientation detection
Add screen resolution formatting