Skip to content

LG-12372: sdk autocaptured id images route to non-cropping workflow#10131

Merged
amirbey merged 12 commits intomainfrom
amirbey/LG-12372-cropped-workflow
Feb 28, 2024
Merged

LG-12372: sdk autocaptured id images route to non-cropping workflow#10131
amirbey merged 12 commits intomainfrom
amirbey/LG-12372-cropped-workflow

Conversation

@amirbey
Copy link
Contributor

@amirbey amirbey commented Feb 21, 2024

🎫 Ticket

LG-12372

🛠 Summary of changes

Only use TrueID cropped workflows when ID images are cropped (autocaptured via Acuant SDK)

📜 Testing Plan

  • Tested via unit tests

@amirbey amirbey self-assigned this Feb 21, 2024
@amirbey amirbey force-pushed the amirbey/LG-12372-cropped-workflow branch 2 times, most recently from 2588db7 to e78f4a8 Compare February 26, 2024 22:24
@amirbey amirbey marked this pull request as ready for review February 26, 2024 22:26
@amirbey amirbey force-pushed the amirbey/LG-12372-cropped-workflow branch from 6f9d00c to 71e2a90 Compare February 27, 2024 01:38
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you refactor this to
image_metadata.dig(:front, :acuantCaptureMode) == 'AUTO' && image_metadata.dig(:back, :acuantCaptureMode) == 'AUTO'? I'd expect that to still work, and it would have less conditional logic.

Copy link
Contributor

Choose a reason for hiding this comment

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

FWIW, I don't think we need to update the AcuantClient, since we're already planning on removing it.

Copy link
Contributor Author

@amirbey amirbey Feb 28, 2024

Choose a reason for hiding this comment

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

@eileen-nava - i understand that we are not currently using acuant ... however, the the post_images method is polymorphic and wherever defined I think the signature should match. if acuant will be removed in a future sprint, I am missing the downside here 🤔

Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Would it be worth adding an attr_reader for @images_cropped? Then you could just call images_cropped on line 75.

Copy link
Contributor

@dawei-nava dawei-nava Feb 28, 2024

Choose a reason for hiding this comment

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

@eileen-nava , if this prop not used by other classes, then I guess it's ok to use prop directly(especially not accessed multiple places in the owning class)?

Copy link
Contributor

Choose a reason for hiding this comment

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

@dawei-nava Hmmm, that's a good point. It does look like front_image, back_image, selfie_image, and liveness_checking_required are all accessed once in the owning class, too.

@amirbey amirbey requested a review from eileen-nava February 28, 2024 16:19
@amirbey amirbey force-pushed the amirbey/LG-12372-cropped-workflow branch from afb64fd to 6ff8361 Compare February 28, 2024 20:30
@amirbey amirbey force-pushed the amirbey/LG-12372-cropped-workflow branch from 6ff8361 to bd0d1a0 Compare February 28, 2024 20:36
Copy link
Contributor

@eileen-nava eileen-nava left a comment

Choose a reason for hiding this comment

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

Approved! 👍🏻

@amirbey amirbey merged commit 7d53024 into main Feb 28, 2024
@amirbey amirbey deleted the amirbey/LG-12372-cropped-workflow branch February 28, 2024 22:56
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.

3 participants