Fix accessible namespaces notification not navigating to correct settings (#4048)

src/jest.setup.ts

@@ -38,6 +38,6 @@ fetchMock.enableMocks();
 // Mock __non_webpack_require__ for tests
 globalThis.__non_webpack_require__ = jest.fn();
 
-process.on("unhandledRejection", (err) => {
+process.on("unhandledRejection", (err: any) => {
   fail(err);
 });

src/main/cluster.ts

@@ -35,6 +35,7 @@ import plimit from "p-limit";
 import type { ClusterState, ClusterRefreshOptions, ClusterMetricsResourceType, ClusterId, ClusterMetadata, ClusterModel, ClusterPreferences, ClusterPrometheusPreferences, UpdateClusterModel } from "../common/cluster-types";
 import { ClusterMetadataKey, initialNodeShellImage, ClusterStatus } from "../common/cluster-types";
 import { storedKubeConfigFolder, toJS } from "../common/utils";
+import type { Response } from "request";
 
 /**
  * Cluster
@@ -642,8 +643,6 @@ export class Cluster implements ClusterModel, ClusterState {
     };
   }
 
-  protected getAllowedNamespacesErrorCount = 0;
-
   protected async getAllowedNamespaces() {
     if (this.accessibleNamespaces.length) {
       return this.accessibleNamespaces;
@@ -655,24 +654,16 @@ export class Cluster implements ClusterModel, ClusterState {
       const { body: { items } } = await api.listNamespace();
       const namespaces = items.map(ns => ns.metadata.name);
 
-      this.getAllowedNamespacesErrorCount = 0; // reset on success
-
       return namespaces;
     } catch (error) {
       const ctx = (await this.getProxyKubeconfig()).getContextObject(this.contextName);
       const namespaceList = [ctx.namespace].filter(Boolean);
 
       if (namespaceList.length === 0 && error instanceof HttpError && error.statusCode === 403) {
-        this.getAllowedNamespacesErrorCount += 1;
+        const { response } = error as HttpError & { response: Response };
 
-        if (this.getAllowedNamespacesErrorCount > 3) {
-          // reset on send
-          this.getAllowedNamespacesErrorCount = 0;
-
-          // then broadcast, make sure it is 3 successive attempts
-          logger.info("[CLUSTER]: listing namespaces is forbidden, broadcasting", { clusterId: this.id, error });
-          broadcastMessage(ClusterListNamespaceForbiddenChannel, this.id);
-        }
+        logger.info("[CLUSTER]: listing namespaces is forbidden, broadcasting", { clusterId: this.id, error: response.body });
+        broadcastMessage(ClusterListNamespaceForbiddenChannel, this.id);
       }
 
       return namespaceList;

src/renderer/components/+entity-settings/entity-settings.tsx

@@ -34,6 +34,7 @@ import type { EntitySettingsRouteParams } from "../../../common/routes";
 import { groupBy } from "lodash";
 import { SettingLayout } from "../layout/setting-layout";
 import { HotbarIcon } from "../hotbar/hotbar-icon";
+import logger from "../../../common/logger";
 
 interface Props extends RouteComponentProps<EntitySettingsRouteParams> {
 }
@@ -45,6 +46,17 @@ export class EntitySettings extends React.Component<Props> {
   constructor(props: Props) {
     super(props);
     makeObservable(this);
+
+    const { hash } = navigation.location;
+
+    if (hash) {
+      const menuId = hash.slice(1);
+      const item = this.menuItems.find((item) => item.id === menuId);
+
+      if (item) {
+        this.activeTab = item.id;
+      }
+    }
   }
 
   get entityId() {
@@ -61,18 +73,10 @@ export class EntitySettings extends React.Component<Props> {
     return EntitySettingRegistry.getInstance().getItemsForKind(this.entity.kind, this.entity.apiVersion, this.entity.metadata.source);
   }
 
-  async componentDidMount() {
-    const { hash } = navigation.location;
+  get activeSetting() {
+    this.activeTab ||= this.menuItems[0]?.id;
 
-    if (hash) {
-      const item = this.menuItems.find((item) => item.title === hash.slice(1));
-
-      if (item) {
-        this.activeTab = item.id;
-      }
-    }
-
-    this.ensureActiveTab();
+    return this.menuItems.find((setting) => setting.id === this.activeTab);
   }
 
   onTabChange = (tabId: string) => {
@@ -122,33 +126,31 @@ export class EntitySettings extends React.Component<Props> {
     );
   }
 
-  ensureActiveTab() {
-    if (!this.activeTab) {
-      this.activeTab = this.menuItems[0]?.id;
-    }
-  }
-
   render() {
     if (!this.entity) {
-      console.error("entity not found", this.entityId);
+      logger.error("[ENTITY-SETTINGS]: entity not found", this.entityId);
 
       return null;
     }
 
-    this.ensureActiveTab();
-    const activeSetting = this.menuItems.find((setting) => setting.id === this.activeTab);
+    const { activeSetting } = this;
+
 
     return (
       <SettingLayout
         navigation={this.renderNavigation()}
         contentGaps={false}
       >
-        <section>
-          <h2 data-testid={`${activeSetting.id}-header`}>{activeSetting.title}</h2>
-          <section>
-            <activeSetting.components.View entity={this.entity} key={activeSetting.title} />
-          </section>
-        </section>
+        {
+          activeSetting && (
+            <section>
+              <h2 data-testid={`${activeSetting.id}-header`}>{activeSetting.title}</h2>
+              <section>
+                <activeSetting.components.View entity={this.entity} key={activeSetting.title} />
+              </section>
+            </section>
+          )
+        }
       </SettingLayout>
     );
   }

src/renderer/components/cluster-manager/cluster-status.tsx

@@ -90,7 +90,7 @@ export class ClusterStatus extends React.Component<Props> {
       params: {
         entityId: this.props.clusterId,
       },
-      fragment: "Proxy",
+      fragment: "proxy",
     }));
   };
 

src/renderer/components/layout/setting-layout.tsx

@@ -23,7 +23,7 @@ import "./setting-layout.scss";
 
 import React from "react";
 import { observer } from "mobx-react";
-import { boundMethod, cssNames, IClassName } from "../../utils";
+import { cssNames, IClassName } from "../../utils";
 import { navigation } from "../../navigation";
 import { Icon } from "../icon";
 
@@ -36,17 +36,10 @@ export interface SettingLayoutProps extends React.DOMAttributes<any> {
   back?: (evt: React.MouseEvent | KeyboardEvent) => void;
 }
 
-function scrollToAnchor() {
-  const { hash } = window.location;
-
-  if (hash) {
-    document.querySelector(`${hash}`).scrollIntoView();
-  }
-}
-
 const defaultProps: Partial<SettingLayoutProps> = {
   provideBackButtonNavigation: true,
   contentGaps: true,
+  back: () => navigation.goBack(),
 };
 
 /**
@@ -56,19 +49,14 @@ const defaultProps: Partial<SettingLayoutProps> = {
 export class SettingLayout extends React.Component<SettingLayoutProps> {
   static defaultProps = defaultProps as object;
 
-  @boundMethod
-  back(evt?: React.MouseEvent | KeyboardEvent) {
-    if (this.props.back) {
-      this.props.back(evt);
-    } else {
-      navigation.goBack();
-    }
-  }
-
   async componentDidMount() {
-    window.addEventListener("keydown", this.onEscapeKey);
+    const { hash } = window.location;
 
-    scrollToAnchor();
+    if (hash) {
+      document.querySelector(hash)?.scrollIntoView();
+    }
+
+    window.addEventListener("keydown", this.onEscapeKey);
   }
 
   componentWillUnmount() {
@@ -82,7 +70,7 @@ export class SettingLayout extends React.Component<SettingLayoutProps> {
 
     if (evt.code === "Escape") {
       evt.stopPropagation();
-      this.back(evt);
+      this.props.back(evt);
     }
   };
 
@@ -107,17 +95,18 @@ export class SettingLayout extends React.Component<SettingLayoutProps> {
             {children}
           </div>
           <div className="toolsRegion">
-            { this.props.provideBackButtonNavigation && (
-              <div className="fixedTools">
-                <div className="closeBtn" role="button" aria-label="Close" onClick={this.back}>
-                  <Icon material="close"/>
+            {
+              this.props.provideBackButtonNavigation && (
+                <div className="fixedTools">
+                  <div className="closeBtn" role="button" aria-label="Close" onClick={back}>
+                    <Icon material="close" />
+                  </div>
+                  <div className="esc" aria-hidden="true">
+                    ESC
+                  </div>
                 </div>
-
-                <div className="esc" aria-hidden="true">
-                  ESC
-                </div>
-              </div>
-            )}
+              )
+            }
           </div>
         </div>
       </div>

src/renderer/ipc/index.tsx

@@ -77,16 +77,15 @@ function UpdateAvailableHandler(event: IpcRendererEvent, ...[backchannel, update
   );
 }
 
-const listNamespacesForbiddenHandlerDisplayedAt = new Map<string, number>();
+const notificationLastDisplayedAt = new Map<string, number>();
 const intervalBetweenNotifications = 1000 * 60; // 60s
 
 function ListNamespacesForbiddenHandler(event: IpcRendererEvent, ...[clusterId]: ListNamespaceForbiddenArgs): void {
-  const lastDisplayedAt = listNamespacesForbiddenHandlerDisplayedAt.get(clusterId);
-  const wasDisplayed = Boolean(lastDisplayedAt);
+  const lastDisplayedAt = notificationLastDisplayedAt.get(clusterId);
   const now = Date.now();
 
-  if (!wasDisplayed || (now - lastDisplayedAt) > intervalBetweenNotifications) {
-    listNamespacesForbiddenHandlerDisplayedAt.set(clusterId, now);
+  if (!notificationLastDisplayedAt.has(clusterId) || (now - lastDisplayedAt) > intervalBetweenNotifications) {
+    notificationLastDisplayedAt.set(clusterId, now);
   } else {
     // don't bother the user too often
     return;
@@ -94,21 +93,39 @@ function ListNamespacesForbiddenHandler(event: IpcRendererEvent, ...[clusterId]:
 
   const notificationId = `list-namespaces-forbidden:${clusterId}`;
 
+  if (notificationsStore.getById(notificationId)) {
+    // notification is still visible
+    return;
+  }
+
   Notifications.info(
     (
       <div className="flex column gaps">
         <b>Add Accessible Namespaces</b>
-        <p>Cluster <b>{ClusterStore.getInstance().getById(clusterId).name}</b> does not have permissions to list namespaces. Please add the namespaces you have access to.</p>
+        <p>
+          Cluster <b>{ClusterStore.getInstance().getById(clusterId).name}</b> does not have permissions to list namespaces.{" "}
+          Please add the namespaces you have access to.
+        </p>
         <div className="flex gaps row align-left box grow">
-          <Button active outlined label="Go to Accessible Namespaces Settings" onClick={() => {
-            navigate(entitySettingsURL({ params: { entityId: clusterId }, fragment: "accessible-namespaces" }));
-            notificationsStore.remove(notificationId);
-          }} />
+          <Button
+            active
+            outlined
+            label="Go to Accessible Namespaces Settings"
+            onClick={() => {
+              navigate(entitySettingsURL({ params: { entityId: clusterId }, fragment: "namespaces" }));
+              notificationsStore.remove(notificationId);
+            }}
+          />
         </div>
       </div>
     ),
     {
       id: notificationId,
+      /**
+       * Set the time when the notification is closed as well so that there is at
+       * least a minute between closing the notification as seeing it again
+       */
+      onClose: () => notificationLastDisplayedAt.set(clusterId, Date.now()),
     }
   );
 }