Skip to content

LG-8050 | Attempts API timestamp parsing fix#7316

Merged
n1zyy merged 3 commits intomainfrom
mattw/LG-8050_irs_timestamp_fix
Nov 8, 2022
Merged

LG-8050 | Attempts API timestamp parsing fix#7316
n1zyy merged 3 commits intomainfrom
mattw/LG-8050_irs_timestamp_fix

Conversation

@n1zyy
Copy link
Contributor

@n1zyy n1zyy commented Nov 8, 2022

🎫 Ticket

https://cm-jira.usa.gov/browse/LG-8050 + https://cm-jira.usa.gov/browse/LG-8051

🛠 Summary of changes

The IRS is sending us timestamps with milliseconds in them, which currently generates an HTTP 422. Just accept this format.

I'm working in a related change to what we call the generated file, based on their request.

Match what the IRS is actually sending

Sneaks in an unrelated change to the generated filename to accommodate
a request they had.

changelog: Improvements, Attempts API, Change timestamp parsing for IRS
@n1zyy n1zyy marked this pull request as ready for review November 8, 2022 21:08
@n1zyy n1zyy requested review from a team and zachmargolis November 8, 2022 21:08
Time.strptime(timestamp_param, '%Y-%m-%dT%H:%M:%S%z')
date_fmt = timestamp_param.match?('\.') ? '%Y-%m-%dT%H:%M:%S.%N%z' : '%Y-%m-%dT%H:%M:%S%z'

Time.strptime(timestamp_param, date_fmt)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hokay, so. I wanted to accommodate both timestamp formats (what everything we built is using, and what the IRS is sending). This felt like the easiest way to accommodate that -- just check for a literal dot.

We could do a more elegant regexp, or potentially nested exception handling -- catch ArgumentError and try again with the other format, potentially catching it again, but that felt gross. This seemed least bad, but I wouldn't put this on my resume as a shining example of the best code I've ever written.


filename =
"FCI-#{IdentityConfig.store.irs_attempt_api_csp_id}_#{formatted_time}_#{digest}.dat.gz.hex"
"FCI-Logingov_#{formatted_time}_#{digest}.dat.gz.hex"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was "LOGIN.gov".

The IRS wanted it to be "Logingov" because they worried that the ".gov" would be treated as a file extension. I don't think we need to accommodate a variable CSP ID, so I just hardcoded it. The "FCI-" portion is already hardcoded.

n1zyy and others added 2 commits November 8, 2022 16:55
Co-authored-by: Zach Margolis <zachmargolis@users.noreply.github.com>
@n1zyy n1zyy merged commit 2c66a27 into main Nov 8, 2022
@n1zyy n1zyy deleted the mattw/LG-8050_irs_timestamp_fix branch November 8, 2022 22:19
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