fix: get_settings rate limit issue (#84)

* fix: get_settings rate limit issue * fix: apply comments from sourcery
deriv-com · Jan 17, 2025 · 4442092 · 4442092
1 parent ecdb88e
commit 4442092
Show file tree

Hide file tree

Showing 4 changed files with 63 additions and 46 deletions.
diff --git a/src/components/CopierDashboard.jsx b/src/components/CopierDashboard.jsx
@@ -1,20 +1,14 @@
 import { useEffect, useState } from "react";
 import { Text, Snackbar, Spinner, SectionMessage } from "@deriv-com/quill-ui";
+import PropTypes from "prop-types";
 import useCopyTradersList from "../hooks/useCopyTradersList";
 import CopyTradingBanner from "./CopyTradingBanner";
-import useSettings from "../hooks/useSettings";
 import useCopyStart from "../hooks/useCopyStart";
 import useCopyStop from "../hooks/useCopyStop";
 import AddTraderForm from "./AddTraderForm";
 import TraderCard from "./TraderCard";
 
-const CopierDashboard = () => {
-    const {
-        settings,
-        updateSettings,
-        fetchSettings,
-        isLoading: isSettingsLoading,
-    } = useSettings();
+const CopierDashboard = ({ settings, updateSettings, fetchSettings }) => {
     const showCopierBanner = settings?.allow_copiers === 1;
     const { startCopyTrading, processingTrader: copyStartProcessingTrader } =
         useCopyStart();
@@ -107,9 +101,7 @@ const CopierDashboard = () => {
         }
     };
 
-    const isPageLoading = isLoading || isSettingsLoading;
-
-    if (isPageLoading) {
+    if (isLoading) {
         return (
             <div className="flex justify-center items-center min-h-[200px]">
                 <Spinner />
@@ -176,4 +168,10 @@ const CopierDashboard = () => {
     );
 };
 
+CopierDashboard.propTypes = {
+    settings: PropTypes.object,
+    updateSettings: PropTypes.func.isRequired,
+    fetchSettings: PropTypes.func.isRequired,
+};
+
 export default CopierDashboard;
diff --git a/src/components/Dashboard.jsx b/src/components/Dashboard.jsx
@@ -1,4 +1,4 @@
-import { useState, useEffect } from "react";
+import { useState } from "react";
 import { SegmentedControlSingleChoice, Skeleton } from "@deriv-com/quill-ui";
 import { useAuth } from "../hooks/useAuth.jsx";
 import useSettings from "../hooks/useSettings.js";
@@ -7,20 +7,15 @@ import CopierDashboard from "./CopierDashboard";
 
 const Dashboard = () => {
     const { isLoading: authLoading } = useAuth();
-    const { settings, isLoading: settingsLoading } = useSettings();
-    const [userType, setUserType] = useState(() => {
-        // Set initial state based on settings if available
-        return settings?.allow_copiers ? "trader" : "copier";
-    });
-
-    useEffect(() => {
-        console.log("Settings updated:", settings);
-        if (settings) {
-            const newUserType = settings.allow_copiers ? "trader" : "copier";
-            console.log("Setting userType to:", newUserType);
-            setUserType(newUserType);
-        }
-    }, [settings]);
+    const {
+        settings,
+        isLoading: settingsLoading,
+        updateSettings,
+        fetchSettings,
+    } = useSettings();
+    const [userType, setUserType] = useState(
+        settings?.allow_copiers ? "trader" : "copier"
+    );
 
     const isLoading = authLoading || settingsLoading;
 
@@ -75,11 +70,23 @@ const Dashboard = () => {
                         </div>
                     </div>
                 ) : userType === "trader" ? (
-                    <TraderDashboard />
+                    <TraderDashboard
+                        settings={settings}
+                        updateSettings={updateSettings}
+                        fetchSettings={fetchSettings}
+                    />
                 ) : userType === "copier" ? (
-                    <CopierDashboard />
+                    <CopierDashboard
+                        settings={settings}
+                        updateSettings={updateSettings}
+                        fetchSettings={fetchSettings}
+                    />
                 ) : (
-                    <CopierDashboard />
+                    <CopierDashboard
+                        settings={settings}
+                        updateSettings={updateSettings}
+                        fetchSettings={fetchSettings}
+                    />
                 )}
             </div>
         </div>

diff --git a/src/components/TraderDashboard.jsx b/src/components/TraderDashboard.jsx
@@ -1,6 +1,6 @@
 import { useState } from "react";
 import { Spinner, SectionMessage } from "@deriv-com/quill-ui";
-import useSettings from "../hooks/useSettings";
+import PropTypes from "prop-types";
 import useCopyTradersList from "../hooks/useCopyTradersList";
 import TraderStatistics from "./TraderStatistics";
 import TraderBanner from "./TraderBanner";
@@ -9,13 +9,7 @@ import Settings from "./Settings";
 import TraderDesktopNavigation from "./TraderDesktopNavigation";
 import TraderMobileNavigation from "./TraderMobileNavigation";
 
-const TraderDashboard = () => {
-    const {
-        settings,
-        isLoading: settingsLoading,
-        updateSettings,
-        fetchSettings,
-    } = useSettings();
+const TraderDashboard = ({ settings, updateSettings, fetchSettings }) => {
     const { traders, isLoading: tradersLoading } = useCopyTradersList();
     const [selectedMenu, setSelectedMenu] = useState("statistics");
 
@@ -28,7 +22,7 @@ const TraderDashboard = () => {
         }
     };
 
-    if (settingsLoading || tradersLoading) {
+    if (tradersLoading) {
         return (
             <div className="flex justify-center items-center min-h-[200px]">
                 <Spinner />
@@ -83,4 +77,10 @@ const TraderDashboard = () => {
     );
 };
 
+TraderDashboard.propTypes = {
+    settings: PropTypes.object,
+    updateSettings: PropTypes.func.isRequired,
+    fetchSettings: PropTypes.func.isRequired,
+};
+
 export default TraderDashboard;
diff --git a/src/hooks/useSettings.js b/src/hooks/useSettings.js
@@ -1,4 +1,4 @@
-import { useState, useEffect, useCallback } from 'react';
+import { useState, useEffect, useCallback, useRef } from 'react';
 import useWebSocket from './useWebSocket';
 import { useAuth } from '../hooks/useAuth.jsx';
 
@@ -8,14 +8,17 @@ const useSettings = () => {
     const [isLoading, setIsLoading] = useState(true);
     const { isAuthorized, isConnected } = useAuth();
     const { sendMessage } = useWebSocket();
+    const hasInitialFetch = useRef(false);
+    const lastUpdateCall = useRef(null);
 
     const fetchSettings = useCallback(() => {
-        if (!isConnected || !isAuthorized) {
+        if (!isConnected || !isAuthorized || hasInitialFetch.current) {
             return;
         }
 
         setIsLoading(true);
         console.log('Fetching user settings');
+        hasInitialFetch.current = true;
         sendMessage(
             { get_settings: 1 },
             (response) => {
@@ -32,18 +35,19 @@ const useSettings = () => {
         );
     }, [isConnected, isAuthorized, sendMessage]);
 
-    // Fetch settings when authorized
+    // Fetch settings on mount and when connection/auth state changes
     useEffect(() => {
-        if (isConnected && isAuthorized && !settings) {
+        if (isConnected && isAuthorized) {
             fetchSettings();
         }
-    }, [isConnected, isAuthorized, settings, fetchSettings]);
+    }, [isConnected, isAuthorized, fetchSettings]);
 
-    // Reset settings when connection is lost
+    // Reset settings and fetch flag when connection is lost
     useEffect(() => {
         if (!isConnected) {
             setSettings(null);
             setIsLoading(true);
+            hasInitialFetch.current = false;
         }
     }, [isConnected]);
 
@@ -52,6 +56,13 @@ const useSettings = () => {
             throw new Error('Not connected or authorized');
         }
 
+        // Add debounce to prevent rapid settings updates
+        const now = Date.now();
+        if (lastUpdateCall.current && now - lastUpdateCall.current < 1000) {
+            throw new Error('Please wait before updating settings again');
+        }
+        lastUpdateCall.current = now;
+
         return new Promise((resolve, reject) => {
             sendMessage(
                 { set_settings: 1, ...newSettings },
@@ -62,8 +73,9 @@ const useSettings = () => {
                         reject(response.error);
                     } else {
                         console.log('Settings updated:', response.set_settings);
-                        setSettings(response.set_settings);
-                        setError(null);
+                        // After successful update, reset fetch flag and get fresh settings
+                        hasInitialFetch.current = false;
+                        fetchSettings();
                         resolve(response.set_settings);
                     }
                 }