Skip to content

LG-14481: DocAuth infinity attempts remaining bug#11238

Merged
amirbey merged 5 commits intomainfrom
amirbey/LG-14481-infinity-bug
Sep 18, 2024
Merged

LG-14481: DocAuth infinity attempts remaining bug#11238
amirbey merged 5 commits intomainfrom
amirbey/LG-14481-infinity-bug

Conversation

@amirbey
Copy link
Contributor

@amirbey amirbey commented Sep 13, 2024

🎫 Ticket

LG-14481

🛠 Summary of changes

Do not display remaining attempts when the remainingSubmitAttempts property has not been changes and is still Infinity

📜 Testing Plan

  1. Upload documents that would trigger a 500 response by the ImageUplioadsController#create
  2. Verify that For security reasons, you have Infinity attempts remaining to add your ID online. is not rendered.

👀 Screenshots

If relevant, include a screenshot or screen capture of the changes.

Before: Screenshot 2024-09-13 at 5 27 42 PM
After: Screenshot 2024-09-13 at 5 26 47 PM

Copy link
Contributor

Choose a reason for hiding this comment

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

do you also want to do a > 0 check on the number of attempts?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it should be rate limited before remainingSubmitAttempts < 0 which'd be pretty concerning ... if that scenario was to arise, it could be helpful to allow it to happen to surface that deeper problem? 🤔

@amirbey amirbey requested review from a team, jmax-gsa and theabrad and removed request for a team September 13, 2024 21:33
@amirbey amirbey marked this pull request as ready for review September 13, 2024 21:33
@amirbey amirbey changed the title LG014481: DocAuth infinity attempts remaining bug LG-14481: DocAuth infinity attempts remaining bug Sep 13, 2024
@amirbey amirbey force-pushed the amirbey/LG-14481-infinity-bug branch from 4fa663b to d8ecc44 Compare September 14, 2024 11:48
Copy link
Contributor

@aduth aduth left a comment

Choose a reason for hiding this comment

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

Can you include test coverage?

@amirbey amirbey requested a review from aduth September 16, 2024 16:59
@amirbey
Copy link
Contributor Author

amirbey commented Sep 16, 2024

Can you include test coverage?

that has been tricky ... to recreate this a 500 has to be thrown by the back end which causes issues with Capybara#raise_server_error! which then my attempts to resovle trigger errors in other tests .. i'll reach out for a pair

@aduth
Copy link
Contributor

aduth commented Sep 16, 2024

Can you include test coverage?

that has been tricky ... to recreate this a 500 has to be thrown by the back end which causes issues with Capybara#raise_server_error! which then my attempts to resovle trigger errors in other tests .. i'll reach out for a pair

I was thinking something in the React component specs, not necessarily a server-side feature spec (although that could be good too, not really sure the circumstances here)

Copy link
Contributor

@theabrad theabrad left a comment

Choose a reason for hiding this comment

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

LGTM

@amirbey amirbey force-pushed the amirbey/LG-14481-infinity-bug branch from 670b593 to b66610f Compare September 18, 2024 15:09
@amirbey amirbey merged commit 05aa3ef into main Sep 18, 2024
@amirbey amirbey deleted the amirbey/LG-14481-infinity-bug branch September 18, 2024 15:32
AShukla-GSA pushed a commit that referenced this pull request Sep 30, 2024
* do not show remaining attempts warning when the user has not received a failed doc auth result

[skip changelog]

* Update app/javascript/packages/document-capture/components/document-capture-review-issues.tsx

Co-authored-by: Andrew Duthie <1779930+aduth@users.noreply.github.com>

* add react test for remiaining events

* Inline props, remove field value for unknown field example

* remove correct file form enforce file

---------

Co-authored-by: Andrew Duthie <1779930+aduth@users.noreply.github.com>
Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov>
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.

4 participants