zulip/zulip-flutter

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

Open

#1,074 opened on Nov 22, 2024

View on GitHub
 (4 comments) (0 reactions) (0 assignees)Dart (456 forks)github user discovery
a-apihelp wanted

Repository metrics

Stars
 (454 stars)
PR merge metrics
 (PR metrics pending)

Description

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. To do this, we'll:

  • Remove unknown from UserTopicVisibilityPolicy.
  • In the places where it was needed, use a nullable type UserTopicVisibilityPolicy? instead.

Out of scope

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.

Contributor guide

Research direction
Remove the `unknown` case from `UserTopicVisibilityPolicy` enum and replace its usages with `UserTopicVisibilityPolicy?`. Ensure all switch statements are updated to handle nullable type.
Tech stack
dartflutter
Domain
mobilebackend
Issue type
Refactor
DifficultyEstimated implementation difficulty for a new contributor, from 1 for very small changes to 5 for expert-level work.
3
Estimated timeA rough time range for an experienced contributor to investigate, implement, test, and prepare a pull request.
Half day
Activity statusHow available the issue appears right now: fresh, active, stale, blocked, or waiting on maintainer input.
Active
ClarityHow clearly the issue explains the expected change, acceptance criteria, and next step.
Clear
Prerequisites
DartFlutterGit
Newbie friendlinessA 1-100 score estimating how approachable this issue is for first-time contributors.
40