Merged
Conversation
…e Acuant document said it's a possible outcome.
… the translated message turns out to be default. Seems caused by some library bug due to value collision of the hash.
changelog: User-Facing Improvements, Document Capturing, Improve image related inline error messages.
dawei-nava
requested review from
a team,
charleyf,
daviddsilvanava and
eileen-nava
and removed request for
a team
daviddsilvanava
approved these changes
Oct 23, 2023
daviddsilvanava
left a comment
daviddsilvanava
left a comment
There was a problem hiding this comment.
Deferring to Kelli, but screenshots looked good to me
Contributor
Author
|
@kellular , attached screenshots. |
eileen-nava
approved these changes
Oct 25, 2023
| end | ||
|
|
||
| def errors_from_http_status(status) | ||
| case status |
|
|
||
| error_keys.each do |category| | ||
| response.errors[category]&.map! do |plain_error| | ||
| cat_errors = response.errors[category] |
Contributor
There was a problem hiding this comment.
I read the commit message, and that's an interesting bug! Glad you figured out a way to fix it. This looks good.
Contributor
Author
There was a problem hiding this comment.
@eileen-nava , yeah very similar to the one we have with hash keys collision. Here I have some test case failed and some worked.
| action | ||
| expect(response.status).to eq(400) | ||
| expect(json[:success]).to eq(false) | ||
| expect(json[:remaining_attempts]).to be_a_kind_of(Numeric) |
| end | ||
| end | ||
|
|
||
| shared_examples_for 'inline error for 4xx status shown' do |status| |
Contributor
There was a problem hiding this comment.
Snaps for shared_examples_for 👍🏻
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🎫 Ticket
LG-10889: Fix bug where user does not see inline errors when Acuant AssureID service encountered errors indicated by http status code 438, 439, 440.
🛠 Summary of changes
Add inline error message for Acuant response status code specific to 438, 439 and 440.
📜 Testing Plan
Testing in dev.
Http status 438:
Http status 439:
Http status 440:
In higher environment:
This is Acuant specific, so testing is not straight forward, basic testing can be done with images not satisfying size requirement which is 2025 x 1275 pixels.
👀 Screenshots
Http status 438: image load failure
English:
Spanish:
French:
Http status 439: pixel depth
English:
Spanish:
French:
Http status 440: image size
English:
Spanish:
French: