Writing a follow-up diff rn to address this!

Oh yeah, this makes sense. @ginsu you might need a quick primer on Promises + async/await, maybe we can talk about this in person

*This doesn't make sense

In the future we can address this by being more specific when defining the relationship between messages. Currently, only two types reference other messages: reaction and sidebar source. Also editing and deleting messages would reference a message. So it looks like a common part is that all of these types reference exactly one message. It means that we can make this relationship more explicit by creating a single field (e.g. targetMessage) that all the messages can have and pass it as a parameter here instead of this function.

An alternative might be to make ReactionMessageInfo consistent with our current approach: in SidebarSourceMessageInfo there's a field sourceMessage which is a message info, but in ReactionMessageInfo we have only targetMessageID which is an id. If we had a full message info, we wouldn't need a function here.

Yeah, that's fair. To make that work we would still need to individually fetch the source message – for SidebarSourceMessageInfo we do that here in createThread, and we pass it into MessageData before we call createMessages.

Consistency is definitely a good reason to do it the same way, but it would've probably been more complicated to refactor the reaction message types here. This ideally would've been something we considered back when we defined those types.

There are a couple of downsides of the current approach. One is that for each notification we need to send a separate request to our db which might have a performance impact. For SIDEBAR_SOURCE we solved this issue by introducing fetchDerivedMessages function and using its result in rawMessageInfoFromServerDBRow. We probably should use the same mechanism here.

I don't think we use fetchDerivedMessages / rawMessageInfoFromServerDBRow in the workflow that creates SIDEBAR_SOURCE... instead, we call fetchMessageInfoByID before calling createMessages (see link above). I could imagine us refactoring this if we ever needed to create multiple SIDEBAR_SOURCE messages, but as it stands we only ever create one SIDEBAR_SOURCE (and one REACTION) at a time, so the performance impact is the same.

Going to land for now, but if refactoring targetMessageID to something like targetMessage or sourceMessage is something we want to do in the future, I can create a task to track it

Ok, that makes sense. Let's keep it for now - I don't think it really makes sense to refactor it soon.