Fix outdated syntax

[JPrevost]

Redis was misconfigured for a long time in timdex-api. That was patched yesterday. On doing so, it exposed this incorrect configuration in rack-attack that was missed during a version upgrade.

https://mitlibraries.atlassian.net/browse/TIMX-679

This was already fixed in TIMDEX-UI and only missed in TIMDEX API because rack-attack was not realized to have been not working for different reasons.

Developer

• All new ENV is documented in README
• All new ENV has been added to Heroku Pipeline, Staging and Prod
• ANDI or Wave has been run in accordance to
  our guide and
  all issues introduced by these changes have been resolved or opened as new
  issues (link to those issues in the Pull Request details above)
• Stakeholder approval has been confirmed (or is not needed)

Code Reviewer

• The commit message is clear and follows our guidelines
  (not just this pull request message)
• There are appropriate tests covering any new functionality
• The documentation has been updated or is unnecessary
• The changes have been verified
• New dependencies are appropriate or there were no changes

Requires database migrations?

NO

Includes new or updated dependencies?

NO

[matt-bernhardt]

I see that the PR has been merged, and this ticket closed. I also think I remember that you wanted to do a retroactive review - so that's what I'm doing now.

I'm fine with the approach we're using here, but have two questions to help me understand.

First, I'm wondering whether it would make sense to extract the calculation of reset to a helper function? Part of me says "no, this is the only place where this value gets used" - but then I wonder whether having a separate helper would let us document its operation more clearly. So I'm torn.

Second, you mention in the PR text that TIMDEX-UI has already fixed this issue, but when I look at the initializer over there, it doesn't seem similar to what we're doing here - and when I look at the Rack::Attack docmentation, what they indicate seems to be what we're moving away from (which caused the issue in Sentry that started this). My question about all of this is whether I'm understanding the intent of this change (that we have a fallback method of calculating reset in case env doesn't have a value in rack.attack.match_data)

Neither of these two questions are me asking for a change, or saying that I'd have denied a code review - this works, which is the concern.

[matt-bernhardt]

Is there a world where we extract lines 85-88 to some helper method to make the assembly of this value more separate / testable? I really like the approach of calculating reset separately from the invocation of it down on line 94, but working out the logic of how we got there took me a minute.