search

zulip / zulip-flutter

Upcoming Zulip mobile apps for Android and iOS, using Flutter
Apache License 2.0
200 stars 193 forks source link

Statically ensure that `UserTopicVisibilityPolicy.unknown` is not possible in our data models #1074

Open PIG208 opened 1 week ago

PIG208 commented 1 week ago

This was brought up here: https://github.com/zulip/zulip-flutter/pull/1041#discussion_r1853230302.

UserTopicVisibilityPolicy.unknown allows us to deserialize unknown enum values gracefully at the edge.

https://github.com/zulip/zulip-flutter/blob/9e42f2694afffd3a40c7ca4786e8641622513350/lib/model/channel.dart#L216-L222

https://github.com/zulip/zulip-flutter/blob/9e42f2694afffd3a40c7ca4786e8641622513350/lib/model/channel.dart#L348-L350

However, while we manually declare that it wouldn't be possible that we encounter UserTopicVisibilityPolicy.unknown when accessing our internal data store (i.e.: ChannelStore), our type doesn't agree with that, and require the UserTopicVisibilityPolicy.unknown case to be handled for an exhaustive switch. We use assert in such cases, but it is not supposed to be reachable (unless we have a bug).

We should make sure that such an assertion is not needed. This might require removing unknown from UserTopicVisibilityPolicy.

We specifically exclude UserTopicVisibilityPolicy.unknown at the edge. This may or may not apply to other unknown enum values. It would be good to do a sweep of those and decide if we need to follow-up on them as well.

RITWIZSINGH commented 6 days ago

Hey there, actually I am new to open source contribution I want to ask how can i help towards solving this issue ? Can you assign this issue to me?

chrisbobbe commented 4 days ago

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