From 23be1fca0f5aa13cb7dec6f820a56ee174fa2c7e Mon Sep 17 00:00:00 2001 From: Tola Leng Date: Sat, 24 May 2025 18:20:33 +0800 Subject: [PATCH] Fix: Reduce maintenance data request frequency. Fix: The maintenance list should refresh immediately after a new maintenance entry is created. --- .../hooks/useMaintenanceData.ts | 130 ++++++++++-------- .../maintenance/api/maintenanceCache.ts | 30 +++- .../maintenance/api/maintenanceFetch.ts | 94 +++++++++---- .../maintenance/maintenanceService.ts | 4 +- 4 files changed, 172 insertions(+), 86 deletions(-) diff --git a/application/src/components/schedule-incident/hooks/useMaintenanceData.ts b/application/src/components/schedule-incident/hooks/useMaintenanceData.ts index 3af4626..8da2bd9 100644 --- a/application/src/components/schedule-incident/hooks/useMaintenanceData.ts +++ b/application/src/components/schedule-incident/hooks/useMaintenanceData.ts @@ -1,5 +1,5 @@ -import { useState, useEffect, useCallback, useMemo } from 'react'; +import { useState, useEffect, useCallback, useMemo, useRef } from 'react'; import { maintenanceService, MaintenanceItem } from '@/services/maintenance'; import { useToast } from '@/hooks/use-toast'; import { useLanguage } from '@/contexts/LanguageContext'; @@ -16,7 +16,10 @@ export const useMaintenanceData = ({ refreshTrigger = 0 }: UseMaintenanceDataPro const [filter, setFilter] = useState("upcoming"); const [error, setError] = useState(null); const [initialized, setInitialized] = useState(false); - const [lastFetchTime, setLastFetchTime] = useState(0); + + // Refs for cleanup and request management + const mountedRef = useRef(true); + const currentRequestRef = useRef | null>(null); // Memoize categorized data to prevent unnecessary recalculations const categorizedData = useMemo(() => { @@ -48,80 +51,91 @@ export const useMaintenanceData = ({ refreshTrigger = 0 }: UseMaintenanceDataPro return { upcoming, ongoing, completed }; }, [allMaintenanceData]); - // Optimized fetch function with improved debouncing + // Simple fetch function - only called when explicitly requested const fetchMaintenanceData = useCallback(async (force = false) => { - // Prevent excessive fetching with improved debounce logic - const now = Date.now(); - if (!force && now - lastFetchTime < 10000 && lastFetchTime > 0 && initialized) { - console.log("Skipping fetch - recently fetched (within 10s)"); + // Check if component is still mounted + if (!mountedRef.current) return; + + // Prevent duplicate requests + if (currentRequestRef.current) { + console.log("Request already in progress, waiting..."); + await currentRequestRef.current; return; } - // Only show loading state for initial load, not refreshes - if (!initialized) { + // Only show loading state for initial load or forced refresh + if (!initialized || force) { setLoading(true); } setError(null); - setLastFetchTime(now); - try { - console.log("Fetching maintenance data from service..."); - const data = await maintenanceService.getMaintenanceRecords(); - console.log("Fetched maintenance data, count:", data.length); - - // Log a sample of the first item's assigned_users for debugging - if (data.length > 0) { - console.log("Sample maintenance item assigned_users:", data[0].id, data[0].assigned_users); - } - - // Update state with fetched data - setAllMaintenanceData(data); - setInitialized(true); - } catch (error) { - console.error('Error fetching maintenance data:', error); - setError('Failed to load maintenance data'); - toast({ - title: t('error'), - description: t('errorFetchingMaintenanceData'), - variant: 'destructive', - }); - } finally { - setLoading(false); - } - }, [t, toast, lastFetchTime, initialized]); - - // Initial fetch on mount with proper cleanup - useEffect(() => { - console.log("useMaintenanceData hook mounted, fetching data"); - let isMounted = true; - - const fetchData = async () => { + const requestPromise = (async () => { try { - await fetchMaintenanceData(true); + console.log("Fetching maintenance data...", force ? "(forced)" : ""); + const data = await maintenanceService.getMaintenanceRecords(); + + // Check if component is still mounted before updating state + if (!mountedRef.current) return; + + console.log(`Fetched ${data.length} maintenance records`); + + // Update state with fetched data + setAllMaintenanceData(data); + setInitialized(true); + + // Clear any previous error + if (error) { + setError(null); + } + } catch (err) { - console.error("Error in initial fetch:", err); + console.error('Error fetching maintenance data:', err); + + // Only update error state if component is still mounted + if (!mountedRef.current) return; + + const errorMessage = 'Failed to load maintenance data. Please try again.'; + setError(errorMessage); + + // Show toast for errors + toast({ + title: t('error'), + description: t('errorFetchingMaintenanceData'), + variant: 'destructive', + }); + } finally { + // Only update loading state if component is still mounted + if (mountedRef.current) { + setLoading(false); + } + currentRequestRef.current = null; } - }; + })(); + + currentRequestRef.current = requestPromise; + await requestPromise; + }, [t, toast, error, initialized]); + + // Initial fetch on mount - NO AUTOMATIC POLLING + useEffect(() => { + console.log("useMaintenanceData hook mounted, fetching initial data"); + mountedRef.current = true; - fetchData(); - - // Set up polling with longer interval (5 minutes instead of 3) - const intervalId = setInterval(() => { - if (isMounted) fetchMaintenanceData(false); - }, 300000); // 5 minutes + // Only fetch initial data, no polling + fetchMaintenanceData(true); return () => { - isMounted = false; - clearInterval(intervalId); + mountedRef.current = false; + currentRequestRef.current = null; }; - }, [fetchMaintenanceData]); + }, []); // Remove fetchMaintenanceData from dependencies to prevent re-runs - // Handle refresh trigger changes + // Handle refresh trigger changes - ONLY when explicitly triggered useEffect(() => { if (refreshTrigger > 0) { - console.log("Refresh trigger changed, forcing data fetch"); - fetchMaintenanceData(true); + console.log("Manual refresh triggered, forcing data fetch"); + fetchMaintenanceData(true); // Force refresh to bypass cache } }, [refreshTrigger, fetchMaintenanceData]); @@ -170,4 +184,4 @@ export const useMaintenanceData = ({ refreshTrigger = 0 }: UseMaintenanceDataPro error, initialized, }; -}; +}; \ No newline at end of file diff --git a/application/src/services/maintenance/api/maintenanceCache.ts b/application/src/services/maintenance/api/maintenanceCache.ts index bfbe0a6..a2a3003 100644 --- a/application/src/services/maintenance/api/maintenanceCache.ts +++ b/application/src/services/maintenance/api/maintenanceCache.ts @@ -6,14 +6,21 @@ import { MaintenanceItem } from '../../types/maintenance.types'; // Cache for maintenance records let cachedMaintenanceRecords: MaintenanceItem[] | null = null; let lastFetchTimestamp = 0; -const CACHE_EXPIRY_MS = 60000; // 1 minute cache expiry +const CACHE_EXPIRY_MS = 600000; // 10 minutes cache expiry (increased significantly) /** * Check if cache is valid and can be used */ export const isCacheValid = (): boolean => { const now = Date.now(); - return !!(cachedMaintenanceRecords && now - lastFetchTimestamp < CACHE_EXPIRY_MS); + const isValid = !!(cachedMaintenanceRecords && now - lastFetchTimestamp < CACHE_EXPIRY_MS); + + // Minimal logging to reduce console spam + if (isValid) { + console.log("Using cached maintenance data"); + } + + return isValid; }; /** @@ -21,7 +28,7 @@ export const isCacheValid = (): boolean => { */ export const getCachedRecords = () => { if (cachedMaintenanceRecords) { - console.log("Using cached maintenance records", cachedMaintenanceRecords.length); + console.log("Returning cached maintenance records:", cachedMaintenanceRecords.length); return [...cachedMaintenanceRecords]; // Return a copy to prevent mutation } return null; @@ -44,3 +51,20 @@ export const clearCache = (): void => { lastFetchTimestamp = 0; console.log("Maintenance cache cleared"); }; + +/** + * Get cache statistics for debugging + */ +export const getCacheStats = () => { + const now = Date.now(); + const cacheAge = lastFetchTimestamp ? now - lastFetchTimestamp : 0; + const timeUntilExpiry = lastFetchTimestamp ? CACHE_EXPIRY_MS - cacheAge : 0; + + return { + hasCache: !!cachedMaintenanceRecords, + cacheSize: cachedMaintenanceRecords?.length || 0, + cacheAge: Math.round(cacheAge / 1000), // in seconds + timeUntilExpiry: Math.round(timeUntilExpiry / 1000), // in seconds + isValid: isCacheValid() + }; +}; \ No newline at end of file diff --git a/application/src/services/maintenance/api/maintenanceFetch.ts b/application/src/services/maintenance/api/maintenanceFetch.ts index cd20a22..b79116c 100644 --- a/application/src/services/maintenance/api/maintenanceFetch.ts +++ b/application/src/services/maintenance/api/maintenanceFetch.ts @@ -2,58 +2,104 @@ import { pb } from '@/lib/pocketbase'; import { MaintenanceItem } from '../../types/maintenance.types'; import { normalizeMaintenanceItem } from '../maintenanceUtils'; -import { isCacheValid, getCachedRecords, updateCache } from './maintenanceCache'; +import { isCacheValid, getCachedRecords, updateCache, clearCache } from './maintenanceCache'; + +// Request management +let currentRequest: Promise | null = null; +let lastRequestTime = 0; +const MIN_REQUEST_INTERVAL = 30000; // 30 seconds minimum between requests /** - * Get all maintenance records from the API with optimized caching + * Get all maintenance records from the API with strict request control */ export const fetchAllMaintenanceRecords = async (forceRefresh = false): Promise => { try { const now = Date.now(); + // If forced refresh, clear cache first + if (forceRefresh) { + clearCache(); + console.log('Cache cleared for forced refresh'); + } + // Return cached data if available and not forced refresh if (!forceRefresh && isCacheValid()) { return getCachedRecords() as MaintenanceItem[]; } - // Use a unique requestKey to prevent caching issues - const timestamp = now; + // Request deduplication: if a request is already in progress, wait for it + if (currentRequest) { + console.log('Request already in progress, waiting for completion'); + return await currentRequest; + } - // Increase items per page and use fields parameter to fetch only needed data - // Also expand the assigned_users relation to get user details + // Strict rate limiting - prevent requests too close together (unless forced) + const timeSinceLastRequest = now - lastRequestTime; + if (timeSinceLastRequest < MIN_REQUEST_INTERVAL && !forceRefresh) { + console.log(`Rate limiting: ${Math.round((MIN_REQUEST_INTERVAL - timeSinceLastRequest) / 1000)}s remaining`); + return getCachedRecords() || []; + } + + lastRequestTime = now; + + // Create the request promise + currentRequest = performRequest(now); + + try { + const result = await currentRequest; + return result; + } finally { + currentRequest = null; // Clear the current request + } + + } catch (error) { + currentRequest = null; // Clear the current request on error + + console.error('Error fetching maintenance records:', error); + + // Return cached data on error if available + return getCachedRecords() || []; + } +}; + +/** + * Perform the actual API request + */ +const performRequest = async (timestamp: number): Promise => { + console.log('Making API request for maintenance records...'); + + // Create abort controller for request timeout + const controller = new AbortController(); + const timeoutId = setTimeout(() => controller.abort(), 30000); // 30 second timeout + + try { const result = await pb.collection('maintenance').getList(1, 200, { sort: '-created', requestKey: `maintenance-${timestamp}`, $cancelKey: `maintenance-fetch-${timestamp}`, - expand: 'assigned_users', // Expand the relation to get user details + expand: 'assigned_users', + signal: controller.signal, }); + clearTimeout(timeoutId); + if (!result || !result.items || result.items.length === 0) { const emptyData: MaintenanceItem[] = []; - updateCache(emptyData, now); + updateCache(emptyData, timestamp); return emptyData; } // Process and normalize data - const normalizedData = result.items.map(item => { - console.log("Processing maintenance item:", item.id); - console.log("Item assigned_users:", item.assigned_users); - console.log("Notification channel:", item.notification_channel_id); - return normalizeMaintenanceItem(item); - }); + const normalizedData = result.items.map(item => normalizeMaintenanceItem(item)); // Update cache - updateCache(normalizedData, now); + updateCache(normalizedData, timestamp); + console.log(`Successfully fetched ${normalizedData.length} maintenance records`); return normalizedData; - } catch (error) { - // Handle abort errors gracefully without console errors - if ((error as any)?.isAbort) { - console.log("Request aborted:", error); - return getCachedRecords() || []; - } - console.error('Error fetching maintenance records:', error); - return getCachedRecords() || []; + } catch (error) { + clearTimeout(timeoutId); + throw error; } -}; +}; \ No newline at end of file diff --git a/application/src/services/maintenance/maintenanceService.ts b/application/src/services/maintenance/maintenanceService.ts index 5a8ffe3..b4e9d28 100644 --- a/application/src/services/maintenance/maintenanceService.ts +++ b/application/src/services/maintenance/maintenanceService.ts @@ -61,12 +61,14 @@ const getCompletedMaintenance = async (): Promise => { const createMaintenance = async (data: CreateMaintenanceInput): Promise => { await createMaintenanceRecord(data); clearMaintenanceCache(); // Force cache refresh after creation + console.log('Maintenance created and cache cleared'); }; // Update an existing maintenance record const updateMaintenanceRecord = async (id: string, data: Partial): Promise => { await updateMaintenance(id, data); clearMaintenanceCache(); // Force cache refresh after update + console.log('Maintenance updated and cache cleared'); }; // Refresh cache manually @@ -89,4 +91,4 @@ export const maintenanceService = { }; // Re-export types for convenience -export type { MaintenanceItem, CreateMaintenanceInput }; +export type { MaintenanceItem, CreateMaintenanceInput }; \ No newline at end of file