Skip to content
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

Metadata Network Connection #306

Open
wants to merge 18 commits into
base: fraud-signals-metadata
Choose a base branch
from

Conversation

tobitech
Copy link
Contributor

@tobitech tobitech commented Mar 12, 2025

Story: https://app.shortcut.com/smileid/story/15152/

Summary

  • Add a Network connection provider class to collect an array of current connection type record.
  • I have done a huge refactor on how we collect metadata

Known Issues

I'm exploring how to make the code easier to follow and trace for other people reading the code

Test Instructions

  • Run Selfie Capture Job and check that the network connection type was added to the metadata sent.

Screenshot

N/A.

@tobitech tobitech self-assigned this Mar 12, 2025
@tobitech tobitech requested a review from a team as a code owner March 12, 2025 14:00
Copy link

github-actions bot commented Mar 12, 2025

Warnings
⚠️ The source files were changed, but the tests remain unmodified. Consider updating or adding to the tests to match the source changes.
⚠️

Sources/SmileID/Classes/BiometricKYC/OrchestratedBiometricKycViewModel.swift#L67 - Opening braces should be preceded by a single space and on the same line as the declaration. (opening_brace)

⚠️

Sources/SmileID/Classes/BiometricKYC/OrchestratedBiometricKycViewModel.swift#L181 - TODOs should be resolved (- Fix when Michael changes thi...). (todo)

⚠️

Sources/SmileID/Classes/DocumentVerification/Model/OrchestratedDocumentVerificationViewModel.swift#L119 - Function should have complexity 10 or less: currently complexity equals 17 (cyclomatic_complexity)

⚠️

Sources/SmileID/Classes/DocumentVerification/Model/OrchestratedDocumentVerificationViewModel.swift#L212 - TODOs should be resolved (- Fix when Michael changes thi...). (todo)

⚠️

Sources/SmileID/Classes/DocumentVerification/Model/OrchestratedDocumentVerificationViewModel.swift#L347 - Colons should be next to the identifier when specifying a type and next to the key in dictionary literals. (colon)

⚠️

Sources/SmileID/Classes/DocumentVerification/Model/OrchestratedDocumentVerificationViewModel.swift#L373 - Colons should be next to the identifier when specifying a type and next to the key in dictionary literals. (colon)

⚠️

Sources/SmileID/Classes/SelfieCapture/SelfieViewModel.swift#L223 - TODOs should be resolved (Use mouth deformation as an al...). (todo)

Generated by 🚫 Danger Swift against 5fce76d

@tobitech tobitech changed the title Metadata Network Info Metadata Network Connection Mar 12, 2025
Copy link
Contributor

@robin-smileid robin-smileid left a comment

Choose a reason for hiding this comment

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

  1. Please address the formatting suggestion.
  2. Let's make the network connection an array and recording it from init till submission and record any changes as discussed in our call today
  3. I'm getting the following error at the moment:
    IMG_2915
    Otherwise it looks good. Thanks @tobitech .

@tobitech tobitech requested a review from robin-smileid March 18, 2025 19:40
Copy link
Contributor

@robin-smileid robin-smileid left a comment

Choose a reason for hiding this comment

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

Left some minor changes. I'll approve since it looks fine and those aren't blockers, but please attend if you think it make sense to change those. Tested the flow on the selfie capture and it adds the network changes as expected.

@tobitech tobitech requested a review from robin-smileid March 19, 2025 12:06
Copy link
Contributor

@robin-smileid robin-smileid left a comment

Choose a reason for hiding this comment

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

Looks fine to me. Did a test on Selfie enrollment and it works fine with the new metadata field being added.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants