mute: Enrich model to support new user_topic API

Ideally we'd rename this to something like `state.userTopic` too.
But it's currently a pain to actually successfully rename a state
subtree (and not have its ghost return at the old name): #5381.
So just keep calling it "mute" for now.

Also add tests for the model.  Note that we don't have to update
any of the existing tests!  They use `makeMuteState`, which uses
the reducer; so they get the new data structure automatically.

Author: gnprice

src/mute/__tests__/muteModel-test.js

@@ -2,14 +2,55 @@
 import deepFreeze from 'deep-freeze';
 
 import fullReducer from '../../boot/reducers';
-import { getMute, reducer } from '../muteModel';
+import { getMute, getTopicVisibilityPolicy, isTopicMuted, reducer } from '../muteModel';
 import { EVENT_MUTED_TOPICS } from '../../actionConstants';
 import * as eg from '../../__tests__/lib/exampleData';
 import { makeMuteState } from './mute-testlib';
 import { tryGetActiveAccountState } from '../../selectors';
+import { UserTopicVisibilityPolicy } from '../../api/modelTypes';
+
+/* eslint jest/expect-expect: ["error", { "assertFunctionNames": ["expect", "check"] }] */
 
 const initialState = makeMuteState([]);
 
+describe('getters', () => {
+  describe('getTopicVisibilityPolicy', () => {
+    function check(state, expected) {
+      expect(getTopicVisibilityPolicy(state, eg.stream.stream_id, 'topic')).toEqual(expected);
+    }
+
+    test('with nothing for stream', () => {
+      check(makeMuteState([]), UserTopicVisibilityPolicy.None);
+    });
+
+    test('with nothing for topic', () => {
+      check(makeMuteState([[eg.stream, 'other topic']]), UserTopicVisibilityPolicy.None);
+    });
+
+    test('with topic muted', () => {
+      check(makeMuteState([[eg.stream, 'topic']]), UserTopicVisibilityPolicy.Muted);
+    });
+  });
+
+  describe('isTopicMuted', () => {
+    function check(state, expected) {
+      expect(isTopicMuted(eg.stream.stream_id, 'topic', state)).toEqual(expected);
+    }
+
+    test('with nothing for stream', () => {
+      check(makeMuteState([]), false);
+    });
+
+    test('with nothing for topic', () => {
+      check(makeMuteState([[eg.stream, 'other topic']]), false);
+    });
+
+    test('with topic muted', () => {
+      check(makeMuteState([[eg.stream, 'topic']]), true);
+    });
+  });
+});
+
 describe('reducer', () => {
   describe('REGISTER_COMPLETE', () => {
     test('when mute data is provided init state with it: local', () => {

src/mute/muteModel.js

@@ -1,6 +1,8 @@
 /* @flow strict-local */
+import Immutable from 'immutable';
 
 import type { MuteState, PerAccountApplicableAction, PerAccountState } from '../types';
+import { UserTopicVisibilityPolicy } from '../api/modelTypes';
 import { REGISTER_COMPLETE, EVENT_MUTED_TOPICS, RESET_ACCOUNT_DATA } from '../actionConstants';
 import { getStreamsByName } from '../subscriptions/subscriptionSelectors';
 import * as logging from '../utils/logging';
@@ -18,27 +20,52 @@ export const getMute = (state: PerAccountState): MuteState => state.mute;
 // Getters.
 //
 
-export const isTopicMuted = (streamId: number, topic: string, mute: MuteState): boolean =>
-  mute.get(streamId)?.has(topic) ?? false;
+export function getTopicVisibilityPolicy(
+  mute: MuteState,
+  streamId: number,
+  topic: string,
+): UserTopicVisibilityPolicy {
+  return mute.get(streamId)?.get(topic) ?? UserTopicVisibilityPolicy.None;
+}
+
+export function isTopicMuted(streamId: number, topic: string, mute: MuteState): boolean {
+  const policy = getTopicVisibilityPolicy(mute, streamId, topic);
+  switch (policy) {
+    case UserTopicVisibilityPolicy.None:
+      return false;
+    case UserTopicVisibilityPolicy.Muted:
+      return true;
+  }
+}
 
 //
 //
 // Reducer.
 //
 
-const initialState: MuteState = new Map();
+const initialState: MuteState = Immutable.Map();
 
 function convert(data, streams): MuteState {
-  const result = new DefaultMap(() => new Set());
+  // Turn the incoming array into a nice, indexed, Immutable data structure
+  // in the same two-phase pattern we use for the unread data, as an
+  // optimization.  Here it's probably much less often enough data for the
+  // optimization to matter; but a longtime user who regularly uses this
+  // feature will accumulate many records over time, and then it could.
+
+  // First, collect together all the data for a given stream, just in a
+  // plain old Array.
+  const byStream = new DefaultMap(() => []);
   for (const [streamName, topic] of data) {
     const stream = streams.get(streamName);
     if (!stream) {
       logging.warn('mute: unknown stream');
       continue;
     }
-    result.getOrCreate(stream.stream_id).add(topic);
+    byStream.getOrCreate(stream.stream_id).push([topic, UserTopicVisibilityPolicy.Muted]);
   }
-  return result.map;
+
+  // Then, from each of those build an Immutable.Map all in one shot.
+  return Immutable.Map(Immutable.Seq.Keyed(byStream.map.entries()).map(Immutable.Map));
 }
 
 export const reducer = (

src/mute/muteModelTypes.js

@@ -1,5 +1,23 @@
 // @flow strict-local
+import Immutable from 'immutable';
 
-// No need for Immutable, because -- the server API actually doesn't support
-// incremental changes!  When something changes, it sends the entire new list.
-export type MuteState = Map<number, Set<string>>; // stream ID, topic
+import { type UserTopicVisibilityPolicy } from '../api/modelTypes';
+
+/**
+ * The "visibility policy" our user has chosen for each topic.
+ *
+ * See jsdoc of UserTopicVisibilityPolicy for background.
+ *
+ * In this data structure, the keys are stream ID and then topic name.
+ * Values of `UserTopicVisibilityPolicy.None` are represented by absence,
+ * and streams where the map would be empty are also omitted.
+ */
+// TODO(#5381): Ideally we'd call this UserTopicState and `state.userTopic`.
+//   But it's currently a pain to actually rename a state subtree: #5381.
+export type MuteState = Immutable.Map<
+  number, // stream ID
+  Immutable.Map<
+    string, // topic name
+    UserTopicVisibilityPolicy,
+  >,
+>;

src/storage/__tests__/migrations-test.js

@@ -105,7 +105,7 @@ describe('migrations', () => {
   // What `base` becomes after all migrations.
   const endBase = {
     ...base52,
-    migrations: { version: 58 },
+    migrations: { version: 59 },
   };
 
   for (const [desc, before, after] of [
@@ -128,9 +128,9 @@ describe('migrations', () => {
     // redundant with this one, because none of the migration steps notice
     // whether any properties outside `storeKeys` are present or not.
     [
-      'check dropCache at 56',
+      'check dropCache at 59',
       // Just before the `dropCache`, plus a `cacheKeys` property, plus junk.
-      { ...base52, migrations: { version: 55 }, mute: [], nonsense: [1, 2, 3] },
+      { ...base52, migrations: { version: 58 }, mute: [], nonsense: [1, 2, 3] },
       // Should wind up with the same result as without the extra properties.
       endBase,
     ],

src/storage/migrations.js

@@ -502,6 +502,9 @@ const migrationsInner: {| [string]: (LessPartialState) => LessPartialState |} =
     ),
   }),
 
+  // Change `state.mute` data structure: was a plain JS Map.
+  '59': dropCache,
+
   // TIP: When adding a migration, consider just using `dropCache`.
   //   (See its jsdoc for guidance on when that's the right answer.)
 };