Migrate all old direct messages to new conversations schema #9085
Conversation
| notifications_about_direct_statuses.find_each do |notification| | ||
| AccountConversation.add_status(notification.account, notification.target_status) | ||
| end | ||
| end |
There was a problem hiding this comment.
What about statuses being added out-of-order?
There was a problem hiding this comment.
Batch query enforces own order and it's based on id and i think it's ascending.
There was a problem hiding this comment.
Any local toot will still be added before any remote toot, regardless of their actual order, right?
This means that if the last toot of a conversation was a local toot, it won't actually show, and that removing toots from the conversation will lead to strange behaviors.
It also means that for servers running on master or the previous RC, old toots will pop up on top of existing conversations.
There was a problem hiding this comment.
You forget the status_ids are sorted before being saved, so inserting out of order should be fine. Updates will be streamed though but only if you are connected to the direct stream and I think even then it's a minor one-off issue.
There was a problem hiding this comment.
I'm sorry, I can't find where they are sorted.
There was a problem hiding this comment.
| Notification.joins(mention: :status) | ||
| .where(activity_type: 'Mention', statuses: { visibility: :direct }) | ||
| .includes(:account, mention: { status: [:account, mentions: :account] }) | ||
| end |
There was a problem hiding this comment.
What about deleted notifications (of statuses that would still be in the DM column)?
There was a problem hiding this comment.
The alternative is not worth it
There was a problem hiding this comment.
I don't know, we currently have a way to clear the notifications, so people might do that.
There was a problem hiding this comment.
No I mean the alternative is running the NotifyService code which performs a lot of checks that are not optimized for n+1. It's a lot faster to iterate over already existing notifications. Also if you deleted the notifications it might be you don't wanna see those DMs anymore anyway. Overall I think it's the most optimal choice for the migration.
First it creates a conversation record for every local DM's author (so everyone can keep track of DMs they sent). Then it iterates over notifications about DMs to create corresponding conversation records. This allows us to not mess with NotifyService filters on the fly, because if a notification exists, it can be shown.