Don't grant badges to users who don't exist?

Looking at one our badge pages I see

which got me curious - what happened to all those avatars?

Turns out these users don’t exist.

We probably shouldn’t grant badges to users that no longer exist? Or perhaps prevent them from being displayed? Regardless, they are ousting the avatars of the real users - the ones we care about!

10 Likes

Hmmm… something is not right here cause:

https://github.com/discourse/discourse/blob/dc6b547ed89f652b5406489d76140b76cf8e0d1d/app/models/user.rb#L30-L30

The grant is already joining into the user, it has to pull the user id from somewhere.

I wonder if the way this is scoped means that dependent destroy is not working right.

Regardless we can add a weekly cleanup.

1 Like

Given when these badges were granted (at time of writing they were the most recent) I suspect they are granted when the user account doesn’t exist, as opposed to granted when the account exists and then not deleted on account deletion.

Will have to check.

2 Likes

It is odd cause the rows have user ids, if the user is purged there is no where to pull these ids from.

3 Likes

I think the user ids are residing in the incoming links table. Those records are not dependently destroyed, and the sharing badge query only protects against the user_id being null in the incoming_links table:

https://github.com/discourse/discourse/blob/main/lib/badge_queries.rb#L199

The First Share badge should have a similar bug:

https://github.com/discourse/discourse/blob/main/lib/badge_queries.rb#L64

3 Likes

I created a pull request that joins those badge queries to the users table–my first pull request, so hopefully I did everything right!

3 Likes