FEATURE: High priority bookmark reminder notifications (PR #9290)

GitHub

There is an index change we need as well to account for this:

https://github.com/discourse/discourse/blob/40b6e278a08d58ffd9525c52318daac397465b37/app/models/notification.rb#L288-L288

https://github.com/discourse/discourse/blob/40b6e278a08d58ffd9525c52318daac397465b37/app/models/notification.rb#L291-L291

Without this the whole thing will be super slow on many sites

@SamSaffron good catch I did not see that index

integer is huge… instead I would say since this is binary … boolean is enough for now, maybe tinyint or byte.

do we need the class .unread-private-messages ?

where high_priority should do the trick, I think the capital true is a bit too verbose.

hmmm why are we adding 2 extra indexes beyond what we had to start with? every index has a pretty high disk cost on this table

hmmm I would still keep it to one statement per execute, that way if it blows up it will be easier to find out where it exploded

You have “if not exists” here, but don’t have it above in the alter table … I am guessing you should be 100% re-runnable is that is the intent here.

I don’t think this will work cause it will still have a DDL transaction, just chuck a not allowed on down?

OK pending any overnight reviews, let’s merge it first thing tomorrow morning! Then we have a full day to watch it!

This pull request has been mentioned on Discourse Meta. There might be relevant details there: