[MM-68797] MBE Phase 8c: registerChannelIconOverride follow-ups (mattermost#36576)

* phase 8c

* merge errors

* PR comments

Author: cpoile

webapp/channels/src/components/channel_type_icon/channel_icon_override.test.ts

@@ -334,3 +334,4 @@ describe('selectors/getChannelIconOverrideForChannel', () => {
         });
     });
 });
+

webapp/channels/src/components/channel_type_icon/channel_icon_override.ts

@@ -27,11 +27,10 @@ export function clearLoggedMatcherErrors(pluginId?: string): void {
 /**
  * Returns the IconGlyphTypes name of the first matching plugin override, or null.
  *
- * If `channel` is present, iterates `state.plugins.components.ChannelIconOverride` in array order
- * (sorted alphabetically by pluginId via the reducer). The first matcher returning true wins.
- * Matcher throws are caught, logged once per pluginId, and treated as no-match.
- *
- * Note: Do not use createSelector here — this is a per-row call over a short override list.
+ * Iterates the registered matchers on every call. The framework does not memoize across
+ * dispatches because the matcher contract takes full Redux state, so we cannot infer
+ * which slices it reads. If a plugin's matcher is expensive, the plugin should memoize
+ * inside its own predicate using `createSelector` keyed on the slices it consults.
  */
 export function getChannelIconOverrideForChannel(
     state: GlobalState,
@@ -43,8 +42,7 @@ export function getChannelIconOverrideForChannel(
     const overrides = state.plugins.components.ChannelIconOverride ?? [];
     for (const entry of overrides) {
         try {
-            const matched = entry.matcher(state, channel);
-            if (matched === true) {
+            if (entry.matcher(state, channel) === true) {
                 return entry.iconName;
             }
         } catch (err) {

webapp/channels/src/components/channel_type_icon/channel_type_icon.tsx

@@ -12,6 +12,15 @@ type Props = {
     channel?: Channel;
 } & HTMLAttributes<HTMLElement>;
 
+/**
+ * Renders a channel icon as a font glyph (`<i className="icon icon-…"/>`) honoring
+ * any plugin override registered via `registerChannelIconOverride`. Use this for
+ * font-icon sites.
+ *
+ * For SVG sites where you need a Compass SVG component directly, use the
+ * `useChannelIconOverrideName(channel)` hook to get the override name, then
+ * resolve via `compassIconForName(name)` and render the resulting component.
+ */
 const ChannelTypeIcon = ({channel, className, ...rest}: Props) => {
     const iconClassName = useChannelIconClassName(channel);
     return (

webapp/channels/src/components/channel_type_icon/useChannelIconOverrideName.ts

@@ -10,6 +10,21 @@ import type {GlobalState} from 'types/store';
 
 import {getChannelIconOverrideForChannel} from './channel_icon_override';
 
+/**
+ * Returns the override icon name (e.g., `'shield-outline'`) for the given channel
+ * from the first matching plugin registration, or null if no override matches.
+ *
+ * For font-icon sites, prefer the `<ChannelTypeIcon channel={channel}/>` component,
+ * which handles override resolution and rendering. Use this hook only when you
+ * need the raw icon name for an SVG render path (resolve via
+ * `compassIconForName(name)` and render the resulting component).
+ *
+ * Matcher cost: the resolver iterates every registered matcher on every call. The
+ * framework does not memoize across dispatches because the matcher contract takes
+ * full Redux state and we cannot know what slices it reads. Plugins with expensive
+ * matchers should memoize internally using `createSelector` keyed on the slices
+ * the matcher actually consults.
+ */
 export function useChannelIconOverrideName(channel?: Channel): IconGlyphTypes | null {
     return useSelector((state: GlobalState) => getChannelIconOverrideForChannel(state, channel));
 }

webapp/channels/src/utils/channel_utils.test.ts

@@ -173,6 +173,32 @@ describe('Channel Utils', () => {
         });
     });
 
+    describe('isArchivedChannel', () => {
+        test('returns false for undefined', () => {
+            expect(Utils.isArchivedChannel(undefined)).toBe(false);
+        });
+
+        test('returns false for channel object missing delete_at', () => {
+            expect(Utils.isArchivedChannel({} as Channel)).toBe(false);
+        });
+
+        test('returns false for channel with delete_at: 0', () => {
+            expect(Utils.isArchivedChannel({delete_at: 0} as Channel)).toBe(false);
+        });
+
+        test('returns true for channel with delete_at: 12345', () => {
+            expect(Utils.isArchivedChannel({delete_at: 12345} as Channel)).toBe(true);
+        });
+
+        test('returns true for channel with delete_at: -1', () => {
+            expect(Utils.isArchivedChannel({delete_at: -1} as Channel)).toBe(true);
+        });
+
+        test('getChannelIconClassName returns icon-globe for open channel missing delete_at', () => {
+            expect(Utils.getChannelIconClassName({type: 'O'} as Channel)).toBe('icon-globe');
+        });
+    });
+
     describe('getChannelRoutePathAndIdentifier', () => {
         test('should return channels path and channel name for open channels', () => {
             const channel = {type: Constants.OPEN_CHANNEL, name: 'town-square'} as Channel;

webapp/channels/src/utils/channel_utils.tsx

@@ -62,7 +62,7 @@ export function findNextUnreadChannelId(curChannelId: string, allChannelIds: str
 }
 
 export function isArchivedChannel(channel?: Channel) {
-    return Boolean(channel && channel.delete_at !== 0);
+    return Boolean(channel?.delete_at);
 }
 
 /**