reactions: Convert module to typescript

[evykassirer]

Split this into several commits since there was a lot going on.

reactions: Calculate vote_text before creating clean reaction. commit feels a bit messy but wasn't sure if there was a nicer way to do that. Would love feedback there

[andersk]

Okay looking at this more closely, it seems to me that emoji_code really is always a string, and the string | number put in emoji.ts by #24875 is just wrong, modulo a handful of Node test cases that are also wrong.

It’s converted here:

zulip/zerver/lib/emoji.py

Lines 74 to 76 in 9fde83c

	if realm_emoji is not None:
	emoji_code = str(realm_emoji["id"])
	return EmojiData(emoji_code=emoji_code, reaction_type=Reaction.REALM_EMOJI)

zulip/zerver/models/realm_emoji.py

Lines 88 to 95 in 9fde83c

	emoji_dict: EmojiInfo = dict(
	id=str(realm_emoji.id),
	name=realm_emoji.name,
	source_url=emoji_url,
	deactivated=realm_emoji.deactivated,
	author_id=author_id,
	still_url=None,
	)

zulip/zerver/lib/import_realm.py

Lines 548 to 549 in 9fde83c

	# Now update emoji_code to the new id.
	row["emoji_code"] = str(new_realm_emoji_id)

[timabbott]

I haven't tried to analyze, but I want to be very cautious about changes in when we do set_clean_reactions -- we had a whole set of bugs around unanticipated consequences of changing which resulted in my needing to do the refactoring ending in #28521 (comment).

If I recall correctly, the short of the issues there were things like that the previous code could do quadratic work (by computing the vote text summing up everyone's reactions for each reaction in the main loop over the reactions in a given message). Can you familiarize yourself with that and comment on the safety of these changes? It might be fine now, but I just want to make sure we're being careful here.

[evykassirer]

@timabbott the PR you're linking to seems like it's only backend changes, and I'm having trouble seeing how it's related. Did you link to the wrong thing maybe?

[evykassirer]

Pulled the less caution-needing prep commits to #28734

[evykassirer]

I haven't seen what the issue was before yet, so I might be missing something, but I added some comments about why I don't think these changes make things quadratic. Let me know if you want further clarification/explanation!

[evykassirer]

@timabbott / @andersk ready for you to take a look :)

[evykassirer]

@timabbott just making sure you saw this was ready

[timabbott]

This looks like a correct conversion, so I think this is good to merge; going to do that without asking for revisions for the above, because it was a slog to read through and none of the issues above have functional impact. But I would appreciate a follow-up PR this week addressing them.

Thanks @evykassirer! This definitely wasn't an easy one.

[evykassirer]

Made a followup PR: #29199