Skip to content

Case-insensitive topics in unreads data #980

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
gnprice opened this issue Oct 3, 2024 · 8 comments
Open

Case-insensitive topics in unreads data #980

gnprice opened this issue Oct 3, 2024 · 8 comments
Assignees
@chrisbobbe
Labels
a-model Implementing our data model (PerAccountStore, etc.) help wanted
Milestone
M5b: Launch goals

Comments

@gnprice
Copy link
Member

gnprice commented Oct 3, 2024 •
edited by chrisbobbe
Loading

Topic names in Zulip are case-insensitive.

Currently, however, our Unreads data structure keeps the data for each channel/stream as just a Map keyed by topic name. It should instead treat messages where the topic names differ only in case (and that have the same stream ID) as belonging to the same topic.

There's actually a server bug where this doesn't properly happen in the unreads data we get in the initial snapshot:
https://chat.zulip.org/#narrow/stream/48-mobile/topic/Case.20sensitivity.20in.20topics/near/1954334
But we should also handle this properly in events, and in looking up data. For example handleMessageEvent should put the new message into an existing topic if it differs only in case.

If fixing this issue for events has a side effect of working around the server-side issue, then that's nice. But if it doesn't, we don't need to add a workaround. The issue seems fairly uncommon, so it's OK if the symptoms continue when using a server that still has the bug.

Related issues
@gnprice gnprice added the a-model Implementing our data model (PerAccountStore, etc.) label Oct 3, 2024
@gnprice gnprice added this to the Launch milestone Oct 3, 2024
@Adityakk9031
Copy link

i am going to work on this issue

@chrisbobbe
Copy link
Collaborator

Please take a look at the Zulip project's guide to getting involved with the code:

https://zulip.readthedocs.io/en/latest/contributing/contributing.html#your-first-codebase-contribution
https://zulip.readthedocs.io/en/latest/contributing/contributing.html#picking-an-issue-to-work-on

@Adityakk9031
Copy link

ok

Adityakk9031 added a commit to Adityakk9031/zulip-flutter that referenced this issue Nov 24, 2024
@Adityakk9031
zulip#980 update the message.dart
9c30b01
This was referenced Dec 26, 2024
Closed
Closed
apoorvapendse added a commit to apoorvapendse/zulip-flutter that referenced this issue Dec 27, 2024
@apoorvapendse
unreads: Use case i9e lookups, preserving case.
31879bb 
This commit makes the topics having different
casings for the same topic be stored under
the same QueueList by making the lookups
lowercase while preserving one of the
variants of the used topic case.

Fixes zulip#980.
apoorvapendse added a commit to apoorvapendse/zulip-flutter that referenced this issue Dec 27, 2024
@apoorvapendse
unreads: Use case i9e lookups, preserving case.
475f35b 
This commit makes the topics having different
casings for the same topic be stored under
the same QueueList by making the lookups
lowercase while preserving one of the
variants of the used topic case.

Fixes zulip#980.
@apoorvapendse apoorvapendse mentioned this issue Dec 27, 2024
Closed
@chrisbobbe chrisbobbe mentioned this issue Jan 11, 2025
Merged
@gnprice gnprice modified the milestones: M5: Launch, M5-a: Server 10 Jan 14, 2025
apoorvapendse added a commit to apoorvapendse/zulip-flutter that referenced this issue Feb 2, 2025
@apoorvapendse
unreads: Fix topic case-sensitivity.
cc82251 
Maps canoncalized topic names to
a variant of the actual topic name.

Fixes zulip#980.
apoorvapendse added a commit to apoorvapendse/zulip-flutter that referenced this issue Feb 2, 2025
@apoorvapendse
unreads: Fix topic case-sensitivity.
6d89782 
Maps canoncalized topic names to
a variant of the actual topic name.

Fixes zulip#980.
apoorvapendse added a commit to apoorvapendse/zulip-flutter that referenced this issue Feb 2, 2025
@apoorvapendse
unreads: Fix topic case-sensitivity.
3f38f48 
Maps canoncalized topic names to
a variant of the actual topic name.

Fixes zulip#980.
apoorvapendse added a commit to apoorvapendse/zulip-flutter that referenced this issue Feb 2, 2025
@apoorvapendse
unreads: Fix topic case-sensitivity.
12ab0a9 
Maps canoncalized topic names to
a variant of the actual topic name.

Fixes zulip#980.
apoorvapendse added a commit to apoorvapendse/zulip-flutter that referenced this issue Feb 2, 2025
@apoorvapendse
unreads: Fix topic case-sensitivity.
711d659 
Maps canoncalized topic names to
a variant of the actual topic name.

Fixes zulip#980.
apoorvapendse added a commit to apoorvapendse/zulip-flutter that referenced this issue Feb 6, 2025
@apoorvapendse
unreads: Fix topic case-sensitivity.
a084caf 
Maps canoncalized topic names to
a variant of the actual topic name.

