api types: Use Flow enum for Role

Author: chrisbobbe

src/__tests__/permissionSelectors-test.js

@@ -4,8 +4,6 @@ import invariant from 'invariant';
 import { getOwnUser, tryGetActiveAccountState, getRealm } from '../selectors';
 import {
   Role,
-  RoleValues,
-  type RoleT,
   CreatePublicOrPrivateStreamPolicy,
   type CreatePublicOrPrivateStreamPolicyT,
   CreateWebPublicStreamPolicy,
@@ -21,18 +19,20 @@ import rootReducer from '../boot/reducers';
 import { EVENT } from '../actionConstants';
 import * as eg from './lib/exampleData';
 import { EventTypes } from '../api/eventTypes';
-import { objectEntries } from '../flowPonyfill';
 
 describe('roleIsAtLeast', () => {
   const { Owner, Admin, Moderator, Member, Guest } = Role;
 
+  const roleValues: $ReadOnlyArray<Role> = Array.from(Role.members());
+
   // Keep this current with all possible roles, from least to most privilege.
   const kRolesAscending = [Guest, Member, Moderator, Admin, Owner];
-  expect(RoleValues).toIncludeSameMembers(kRolesAscending);
-  expect(RoleValues).toBeArrayOfSize(kRolesAscending.length);
+  expect(roleValues).toIncludeSameMembers(kRolesAscending);
+  expect(roleValues).toBeArrayOfSize(kRolesAscending.length);
 
-  objectEntries(Role).forEach(([thresholdRoleName, thresholdRole]) => {
-    objectEntries(Role).forEach(([thisRoleName, thisRole]) => {
+  const roleEntries = roleValues.map(role => [Role.getName(role), role]);
+  roleEntries.forEach(([thresholdRoleName, thresholdRole]) => {
+    roleEntries.forEach(([thisRoleName, thisRole]) => {
       const expected = kRolesAscending.indexOf(thisRole) >= kRolesAscending.indexOf(thresholdRole);
 
       const testName = expected
@@ -86,7 +86,7 @@ describe('getCanCreatePublicStreams', () => {
       expected,
     }: {
       policy: CreatePublicOrPrivateStreamPolicyT,
-      role: RoleT,
+      role: Role,
       waitingPeriodPassed: boolean | void,
       expected: boolean,
     }) => {
@@ -166,7 +166,7 @@ describe('getCanCreatePrivateStreams', () => {
       expected,
     }: {
       policy: CreatePublicOrPrivateStreamPolicyT,
-      role: RoleT,
+      role: Role,
       waitingPeriodPassed: boolean | void,
       expected: boolean,
     }) => {
@@ -301,7 +301,7 @@ describe('getCanCreateWebPublicStreams', () => {
       expected,
     }: {
       policy: CreateWebPublicStreamPolicy,
-      role: RoleT,
+      role: Role,
       expected: boolean,
     }) => {
       const globalState = [

src/action-sheets/index.js

@@ -40,7 +40,7 @@ import { getUnreadCountForTopic } from '../unread/unreadModel';
 import getIsNotificationEnabled from '../streams/getIsNotificationEnabled';
 import { getStreamTopicUrl, getStreamUrl } from '../utils/internalLinks';
 import { reactionTypeFromEmojiType } from '../emoji/data';
-import { Role, type RoleT } from '../api/permissionsTypes';
+import { Role } from '../api/permissionsTypes';
 import { roleIsAtLeast } from '../permissionSelectors';
 import { kNotificationBotEmail } from '../api/constants';
 import type { AppNavigationMethods } from '../nav/AppNavigator';
@@ -616,7 +616,7 @@ export const constructStreamActionButtons = (args: {|
 export const constructTopicActionButtons = (args: {|
   backgroundData: $ReadOnly<{
     mute: MuteState,
-    ownUserRole: RoleT,
+    ownUserRole: Role,
     subscriptions: Map<number, Subscription>,
     unread: UnreadState,
     ...
@@ -853,7 +853,7 @@ export const showTopicActionSheet = (args: {|
     subscriptions: Map<number, Subscription>,
     unread: UnreadState,
     ownUser: User,
-    ownUserRole: RoleT,
+    ownUserRole: Role,
     zulipFeatureLevel: number,
     ...
   }>,

src/api/modelTypes.js

@@ -8,7 +8,7 @@ import { typesEquivalent } from '../generics';
 import { objectValues } from '../flowPonyfill';
 import type { AvatarURL } from '../utils/avatar';
 import type { UserId } from './idTypes';
-import type { RoleT } from './permissionsTypes';
+import type { Role } from './permissionsTypes';
 
 export type * from './idTypes';
 
@@ -312,7 +312,7 @@ export type User = {|
   +bot_owner?: string,
 
   // TODO(server-4.0): New in FL 59
-  +role?: RoleT,
+  +role?: Role,
 
   // The ? is for future-proofing. Greg explains in 2020-02, at
   // https://github.com/zulip/zulip-mobile/pull/3789#discussion_r378554698 ,
@@ -400,7 +400,7 @@ export type CrossRealmBot = {|
   // +bot_owner?: string,
 
   // TODO(server-4.0): New in FL 59
-  +role?: RoleT,
+  +role?: Role,
 
   // The ? is for future-proofing.  For bots it's always '':
   //   https://github.com/zulip/zulip-mobile/pull/3789#issuecomment-581218576

src/api/permissionsTypes.js

@@ -5,40 +5,17 @@ import { objectValues } from '../flowPonyfill';
 
 /**
  * Organization-level role of a user.
- */
-// 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 RoleT = 100 | 200 | 300 | 400 | 600;
-
-/**
- * An enum of all valid values for `RoleT`.
  *
  * Described in the server API doc, e.g., at `.role` in `realm_users` at
  *   https://zulip.com/api/register-queue
- *
- * See RoleValues for the list of values.
  */
-export const Role = {
-  Owner: (100: 100),
-  Admin: (200: 200),
-  Moderator: (300: 300),
-  Member: (400: 400),
-  Guest: (600: 600),
-};
-
-// Check that the enum indeed has all and exactly the values of the type.
-typesEquivalent<RoleT, $Values<typeof Role>>();
-
-/**
- * A list of all valid values for `RoleT`.
- *
- * See Role for an enum to refer to these by meaningful names.
- */
-export const RoleValues: $ReadOnlyArray<RoleT> = objectValues(Role);
+export enum Role {
+  Owner = 100,
+  Admin = 200,
+  Moderator = 300,
+  Member = 400,
+  Guest = 600,
+}
 
 /**
  * This organization's policy for which users can create public streams, or

src/permissionSelectors.js

@@ -1,7 +1,7 @@
 /* @flow strict-local */
 import type { PerAccountState, UserId } from './types';
 import { ensureUnreachable } from './types';
-import { Role, type RoleT, CreateWebPublicStreamPolicy } from './api/permissionsTypes';
+import { Role, CreateWebPublicStreamPolicy } from './api/permissionsTypes';
 import { getRealm, getOwnUser, getUserForId } from './selectors';
 
 const { Guest, Member, Moderator, Admin, Owner } = Role;
@@ -13,7 +13,7 @@ const { Guest, Member, Moderator, Admin, Owner } = Role;
  * when the role changes while we're polling an event queue.
  */
 // TODO(server-4.0): Probably just delete this and use User.role directly?
-export function getOwnUserRole(state: PerAccountState): RoleT {
+export function getOwnUserRole(state: PerAccountState): Role {
   const fromUserObject = getOwnUser(state).role;
 
   if (fromUserObject !== undefined) {
@@ -38,8 +38,8 @@ export function getOwnUserRole(state: PerAccountState): RoleT {
   }
 }
 
-export function roleIsAtLeast(thisRole: RoleT, thresholdRole: RoleT): boolean {
-  return thisRole <= thresholdRole; // Roles with more privilege have lower numbers.
+export function roleIsAtLeast(thisRole: Role, thresholdRole: Role): boolean {
+  return (thisRole: number) <= (thresholdRole: number); // Roles with more privilege have lower numbers.
 }
 
 /**

src/users/__tests__/usersReducer-test.js

@@ -6,7 +6,7 @@ import { UploadedAvatarURL } from '../../utils/avatar';
 import { EVENT_USER_ADD, EVENT } from '../../actionConstants';
 import { EventTypes, type RealmUserUpdateEvent } from '../../api/eventTypes';
 import type { User } from '../../types';
-import { RoleValues } from '../../api/permissionsTypes';
+import { Role } from '../../api/permissionsTypes';
 import usersReducer from '../usersReducer';
 import { randString } from '../../utils/misc';
 
@@ -115,7 +115,8 @@ describe('usersReducer', () => {
     });
 
     test('When the role of a user changes.', () => {
-      check({ role: RoleValues[(RoleValues.indexOf(theUser.role) + 1) % RoleValues.length] });
+      const roleValues = Array.from(Role.members());
+      check({ role: roleValues[(roleValues.indexOf(theUser.role) + 1) % roleValues.length] });
     });
 
     test("When a user's billing-admin status changes", () => {

src/webview/backgroundData.js

@@ -34,7 +34,7 @@ import { getMute } from '../mute/muteModel';
 import { getUnread } from '../unread/unreadModel';
 import { getUserStatuses } from '../user-statuses/userStatusesModel';
 import { type UserStatusesState } from '../user-statuses/userStatusesCore';
-import { type RoleT } from '../api/permissionsTypes';
+import { Role } from '../api/permissionsTypes';
 import { getOwnUserRole } from '../permissionSelectors';
 import type { ServerEmojiData } from '../api/modelTypes';
 
@@ -58,7 +58,7 @@ export type BackgroundData = $ReadOnly<{|
   allUsersById: Map<UserId, UserOrBot>,
   mutedUsers: MutedUsersState,
   ownUser: User,
-  ownUserRole: RoleT,
+  ownUserRole: Role,
   streams: Map<number, Stream>,
   subscriptions: Map<number, Subscription>,
   unread: UnreadState,