Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LG-13763: Stop writing to the proofing_components table #11564

Merged
merged 8 commits into from
Dec 2, 2024

Conversation

matthinz
Copy link
Contributor

🎫 Ticket

Link to the relevant ticket:
LG-13763

🛠 Summary of changes

ProofingComponent is a model that tracks, in general terms, how a user has verified their identity. It exists in a 1:1 relationship with User, which gets kind of sticky / tricky for users with multiple Profiles.

Proofing components are useful for logging purposes, so we want to keep them. But instead of storing them attached to User in this way, we are now dynamically deriving them from the user's IdV::Session.

So then, now that we are doing that, we can stop writing to the old user-bound proofing_components stuff. That's what this PR does. We'll be dropping the table itself in a future PR.

Just stop.

These values are now generated dynamically as we need them, then serialized to the Profile.

[skip changelog]
I moved idv_session to be public on the controller (matching the behavior of other controllers that use IdvSessionConcern).

Then updated tests to use dynamically-generated proofing components. This requires reloading the user object. I created https://cm-jira.usa.gov/browse/LG-15167 to address computing the proofing component value without using the User object
We don't have access to idv_session here so we have to wait until the proofing component has been written to the database so we can access it.
We were clearing out the _user's_ proofing component here, but not doing anything with the profile.
@matthinz matthinz requested a review from a team November 26, 2024 18:07
Copy link
Contributor

@jmhooper jmhooper left a comment

Choose a reason for hiding this comment

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

🎊🎊🎊

Copy link
Contributor

@n1zyy n1zyy left a comment

Choose a reason for hiding this comment

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

I certainly like the idea of this. The tests made me realize I might not understand the whole picture here as well as I thought I did. That's likely on me. I wouldn't mind syncing up next time we're both online, unless someone beats me to approving this.

user:,
)

expect(proofing_components.document_check).to be_nil
Copy link
Contributor

Choose a reason for hiding this comment

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

I feel like I'm missing some context here. Is an equivalent to Idv::ProofingComponents.new happening in the controller?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Idv::ProofingComponents reads info from sessions (Rails, user, and Idv::) to derive the current components. Previously the controller was updating the proofing_component attached to the user. Here we're just taking the various sessions the controller is interacting with and essentially checking that it made the changes we are expecting. You could make an argument that this test might be duplicative now (we should be testing that the controller is updating Idv::Session appropriately elsewhere)

@matthinz
Copy link
Contributor Author

matthinz commented Dec 2, 2024

This was previously blocked by #11527, which is now in prod. Sooooo.... :shipit:

@matthinz matthinz merged commit 9004d02 into main Dec 2, 2024
2 checks passed
@matthinz matthinz deleted the matthinz/13763-proofing-components-stop-just-stop branch December 2, 2024 21:36
matthinz added a commit that referenced this pull request Dec 18, 2024
We missed this when we got rid of the ProofingComponent model in #11564

[skip changelog]
matthinz added a commit that referenced this pull request Dec 19, 2024
We missed this when we got rid of the ProofingComponent model in #11564

[skip changelog]
matthinz added a commit that referenced this pull request Dec 19, 2024
We missed this when we got rid of the ProofingComponent model in #11564

[skip changelog]
matthinz added a commit that referenced this pull request Jan 6, 2025
Since #11564 we've stopped writing to this table, so now is the time to drop it.

This table was previously used as a temporary datastore for proofing component values as a user moves through IdV. We now dynamically derive those values from the Idv::Session, so this table is not needed any longer.

[skip changelog]
@matthinz matthinz mentioned this pull request Jan 6, 2025
1 task
matthinz added a commit that referenced this pull request Jan 7, 2025
* Drop proofing_components table

Since #11564 we've stopped writing to this table, so now is the time to drop it.

This table was previously used as a temporary datastore for proofing component values as a user moves through IdV. We now dynamically derive those values from the Idv::Session, so this table is not needed any longer.

[skip changelog]

* Update db/primary_migrate/20250106232958_drop_proofing_components_table.rb

Co-authored-by: Zach Margolis <[email protected]>

---------

Co-authored-by: Zach Margolis <[email protected]>
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