This is achieved by introducing a TopicMap class
similar to FoldDict in web.

See https://chat.zulip.org/#narrow/channel/516-mobile-dev-help/topic/Expected.20behaviour.20for.20case.20insensitivity.20for.20unreads.20.23F980/near/2076806

Fixes zulip#980.
apoorvapendse added a commit to apoorvapendse/zulip-flutter that referenced this issue Feb 8, 2025
@apoorvapendse
unreads: Fix topic case-sensitivity.
8d16408 
Maps canoncalized topic names to
a variant of the actual topic name.

This is achieved by introducing a TopicMap class
similar to FoldDict in web.

See https://chat.zulip.org/#narrow/channel/516-mobile-dev-help/topic/Expected.20behaviour.20for.20case.20insensitivity.20for.20unreads.20.23F980/near/2076806

Fixes zulip#980.
apoorvapendse added a commit to apoorvapendse/zulip-flutter that referenced this issue Feb 8, 2025
@apoorvapendse
unreads: Fix topic case-sensitivity.
07f29ad 
Maps canoncalized topic names to
a variant of the actual topic name.

This is achieved by introducing a TopicMap class
similar to FoldDict in web.

See https://chat.zulip.org/#narrow/channel/516-mobile-dev-help/topic/Expected.20behaviour.20for.20case.20insensitivity.20for.20unreads.20.23F980/near/2076806

Fixes zulip#980.
apoorvapendse added a commit to apoorvapendse/zulip-flutter that referenced this issue Feb 8, 2025
@apoorvapendse
unreads: Fix topic case-sensitivity.
8eafd1d 
Maps canoncalized topic names to
a variant of the actual topic name.

This is achieved by introducing a TopicMap class
similar to FoldDict in web.

See https://chat.zulip.org/#narrow/channel/516-mobile-dev-help/topic/Expected.20behaviour.20for.20case.20insensitivity.20for.20unreads.20.23F980/near/2076806

Fixes zulip#980.
apoorvapendse added a commit to apoorvapendse/zulip-flutter that referenced this issue Feb 14, 2025
@apoorvapendse
unreads: Fix topic case-sensitivity.
74ed495 
Maps canoncalized topic names to
a variant of the actual topic name.

This is achieved by introducing a TopicMap class
similar to FoldDict in web.

I referred to the FoldDict code in
zulip/zulip as @gnprice suggested.
Thanks to the folks who wrote it!

See https://chat.zulip.org/#narrow/channel/516-mobile-dev-help/topic/Expected.20behaviour.20for.20case.20insensitivity.20for.20unreads.20.23F980/near/2076806

Fixes zulip#980.
apoorvapendse added a commit to apoorvapendse/zulip-flutter that referenced this issue Feb 14, 2025
@apoorvapendse
unreads: Fix topic case-sensitivity.
6f6c85d 
Maps canoncalized topic names to
a variant of the actual topic name.

This is achieved by introducing a TopicMap class
similar to FoldDict in web.

I referred to the FoldDict code in
zulip/zulip as @gnprice suggested.
Thanks to the folks who wrote it!

See https://chat.zulip.org/#narrow/channel/516-mobile-dev-help/topic/Expected.20behaviour.20for.20case.20insensitivity.20for.20unreads.20.23F980/near/2076806

Fixes zulip#980.
@AshutoshKhadse23
Copy link

AshutoshKhadse23 commented Apr 3, 2025 •
edited
Loading

Hello @gnprice ,
I know @chrisbobbe is working on it but could you once take a look at my implement, if yes should I create a PR for that?
And in my implementation the changes are only in unread.dart and unread_check.dart

Image Image

@gnprice
Copy link
Member Author

gnprice commented Apr 3, 2025

Well, if it only touches those two files then it definitely isn't ready for review 🙂 — as you've heard before, a PR needs tests.

@AshutoshKhadse23

This comment has been minimized.

Sign in to view
@gnprice
Copy link
Member Author

gnprice commented Apr 3, 2025

If you'd like feedback on code, a PR is the most effective way to present it.

Do take a look also at the last comment on the previous PR #1214 — some of that feedback also applies to your draft above.

@PIG208
Copy link
Member

PIG208 commented Apr 25, 2025

This issue also applies to ChannelStore, where topicVisibility is a map that uses TopicName as a key. That data structure should also be case-insensitive.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@chrisbobbe chrisbobbe

Labels
a-model Implementing our data model (PerAccountStore, etc.) help wanted
Projects
Flutter app
Status: No status
Milestone
M5b: Launch goals
Development

Successfully merging a pull request may close this issue.

Fix unread topic case sensitivity. apoorvapendse/zulip-flutter
5 participants
@gnprice @chrisbobbe @PIG208 @AshutoshKhadse23 @Adityakk9031