Skip to content

LG-10170 rename Throttle to RateLimiter#8706

Merged
soniaconnolly merged 19 commits intomainfrom
sonia-lg-10170-rename-throttle-to-rate-limit
Jul 5, 2023
Merged

LG-10170 rename Throttle to RateLimiter#8706
soniaconnolly merged 19 commits intomainfrom
sonia-lg-10170-rename-throttle-to-rate-limit

Conversation

@soniaconnolly
Copy link
Contributor

@soniaconnolly soniaconnolly commented Jun 30, 2023

🎫 Ticket

LG-10170

🛠 Summary of changes

Rename Throttle class to RateLimiter, including all uses
Rename Throttle methods that use 'throttle'
Rename throttle variables to rate_limiter
Rename throttled? method to limited?
Rename increment_to_throttled! to increment_to_limited!

Left unchanged for now:

  • analytics events
  • config/initializers
  • routes: idv_session_errors_throttled_url
  • translation keys

📜 Testing Plan

  • Create account, start IdV
  • Get rate limited at DocumentCapture with a yaml file that causes an error
  • Create account, start IdV
  • Get rate limited at VerifyInfo by entering an SSN that doesn't start with 900 or 666.
  • Create account, start IdV
  • Get rate limited at phone step by entering 703-555-5555

@soniaconnolly soniaconnolly changed the title LG-10170 rename throttle to rate limit LG-10170 rename Throttle to RateLimiter Jul 3, 2023
flow_session[:document_capture_session_uuid]
end

def throttled?
Copy link
Contributor

@amirbey amirbey Jul 5, 2023

Choose a reason for hiding this comment

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

nice to get rid of this 🥳


@expires_at = throttle.expires_at
@expires_at = rate_limiter.expires_at
render :throttled
Copy link
Contributor

Choose a reason for hiding this comment

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

to be changed in follow up ticket/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.

Yes, I was avoiding changing anything route-related. Could this be changed by changing just the name of the template file? I still think it should be in a followup PR.

@amirbey amirbey self-requested a review July 5, 2023 20:32
Copy link
Contributor

@amirbey amirbey left a comment

Choose a reason for hiding this comment

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

LGTM 👍🏿

@soniaconnolly soniaconnolly merged commit a6100b6 into main Jul 5, 2023
@soniaconnolly soniaconnolly deleted the sonia-lg-10170-rename-throttle-to-rate-limit branch July 5, 2023 21:05
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