Skip to content

Modularize Attempts API Tracker - FCMS Prework#12472

Merged
astrogeco merged 7 commits intomainfrom
modular_attempts
Sep 9, 2025
Merged

Modularize Attempts API Tracker - FCMS Prework#12472
astrogeco merged 7 commits intomainfrom
modular_attempts

Conversation

@astrogeco
Copy link
Copy Markdown
Contributor

@astrogeco astrogeco commented Sep 2, 2025

🎫 Ticket

🔒 GL-Data-1105

🛠 Summary of changes

In preparation to leverage the attempts API tracker for the Data Warehouse ingestion, we need to break up track_event in tracker.rb. Also need to break out various methods in redis_client.rb

In tracker, defined the following new methods

  • jwe
  • extra_attributes
  • event_metadata
  • extra_metadata
  • failure_metadata

In redis_client, defined the following:

  • initialize - sets a new @redis_pool variable to wrap REDIS_ATTEMPTS_API_POOL
  • event_ttl_seconds - wraps IdentityConfig.store.attempts_api_event_ttl_seconds

📜 Testing Plan

  • Make sure specs still run succesfully
  • Enable attempts API and use sinatra app to generate some attempts events
  • Check that events still work and have the expected content

👀 Screenshots

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

@astrogeco astrogeco requested a review from Sgtpluck September 2, 2025 18:11
@astrogeco astrogeco force-pushed the modular_attempts branch 5 times, most recently from 2adcda4 to 07530ff Compare September 2, 2025 18:18
- jwe
- extra_attributes
- event_metadata
- extra_metadata
- failure_metadata

Co-authored-by: Davi <davida.marion@gsa.gov>
@astrogeco astrogeco force-pushed the modular_attempts branch 2 times, most recently from 46bc6cf to 11bbba1 Compare September 2, 2025 19:20
@astrogeco astrogeco marked this pull request as ready for review September 3, 2025 18:27
@astrogeco
Copy link
Copy Markdown
Contributor Author

Realized I didn't have the redis client changes, added them with the last two commits

Copy link
Copy Markdown
Contributor

@Sgtpluck Sgtpluck left a comment

Choose a reason for hiding this comment

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

looks good to me! i had one comment, but i don't think it's an issue either way.

client_port: CloudFrontHeaderParser.new(request).client_port,
aws_region: IdentityConfig.store.aws_region,
google_analytics_cookies: google_analytics_cookies(request),
}.merge!(extra_metadata(metadata:))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

i think the ! is technically unnecessary here but i don't think it's hurting anything (the difference between merge and merge! is whether it's replacing the object or returning a copy of it. since we turned event_metadata into a method, it doesn't need to replace the object. but, not a big deal.

@astrogeco
Copy link
Copy Markdown
Contributor Author

Testing evidence with Sinatra

Screenshot 2025-09-09 at 3 16 48 PM

@astrogeco astrogeco merged commit 46dd4be into main Sep 9, 2025
1 check passed
@astrogeco astrogeco deleted the modular_attempts branch September 9, 2025 19:54
mdiarra3 added a commit that referenced this pull request Sep 11, 2025
* Update "Get Help" link to be more specific (#12485)

Fixes https://cm-jira.usa.gov/browse/LG-16738

changelog: Bug Fixes, One Account, Update "Get Help" link to be more specific

* LG-16647:  remove zipcode extension if invalid (#12484)

* truncate trueid zipcode if chars missing

changelog: Internal, Document Authentication, Parse 5 digits of zipcode if unexpected format received

* ensure is a string and fix typo

* add spec to show shortening of zip with invalid extension

* rename context for specs

* LG-16722: remove passport A/B and allowed (#12483)

* remove passport A/B and allowed

* fix specs

* fix specs

* fix specs

* fix specs

* update ipp passports on ff and default passports to true

* fix specs

* fix specs

* include passports enabled in ipp passports enabled

* add choose Id type to complete steps before doc cap

* fix specs

* fix specs

* fix specs

* fix specs

* disable ipp passports now that passports are enabled by default

* fix specs

* fix specs

* fix specs

* comment Todo preconditions for choose id type standard

* fix specs

* fix specs

* remove byebug

* fix specs

* fix specs

* rename doc_auth.instructions.bullet1b to doc_auth.instructions.bullet1'

* fix specs

* undo this change

* fix specs

* fix specs

* fix specs

* fix specs

* fix specs

* fix specs

* fix specs

* fix specs

* fix specs

* fix specs

* fix specs

* add choose id type to analytics specs

* fix specs

* remove comments

* happy linting

* changelog: Internal, Document Authentication, Remove A/B test for permits and permits_allowed from IdVSession

* add local attrs test if passport disabled

* Modularize Attempts API Tracker - FCMS Prework (#12472)

* changelog: Internal, Attempts-API, Modularize attempts API tracker methods to allow class inheritance and modification

In tracker, defined the following new methods
- jwe
- extra_attributes
- event_metadata
- extra_metadata
- failure_metadata

In redis_client, defined the following:
- initialize - sets a new @redis_pool variable to wrap REDIS_ATTEMPTS_API_POOL
event_ttl_seconds - wraps IdentityConfig.store.attempts_api_event_ttl_seconds

---------

Co-authored-by: Davi <davida.marion@gsa.gov>

* LG-16672 Fix rate limit in hybrid flow (#12449)

When the user would submit successfully on the last attempt of doc auth
the application would rate limit the user. This was due to a race
condition between the javascript poller on the link sent page, and the
doc auth rate limit incrementing and processing time of the images. If
the image processing took longer than the polling time. The user would
be rate limited because the rate limit value was already incremented
when the value was polled.

changelog: Bug Fixes, Hybrid Doc Auth, Fix rate limiting bug for a successful last submission in IdV hybrid flow during doc auth.

* LG-16559 fetch new docv session if user changes document type (#12458)

* add method choose_document_type_changed? to see if doc type was changed

* skip exsisting docv capture if user has changed doc type

* add changelog

changelog: Bug Fixes, Doc Auth, fetch new docv session if user changes document type

* set socure docv attributes to nil in choose_id_type_concern

* remove choose_document_type_changed? from socure document capture controllers

* remove DocumentCaptureSession#choose_document_type_changed?

this is no longer needed as we set the variables to nil in the
choose_id_type_concern

* clean up socure document capture controller specs

* go back to old return if socure_capture_url is present

* move set_passport_requested methods into DocumentCaptureSession

* lint

* simplify @url instance variable

* move document_capture_session methods up to reduce repeating

* rename dcs functions to request_passport! and request_state_id!

* config update (#12488)

[skip changelog]

---------

Co-authored-by: Vraj Mohan <vraj.mohan@gsa.gov>
Co-authored-by: Amir Reavis-Bey <amir.reavis-bey@gsa.gov>
Co-authored-by: Gerardo E. Cruz-Ortiz <59618057+astrogeco@users.noreply.github.com>
Co-authored-by: Davi <davida.marion@gsa.gov>
Co-authored-by: Shane Chesnutt <shane.chesnutt@gsa.gov>
Co-authored-by: Alex Bradley <alexander.bradley@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.

2 participants