[WIP] Unify case sensitive topic names

[preetmishra]

This unifies case sensitive topic names using lowercase topic names as an invariant.

Crux

We need some kind of invariant which we can rely upon for lookups and comparisons given that topics can change their casing at any point. Consequently, I propose to use lowercase topics as keys wherever store/index data.

Commits

The current commit structure is temporary. I have fixed one thing per commit to represent how I went about the changes but we would definitely want to squash them (except the first) before merging.

I would greatly appreciate feedback about the proposal and what else should be fixed.

[neiljp]

@preetmishra This seems like a good first step (and the refactoring exposes a potential edge case bug?), but there various points that we need to address, as demonstrated by manual testing. This is not a complete list, but for example:

• If a user sends to case and CASE then two separate unread counts appear in the topic list, and only one appears in the message list if you're in that topic narrow
• I have an existing instance which triggered this where there are unreads in topics with two 'different' names (by case), both with unreads on czo, but your code is not combining them.
• Editing a message topic which matches by case causes it to disappear, but not appear in a new/different topic

Essentially, while fetching messages goes through index_messages, there are other situations we need to consider - anywhere where we compare topics, which is potentially a lot of different places.

Perhaps fundamentally, if we get an update to a topic (eg. edited latest message, or new messge), should we change the topic of every message we have stored? Or just update the rendering?

[neiljp]

narrow_topic vs narrow_topics?

[preetmishra]

The narrow can only have one topic, right?

[preetmishra]

@neiljp Thanks for the review and the pointers! 👍

I have reworked the fundamental approach that I had to now store lowercase topic names for lookups and comparisons (see #733 (comment)). I have also addressed the three issues that you reported.

[preetmishra]

Updated with improved commits, more comments and test amendments (except which are related to muted topics).

[preetmishra]

Updated to resolve conflicts.

[neiljp]

@preetmishra This looks to cover a good number of comparison cases; this was blocked on another PR?

To clarify, this uses:

• lower case in internal structures (that seems clearest)
• "latest" topic names in display? (last-but-one commit references that?)
• the 'real' case in what remaining places? (for display ^)

I think this will be clearer with the now-merged #675 and when the topic list updates with something like #785, as we should be able to test internally more easily.

This looks good, though I've not dug into all the cases so far. Pending further review, this seems reasonable - my concern is whether we might consider locally handling topics with ids, which may simplify this issue - ie. each topic_id in a stream and (stream_id, topic_id) would be unique, and so we can have a 'latest name' (for display) for each id, and the comparisons would all occur at the point where topic names are converted to ids.

The first commit seems like a separate cleanup, is that correct?

[zulipbot]

Heads up @preetmishra, we just merged some commits that conflict with the changes your made in this pull request! You can review this repository's recent commits to see where the conflicts occur. Please rebase your feature branch against the upstream/main branch and resolve your pull request's merge conflicts accordingly.