Prevent multiple fetch of settings stage (#9878)

* Prevent multiple fetch of settings stage

- Cached in the useSettingsState hook already

* Error handling

src/frontend/lib/types/Settings.tsx

@@ -50,6 +50,7 @@ export interface SettingsStateProps {
   lookup: SettingsLookup;
   fetchSettings: () => Promise<boolean>;
   loaded: boolean;
+  isError: boolean;
   endpoint: ApiEndpoints;
   pathParams?: PathParams;
   getSetting: (key: string, default_value?: string) => string; // Return a raw setting value

src/frontend/src/components/settings/SettingList.tsx

@@ -1,19 +1,14 @@
 import { t } from '@lingui/core/macro';
 import { Trans } from '@lingui/react/macro';
-import { Skeleton, Stack, Text } from '@mantine/core';
+import { Alert, Skeleton, Stack, Text } from '@mantine/core';
 import { notifications } from '@mantine/notifications';
-import React, {
-  useCallback,
-  useEffect,
-  useMemo,
-  useRef,
-  useState
-} from 'react';
+import React, { useCallback, useMemo, useRef, useState } from 'react';
 import { useStore } from 'zustand';
 
 import type { ModelType } from '@lib/enums/ModelType';
 import { apiUrl } from '@lib/functions/Api';
 import type { Setting, SettingsStateProps } from '@lib/types/Settings';
+import { IconExclamationCircle } from '@tabler/icons-react';
 import { useApi } from '../../contexts/ApiContext';
 import { useEditApiFormModal } from '../../hooks/UseForm';
 import {
@@ -36,10 +31,6 @@ export function SettingList({
   keys?: string[];
   onChange?: () => void;
 }>) {
-  useEffect(() => {
-    settingsState.fetchSettings();
-  }, []);
-
   const api = useApi();
 
   const allKeys = useMemo(
@@ -133,6 +124,14 @@ export function SettingList({
     [settingsState]
   );
 
+  if (settingsState.isError) {
+    return (
+      <Alert color='red' icon={<IconExclamationCircle />} title={t`Error`}>
+        <Text>{t`Error loading settings`}</Text>
+      </Alert>
+    );
+  }
+
   if (!settingsState?.loaded) {
     return <Skeleton animate />;
   }

src/frontend/src/states/SettingsState.tsx

@@ -11,6 +11,7 @@ import type {
   SettingsLookup,
   SettingsStateProps
 } from '@lib/types/Settings';
+import { useEffect } from 'react';
 import { api } from '../App';
 import { isTrue } from '../functions/conversion';
 import { useUserState } from './UserState';
@@ -22,6 +23,7 @@ export const useGlobalSettingsState = create<SettingsStateProps>(
   (set, get) => ({
     settings: [],
     loaded: false,
+    isError: false,
     lookup: {},
     endpoint: ApiEndpoints.settings_global_list,
     fetchSettings: async () => {
@@ -30,7 +32,8 @@ export const useGlobalSettingsState = create<SettingsStateProps>(
 
       if (!isLoggedIn()) {
         set({
-          loaded: false
+          loaded: false,
+          isError: true
         });
         return success;
       }
@@ -40,8 +43,9 @@ export const useGlobalSettingsState = create<SettingsStateProps>(
         .then((response) => {
           set({
             settings: response.data,
+            lookup: generate_lookup(response.data),
             loaded: true,
-            lookup: generate_lookup(response.data)
+            isError: false
           });
         })
         .catch((_error) => {
@@ -49,7 +53,8 @@ export const useGlobalSettingsState = create<SettingsStateProps>(
           success = false;
 
           set({
-            loaded: false
+            loaded: false,
+            isError: true
           });
         });
 
@@ -72,6 +77,7 @@ export const useUserSettingsState = create<SettingsStateProps>((set, get) => ({
   settings: [],
   lookup: {},
   loaded: false,
+  isError: false,
   endpoint: ApiEndpoints.settings_user_list,
   fetchSettings: async () => {
     let success = true;
@@ -124,15 +130,30 @@ export const createPluginSettingsState = ({
 }: CreatePluginSettingStateProps) => {
   const pathParams: PathParams = { plugin };
 
-  return createStore<SettingsStateProps>()((set, get) => ({
+  const store = createStore<SettingsStateProps>()((set, get) => ({
     settings: [],
     lookup: {},
     loaded: false,
+    isError: false,
     endpoint: ApiEndpoints.plugin_setting_list,
     pathParams,
     fetchSettings: async () => {
       let success = true;
 
+      if (!plugin) {
+        set({
+          loaded: false,
+          isError: true
+        });
+
+        return false;
+      }
+
+      set({
+        loaded: false,
+        isError: false
+      });
+
       await api
         .get(apiUrl(ApiEndpoints.plugin_setting_list, undefined, { plugin }))
         .then((response) => {
@@ -140,14 +161,16 @@ export const createPluginSettingsState = ({
           set({
             settings,
             lookup: generate_lookup(settings),
-            loaded: true
+            loaded: true,
+            isError: false
           });
         })
         .catch((_error) => {
           console.error(`Error fetching plugin settings for plugin ${plugin}`);
           success = false;
           set({
-            loaded: false
+            loaded: false,
+            isError: true
           });
         });
 
@@ -161,6 +184,13 @@ export const createPluginSettingsState = ({
       return isTrue(value);
     }
   }));
+
+  useEffect(() => {
+    console.log('fetching plugin settings for', plugin);
+    store.getState().fetchSettings();
+  }, [plugin]);
+
+  return store;
 };
 
 /**
@@ -181,6 +211,7 @@ export const createMachineSettingsState = ({
     settings: [],
     lookup: {},
     loaded: false,
+    isError: false,
     endpoint: ApiEndpoints.machine_setting_detail,
     pathParams,
     fetchSettings: async () => {
@@ -195,7 +226,8 @@ export const createMachineSettingsState = ({
           set({
             settings,
             lookup: generate_lookup(settings),
-            loaded: true
+            loaded: true,
+            isError: false
           });
         })
         .catch((error) => {
@@ -205,7 +237,8 @@ export const createMachineSettingsState = ({
           );
           success = false;
           set({
-            loaded: false
+            loaded: false,
+            isError: true
           });
         });