Skip to content

Include deleted users in daily registrations report#7731

Merged
zachmargolis merged 1 commit intomainfrom
margolis-daily-report-count-deleted
Feb 3, 2023
Merged

Include deleted users in daily registrations report#7731
zachmargolis merged 1 commit intomainfrom
margolis-daily-report-count-deleted

Conversation

@zachmargolis
Copy link
Contributor

Currently, the reports do the equivalent of a User.count to detect how many users were created. However, users can be deleted so that count could be inaccurate over time.

This PR attempts to use the deleted_users table to create a more stable number over time, by including deleted users by the date they were originally created on.

Example: I create an account Monday, but delete it on Tuesday (assuming no other users were created, etc)

  • Before: Monday's report would include me but Tuesday would have subtracted one user
  • Now: Both Monday's and Tuesday's reports would count me and the count should be consistent moving forward

Open Question:

  • Do we want to change this? Or leave it? This changes the way we calculate total registered users, so if somebody pulled the "total registered" users query from the handbook, the numbers would not match (I could also go back and patch that handbook page to match this)
  • Do we want to count deleted users by the day they were deleted or created? (I think counting by the day they're deleted is clearer for reporting and more stable over time, so that's why I went that way)

Notes:

  • registration_logs table which we use for "fully registered users" has a cascading delete meaning that number won't be stable over time as users get deleted

changelog: Internal, Reporting, Include deleted users in reports on users created
@zachmargolis zachmargolis requested a review from a team January 31, 2023 01:59
FROM users
WHERE users.created_at <= %{finish}

UNION ALL
Copy link
Contributor Author

Choose a reason for hiding this comment

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

UNION ALL is probably not efficient, another approach would be to do the addition/joining in Ruby at the end, I could go either way. It's also a reporting query so I'm not overly concerned with efficiency? 🤷

Copy link
Contributor

@mitchellhenke mitchellhenke 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! 👍🏼

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