404 on earned badge event when badge is disabled

Yes, I agree this is not ideal.

Our notifications table stores information about the “reason” in a text column containing json. This happens because there are a wide range of reasons you could be getting a notification and creating a big pile of columns would widen the table a lot and decrease flexibility around notifications.

We keep topic and post number, but there is a range of other types of ids (like group mentions, invites, badges and so on)

The “lookup my notifications” query needs to stay super fast, I did not want to force joins into 5 or so tables just to determine if the source is legit. (we do ensure post notifications are legit though - deleting a topic hides the related notifications)

Historically, people don’t go about disabling badges beyond the first few weeks of running a forum. So in practice this is a very minor issue.

That said there are 2 options you have here:

  • In practice nobody ever looks beyond the first 20 or so notifications, this issue will drown in a few days of active use.

  • If it is really bugging you you can clean up the old notifications from the table:

cd /var/discourse
./launcher enter app
rails c
% Notification.where('notification_type=12 AND data like \'%"badge_id":5,%\'').destroy_all

(5 is the badge id for welcome, destroy_all calls object model so it will live refresh all browsers)

On our side there are a couple of things I would like to clean up.

  1. Our “client side” 404 should look exactly like the server side 404, reloading a 404 page should always result in the same 404

  2. We should optionally be allowed to send in a bit more information into the 404 pages. As it stands we do a raise Discourse::NotFound we should allow raise Discourse::NotFound, "Badge is no longer enabled" and float that info into the 404.

7 « J'aime »