Skip to content

IdV Rate limiting events#12076

Merged
Sgtpluck merged 11 commits intomainfrom
dmm/idv-events
Apr 29, 2025
Merged

IdV Rate limiting events#12076
Sgtpluck merged 11 commits intomainfrom
dmm/idv-events

Conversation

@Sgtpluck
Copy link
Contributor

@Sgtpluck Sgtpluck commented Apr 15, 2025

🎫 Ticket

Link to the relevant ticket:
163

🛠 Summary of changes

Adding a general rate limiter event. Opening draft PR to run tests (feature specs seem to be broken locally)

@Sgtpluck Sgtpluck marked this pull request as ready for review April 16, 2025 20:00
@Sgtpluck Sgtpluck requested a review from mitchellhenke April 16, 2025 20:00
Copy link
Contributor Author

Choose a reason for hiding this comment

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

these two additions and the document_capture_step_helper.rb addition seeme to make these tests run better (although not perfectly) locally -- am happy to remove though if it's adding to the noise of this larger PR

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i wonder if there is a more holistically way to solve this issue :(

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, I think based on similar discussion here and a follow-up in LG-16112 to find a more comprehensive fix, it's preferable to back out the page.has_content/button? changes.

Adding additional expectations for have_current_path seems good though.

@Sgtpluck Sgtpluck changed the title Dmm/idv events IdV Rate limiting events Apr 16, 2025
Copy link
Contributor

@mitchellhenke mitchellhenke left a comment

Choose a reason for hiding this comment

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

One small removal suggestion

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
page.has_content? t('forms.buttons.back')

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed in 02db17a

Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i added it because i was confused about why the tracker was receiving idv_rate_limited twice, based on the name of the test. i thought it might be helpful for other people like myself, but happy to remove it if you think it's not adding value. (very few folks have to dive into the tests of other teams in the way i'm doing it right now, after all)

@Sgtpluck Sgtpluck merged commit bcc52ae into main Apr 29, 2025
1 check passed
@Sgtpluck Sgtpluck deleted the dmm/idv-events branch April 29, 2025 17:11
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