Skip to content

Don't use stale document capture user ids for effective_user#10293

Merged
matthinz merged 2 commits intomainfrom
matthinz/12793-detect-stale-effective-user
Mar 22, 2024
Merged

Don't use stale document capture user ids for effective_user#10293
matthinz merged 2 commits intomainfrom
matthinz/12793-detect-stale-effective-user

Conversation

@matthinz
Copy link
Contributor

🎫 Ticket

Link to the relevant ticket:
LG-12793

🛠 Summary of changes

If the user deletes their account, it is possible that the user id stored in the session for document capture will not be valid. This results in analytics events being improperly attributed to anonymous-uuid.

If the user deletes their account, it is possible that the user id stored in the session for document capture will not be valid. This results in analytics events being improperly attributed to `anonymous-uuid`.

changelog: Bug Fixes, Identity verification, Fix log attribute issue associated with hybrid handoff.

Co-authored-by: Doug Price <douglas.price@gsa.gov>
@matthinz matthinz requested a review from a team March 22, 2024 18:25
Copy link
Contributor

@theabrad theabrad left a comment

Choose a reason for hiding this comment

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

LGTM

Co-authored-by: Zach Margolis <zachmargolis@users.noreply.github.com>
@matthinz matthinz merged commit 15fa2dc into main Mar 22, 2024
@matthinz matthinz deleted the matthinz/12793-detect-stale-effective-user branch March 22, 2024 21:41
user = User.find_by(id: effective_user_id) if effective_user_id

if user.nil?
session.delete(:doc_capture_user_id)
Copy link
Contributor

Choose a reason for hiding this comment

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

I find it a little strange we delete this after-the-fact, and as a side-effect of a getter method.

Should we delete this at the time that it becomes invalid? As in your pull request comment: "If the user deletes their account".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, we considered that at first. But I think keeping it here means that things are slightly less "spread out" and easier to keep track of. Hopefully it will be academic because I am trying to get a ticket to remove effective_user refined and on Team Ada's plate.

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.

4 participants