Skip to content

LG-12080: Send selfie_status to FE #10073

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.

Sign up for GitHub

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

Merged
charleyf merged 12 commits into from
Feb 14, 2024
Merged

LG-12080: Send selfie_status to FE #10073

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
5053ab9
Draft in `selfie_status` send to BE
charleyf Feb 12, 2024
52cd3d2
Fix the naming of the liveness check
charleyf Feb 12, 2024
7b4c047
Add to mock proofer
charleyf Feb 12, 2024
052f975
Don't modify mock proofer
charleyf Feb 13, 2024
a3be2a3
Fix fallback value
charleyf Feb 13, 2024
d8dc2a8
Add `?` to method name
charleyf Feb 13, 2024
334bc49
Merge branch 'main' into charley/lg-12080-send-selfie-success-to-fe
charleyf Feb 13, 2024
efd9f5f
changelog: Internal, In-Person Proofing, send selfie failure to the FE
charleyf Feb 13, 2024
cd53477
Revert change to presenter
charleyf Feb 13, 2024
c96a613
Add selfie_status to response
charleyf Feb 13, 2024
79f849f
LG-12080: add a test where expecting correct structure.
dawei-nava Feb 13, 2024
2c288be
Add liveness_checking_required to mock proofer
charleyf Feb 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions app/presenters/image_upload_response_presenter.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,9 @@ def as_json(*)
json[:ocr_pii] = ocr_pii
json[:result_failed] = doc_auth_result_failed?
json[:doc_type_supported] = doc_type_supported?
json[:selfie_live] = selfie_live? if show_selfie_failures
json[:selfie_quality_good] = selfie_quality_good? if show_selfie_failures
json[:selfie_status] = selfie_status if show_selfie_failures?
json[:selfie_live] = selfie_live? if show_selfie_failures?
json[:selfie_quality_good] = selfie_quality_good? if show_selfie_failures?
json[:failed_image_fingerprints] = failed_fingerprints
json
end
Expand Down Expand Up @@ -90,10 +91,14 @@ def failed_fingerprints
@form_response.extra[:failed_image_fingerprints] || { front: [], back: [], selfie: [] }
end

def show_selfie_failures
def show_selfie_failures?
@form_response.extra[:liveness_checking_required] == true
end

def selfie_status
@form_response.respond_to?(:selfie_status) ? @form_response.selfie_status : :not_processed
end

def selfie_live?
@form_response.respond_to?(:selfie_live?) ? @form_response.selfie_live? : true
end
Expand Down
2 changes: 2 additions & 0 deletions app/services/doc_auth/mock/result_response.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ def initialize(uploaded_file, config, selfie_required = false)
portrait_match_results: portrait_match_results,
billed: true,
classification_info: classification_info,
liveness_checking_required: @selfie_required,
}.compact,
)
end
Expand Down Expand Up @@ -245,6 +246,7 @@ def create_response_info(
liveness_enabled: liveness_enabled,
classification_info: classification_info,
portrait_match_results: selfie_check_performed? ? portrait_match_results : nil,
extra: { liveness_checking_required: liveness_enabled },
}.compact
end
end
Expand Down
3 changes: 3 additions & 0 deletions app/services/doc_auth/response.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ def initialize(
pii_from_doc: {},
attention_with_barcode: false,
doc_type_supported: true,
selfie_status: :not_processed,
selfie_live: true,
selfie_quality_good: true
)
Expand All @@ -28,6 +29,7 @@ def initialize(
@pii_from_doc = pii_from_doc
@attention_with_barcode = attention_with_barcode
@doc_type_supported = doc_type_supported
@selfie_status = selfie_status
@selfie_live = selfie_live
@selfie_quality_good = selfie_quality_good
end
Expand All @@ -41,6 +43,7 @@ def merge(other)
pii_from_doc: pii_from_doc.merge(other.pii_from_doc),
attention_with_barcode: attention_with_barcode? || other.attention_with_barcode?,
doc_type_supported: doc_type_supported? || other.doc_type_supported?,
selfie_status: selfie_status,
selfie_live: selfie_live?,
Copy link
Copy Markdown
Contributor Author

@charleyf charleyf Feb 13, 2024

Choose a reason for hiding this comment

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

Response to this:
I think that confusingly we already have selfie_status tests in various specs. Search for "xpect(response.selfie_status).to eq to find a few.

Copy link
Copy Markdown
Contributor

@dawei-nava dawei-nava Feb 13, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

@charleyf, my point is that expect(response.extra[:liveness_checking_required]).to eq(false/true) should work properly for result_response, unless we add it to extra of result_response , the following method in presenter will always be false for mock client

  def show_selfie_failures?
    @form_response.extra[:liveness_checking_required] == true
  end

I think variable name liveness_enabled changed to liveness_checking_required

Copy link
Copy Markdown
Contributor

@night-jellyfish night-jellyfish Feb 13, 2024

Choose a reason for hiding this comment

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

I could be wrong, but I think what @dawei-nava is saying in the linked comment is that extra hash in the mock proofer should match the extra in the real acuant response.

But, I might not quite be understanding that right. From what I can see the liveness_enabled is included in the Mock Proofer's response here. But I could see an argument for adding the selfie status.

Maybe I'm way off base here though.

Copy link
Copy Markdown
Contributor

@night-jellyfish night-jellyfish Feb 13, 2024

Choose a reason for hiding this comment

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

Oh and while I was typing out my response I see Dawei replied! I'm still wrapping my head around it but I think maybe my comment was wrong, and I misunderstood.

Copy link
Copy Markdown
Contributor

@dawei-nava dawei-nava Feb 13, 2024

Choose a reason for hiding this comment

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

Yes, this following basically is the contract between the presenter and response(mock or trueid etc). The presenter expects it can fetch @form_response.extra[:liveness_checking_required]

 def show_selfie_failures?
    @form_response.extra[:liveness_checking_required] == true
  end

Copy link
Copy Markdown
Contributor

@dawei-nava dawei-nava Feb 13, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

To clarify what I meant, I just pushed a check to the result_response_spec.rb to show it does not contains the information the presenter expected.

Copy link
Copy Markdown
Contributor Author

@charleyf charleyf Feb 14, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Thank you both for clarifying and thanks for adding that test Dawei.

I think variable name liveness_enabled changed to liveness_checking_required

I'm not sure what happened, but I do think that I'm going to address it by changing liveness_enabled to liveness_checking_required in the mock proofer.

selfie_quality_good: selfie_quality_good?,
)
Expand Down
6 changes: 6 additions & 0 deletions spec/services/doc_auth/mock/result_response_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,7 @@
doc_auth_result: DocAuth::Acuant::ResultCodes::PASSED.name,
billed: true,
classification_info: {},
liveness_checking_required: false,
)
expect(response.doc_auth_success?).to eq(true)
expect(response.selfie_status).to eq(:not_processed)
Expand Down Expand Up @@ -332,6 +333,7 @@
doc_auth_result: DocAuth::Acuant::ResultCodes::CAUTION.name,
billed: true,
classification_info: {},
liveness_checking_required: false,
)
end
end
Expand All @@ -357,6 +359,7 @@
doc_auth_result: DocAuth::Acuant::ResultCodes::FAILED.name,
billed: true,
classification_info: {},
liveness_checking_required: false,
)
end
end
Expand Down Expand Up @@ -403,6 +406,7 @@
doc_auth_result: DocAuth::Acuant::ResultCodes::PASSED.name,
billed: true,
classification_info: {},
liveness_checking_required: false,
)
end
end
Expand Down Expand Up @@ -700,6 +704,7 @@
expect(response.extra[:portrait_match_results]).to eq(selfie_results)
expect(response.doc_auth_success?).to eq(true)
expect(response.selfie_status).to eq(:fail)
expect(response.extra[:liveness_checking_required]).to eq(true)
end
end
end
Expand All @@ -713,6 +718,7 @@
expect(response.extra).not_to have_key(:portrait_match_results)
expect(response.doc_auth_success?).to eq(true)
expect(response.selfie_status).to eq(:not_processed)
expect(response.extra[:liveness_checking_required]).to eq(false)
end
end
end