flow: Use a Flow enum for the first time! (CreateWebPublicStreamPolicy)

This is nice and simple, particularly in the definition.

We remove the `default` case in the two `switch`es on a
CreateWebPublicStreamPolicy value, and in particular that means
removing the `ensureUnreachable`s there. But its job will still be
done: Flow still tracks whether the cases are exhaustive and will
complain if not.

Author: chrisbobbe

src/__tests__/permissionSelectors-test.js

@@ -9,7 +9,6 @@ import {
   CreatePublicOrPrivateStreamPolicy,
   type CreatePublicOrPrivateStreamPolicyT,
   CreateWebPublicStreamPolicy,
-  type CreateWebPublicStreamPolicyT,
 } from '../api/permissionsTypes';
 import {
   getHasUserPassedWaitingPeriod,
@@ -301,7 +300,7 @@ describe('getCanCreateWebPublicStreams', () => {
       role,
       expected,
     }: {
-      policy: CreateWebPublicStreamPolicyT,
+      policy: CreateWebPublicStreamPolicy,
       role: RoleT,
       expected: boolean,
     }) => {

src/api/initialDataTypes.js

@@ -18,7 +18,7 @@ import type {
 } from './apiTypes';
 import type {
   CreatePublicOrPrivateStreamPolicyT,
-  CreateWebPublicStreamPolicyT,
+  CreateWebPublicStreamPolicy,
 } from './permissionsTypes';
 
 /*
@@ -176,7 +176,7 @@ export type InitialDataRealm = $ReadOnly<{|
 
   // TODO(server-5.0): Added in feat. 103; when absent, treat as
   //   CreateWebPublicStreamPolicy.Nobody.
-  realm_create_web_public_stream_policy?: CreateWebPublicStreamPolicyT,
+  realm_create_web_public_stream_policy?: CreateWebPublicStreamPolicy,
 
   realm_default_code_block_language: string | null,
 

src/api/permissionsTypes.js

@@ -47,7 +47,7 @@ export const RoleValues: $ReadOnlyArray<RoleT> = objectValues(Role);
  * (These policies could be set differently, but their types are the same,
  * so it's convenient to have just one definition.)
  *
- * For the policy for web-public streams, see CreateWebPublicStreamPolicyT.
+ * For the policy for web-public streams, see CreateWebPublicStreamPolicy.
  */
 // eslint-disable-next-line ft-flow/type-id-match
 export type CreatePublicOrPrivateStreamPolicyT = 1 | 2 | 3 | 4;
@@ -89,33 +89,9 @@ export const CreatePublicOrPrivateStreamPolicyValues: $ReadOnlyArray<CreatePubli
  * organization. Ignore if the server hasn't opted into the concept of
  * web-public streams.
  */
-// Ideally both the type and enum would have the simple name; but a type
-// and value sharing a name is one nice TS feature that Flow lacks.
-// Or we could leapfrog TS and make this a Flow enum:
-//   https://flow.org/en/docs/enums/
-// (TS has its own enums, but they are a mess.)
-// eslint-disable-next-line ft-flow/type-id-match
-export type CreateWebPublicStreamPolicyT = 2 | 4 | 6 | 7;
-
-/**
- * An enum of all valid values for `CreateWebPublicStreamPolicyT`.
- *
- * See CreateWebPublicStreamPolicyValues for the list of values.
- */
-export const CreateWebPublicStreamPolicy = {
-  AdminOrAbove: (2: 2),
-  ModeratorOrAbove: (4: 4),
-  Nobody: (6: 6),
-  OwnerOnly: (7: 7),
-};
-
-// Check that the enum indeed has all and exactly the values of the type.
-typesEquivalent<CreateWebPublicStreamPolicyT, $Values<typeof CreateWebPublicStreamPolicy>>();
-
-/**
- * A list of all valid values for `CreateWebPublicStreamPolicyT`.
- *
- * See CreateWebPublicStreamPolicy for an enum to refer to these by meaningful names.
- */
-export const CreateWebPublicStreamPolicyValues: $ReadOnlyArray<CreateWebPublicStreamPolicyT> =
-  objectValues(CreateWebPublicStreamPolicy);
+export enum CreateWebPublicStreamPolicy {
+  AdminOrAbove = 2,
+  ModeratorOrAbove = 4,
+  Nobody = 6,
+  OwnerOnly = 7,
+}

src/permissionSelectors.js

@@ -1,7 +1,7 @@
 /* @flow strict-local */
 import type { PerAccountState, UserId } from './types';
 import { ensureUnreachable } from './types';
-import { Role, type RoleT } from './api/permissionsTypes';
+import { Role, type RoleT, CreateWebPublicStreamPolicy } from './api/permissionsTypes';
 import { getRealm, getOwnUser, getUserForId } from './selectors';
 
 const { Guest, Member, Moderator, Admin, Owner } = Role;
@@ -164,25 +164,13 @@ export function getCanCreateWebPublicStreams(state: PerAccountState): boolean {
   }
 
   switch (createWebPublicStreamPolicy) {
-    // FlowIssue: sad that we end up having to write numeric literals here :-/
-    //   But the most important thing to get from the type-checker here is
-    //   that the ensureUnreachable works -- that ensures that when we add a
-    //   new possible value, we'll add a case for it here.  Couldn't find a
-    //   cleaner way to write this that still accomplished that. Discussion:
-    //     https://github.com/zulip/zulip-mobile/pull/5384#discussion_r875147220
-    case 6: // CreateWebPublicStreamPolicy.Nobody
+    case CreateWebPublicStreamPolicy.Nobody:
       return false;
-    case 7: // CreateWebPublicStreamPolicy.OwnerOnly
+    case CreateWebPublicStreamPolicy.OwnerOnly:
       return roleIsAtLeast(role, Owner);
-    case 2: // CreateWebPublicStreamPolicy.AdminOrAbove
+    case CreateWebPublicStreamPolicy.AdminOrAbove:
       return roleIsAtLeast(role, Admin);
-    case 4: // CreateWebPublicStreamPolicy.ModeratorOrAbove
+    case CreateWebPublicStreamPolicy.ModeratorOrAbove:
       return roleIsAtLeast(role, Moderator);
-    default: {
-      ensureUnreachable(createWebPublicStreamPolicy);
-
-      // (Unreachable as long as the cases are exhaustive.)
-      return false;
-    }
   }
 }

src/realm/__tests__/realmReducer-test.js

@@ -20,6 +20,15 @@ import { CustomProfileFieldType } from '../../api/modelTypes';
 import { EventTypes } from '../../api/eventTypes';
 import * as eg from '../../__tests__/lib/exampleData';
 
+const labelFromValue = value =>
+  /* $FlowFixMe[incompatible-use] - CreateWebPublicStreamPolicy is a
+     Flow enum. Values are numbers, so they do have .toString…but
+     Flow reasonably hides that detail from consumers. For
+     CreateWebPublicStreamPolicy, it would actually be better to use
+     CreateWebPublicStreamPolicy.getName(initialStateValue), if we
+     find a nice way to write that with type checking. */
+  value?.toString() ?? '[nullish]';
+
 describe('realmReducer', () => {
   describe('REGISTER_COMPLETE', () => {
     test('updates as appropriate on a boring but representative REGISTER_COMPLETE', () => {
@@ -289,8 +298,7 @@ describe('realmReducer', () => {
           eventPropertyName: E,
         ): ((RealmState[S], UserSettings[E]) => void) =>
         (initialStateValue, eventValue) => {
-          /* prettier-ignore */ // (wants to wrap the name weirdly)
-          test(`${initialStateValue?.toString() ?? '[nullish]'} → ${eventValue?.toString() ?? '[nullish]'}`, () => {
+          test(`${labelFromValue(initialStateValue)} → ${labelFromValue(eventValue)}`, () => {
             const initialState = { ...eg.plusReduxState.realm };
             // $FlowFixMe[prop-missing]
             // $FlowFixMe[class-object-subtyping]
@@ -331,8 +339,7 @@ describe('realmReducer', () => {
           eventPropertyName: E,
         ): ((RealmState[S], RealmDataForUpdate[E]) => void) =>
         (initialStateValue, eventValue) => {
-          /* prettier-ignore */ // (wants to wrap the name weirdly)
-          test(`${initialStateValue?.toString() ?? '[nullish]'} → ${eventValue?.toString() ?? '[nullish]'}`, () => {
+          test(`${labelFromValue(initialStateValue)} → ${labelFromValue(eventValue)}`, () => {
             const initialState = { ...eg.plusReduxState.realm };
             // $FlowFixMe[prop-missing]
             // $FlowFixMe[class-object-subtyping]

src/reduxTypes.js

@@ -26,7 +26,7 @@ import type {
   UserId,
   UserPresence,
   CreatePublicOrPrivateStreamPolicyT,
-  CreateWebPublicStreamPolicyT,
+  CreateWebPublicStreamPolicy,
 } from './api/apiTypes';
 import type {
   PerAccountSessionState,
@@ -302,7 +302,7 @@ export type RealmState = {|
   +createPublicStreamPolicy: CreatePublicOrPrivateStreamPolicyT,
   +createPrivateStreamPolicy: CreatePublicOrPrivateStreamPolicyT,
   +webPublicStreamsEnabled: boolean,
-  +createWebPublicStreamPolicy: CreateWebPublicStreamPolicyT,
+  +createWebPublicStreamPolicy: CreateWebPublicStreamPolicy,
   +enableSpectatorAccess: boolean,
   +waitingPeriodThreshold: number,
   +allowEditHistory: boolean,

src/streams/EditStreamCard.js

@@ -7,7 +7,11 @@ import type { Privacy } from './streamsActions';
 import { ensureUnreachable } from '../types';
 import { useSelector } from '../react-redux';
 import { getRealm, getRealmUrl } from '../selectors';
-import { Role } from '../api/permissionsTypes';
+import {
+  Role,
+  type CreatePublicOrPrivateStreamPolicyT,
+  CreateWebPublicStreamPolicy,
+} from '../api/permissionsTypes';
 import {
   getCanCreatePublicStreams,
   getCanCreatePrivateStreams,
@@ -22,10 +26,6 @@ import ZulipTextIntl from '../common/ZulipTextIntl';
 import ZulipButton from '../common/ZulipButton';
 import styles from '../styles';
 import { TranslationContext } from '../boot/TranslationProvider';
-import type {
-  CreatePublicOrPrivateStreamPolicyT,
-  CreateWebPublicStreamPolicyT,
-} from '../api/permissionsTypes';
 import type { LocalizableText } from '../types';
 import { showConfirmationDialog } from '../utils/info';
 
@@ -64,31 +64,20 @@ type PropsCreateStream = $ReadOnly<{|
 type Props = $ReadOnly<PropsEditStream | PropsCreateStream>;
 
 function explainCreateWebPublicStreamPolicy(
-  policy: CreateWebPublicStreamPolicyT,
+  policy: CreateWebPublicStreamPolicy,
   realmName: string,
 ): LocalizableText {
   return {
     text: (() => {
       switch (policy) {
-        // FlowIssue: sad that we end up having to write numeric literals here :-/
-        //   But the most important thing to get from the type-checker here is
-        //   that the ensureUnreachable works -- that ensures that when we add a
-        //   new possible value, we'll add a case for it here.  Couldn't find a
-        //   cleaner way to write this that still accomplished that. Discussion:
-        //     https://github.com/zulip/zulip-mobile/pull/5384#discussion_r875147220
-        case 6: // CreateWebPublicStreamPolicy.Nobody
+        case CreateWebPublicStreamPolicy.Nobody:
           return '{realmName} does not allow anybody to make web-public streams.';
-        case 7: // CreateWebPublicStreamPolicy.OwnerOnly
+        case CreateWebPublicStreamPolicy.OwnerOnly:
           return '{realmName} only allows organization owners to make web-public streams.';
-        case 2: // CreateWebPublicStreamPolicy.AdminOrAbove
+        case CreateWebPublicStreamPolicy.AdminOrAbove:
           return '{realmName} only allows organization administrators or owners to make web-public streams.';
-        case 4: // CreateWebPublicStreamPolicy.ModeratorOrAbove
+        case CreateWebPublicStreamPolicy.ModeratorOrAbove:
           return '{realmName} only allows organization moderators, administrators, or owners to make web-public streams.';
-        default: {
-          ensureUnreachable(policy);
-          // (Unreachable as long as the cases are exhaustive.)
-          return '';
-        }
       }
     })(),
     values: { realmName },