Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Analytics] Added maintenance banner #2296

Merged
merged 7 commits into from
Dec 4, 2024
Merged

[Analytics] Added maintenance banner #2296

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
1e9b10f
added maintenance banner
Codebmk Dec 4, 2024
d97584a
Update index.jsx
Codebmk Dec 4, 2024
b84c5de
adjust size and positioning
Codebmk Dec 4, 2024
bdda6b6
format time
Codebmk Dec 4, 2024
5202804
fix date time
Codebmk Dec 4, 2024
d27cd8e
Update index.jsx
Codebmk Dec 4, 2024
5b332d0
Update index.jsx
Codebmk Dec 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions platform/src/common/components/Layout/index.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,14 @@ import AuthenticatedSideBar from '@/components/SideBar/AuthenticatedSidebar';
import TopBar from '../TopBar';
import SideBarDrawer from '../SideBar/SideBarDrawer';
import Modal from '../Modal/dataDownload';
import MaintenanceBanner from '../MaintenanceBanner';

import { setOpenModal } from '@/lib/store/services/downloadModal';

import useUserPreferences from '@/core/hooks/useUserPreferences';
import useUserChecklists from '@/core/hooks/useUserChecklists';
import useInactivityLogout from '@/core/hooks/useInactivityLogout';
import useMaintenanceStatus from '@/core/hooks/useMaintenanceStatus';

const Layout = ({
pageTitle = 'AirQo Analytics',
Expand All @@ -36,6 +38,8 @@ const Layout = ({
const isCollapsed = useSelector((state) => state.sidebar.isCollapsed);
const isOpen = useSelector((state) => state.modal.openModal);

const { maintenance } = useMaintenanceStatus();

// Handler to close the modal
const handleCloseModal = () => {
dispatch(setOpenModal(false));
Expand Down Expand Up @@ -66,6 +70,9 @@ const Layout = ({
: 'max-w-[1200px] mx-auto space-y-8 px-4 py-8 sm:px-6 lg:px-8'
} overflow-hidden`}
>
{/* Maintenance Banner */}
<MaintenanceBanner maintenance={maintenance} />

{/* TopBar */}
{noTopNav && (
<TopBar
Expand Down
34 changes: 34 additions & 0 deletions platform/src/common/components/MaintenanceBanner/index.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
import React from 'react';
import { IoWarningOutline } from 'react-icons/io5';
import dayjs from 'dayjs';

const MaintenanceBanner = ({ maintenance }) => {
if (!maintenance?.isActive) return null;

Comment on lines +5 to +7
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add PropTypes and handle invalid maintenance object

The component lacks prop type validation and could crash with invalid date values.

+import PropTypes from 'prop-types';
+import { isValid } from 'date-fns';

 const MaintenanceBanner = ({ maintenance }) => {
-  if (!maintenance?.isActive) return null;
+  if (!maintenance?.isActive || !maintenance?.startDate || !maintenance?.endDate) return null;
+
+  const startDate = new Date(maintenance.startDate);
+  const endDate = new Date(maintenance.endDate);
+
+  if (!isValid(startDate) || !isValid(endDate)) {
+    console.warn('Invalid maintenance dates provided');
+    return null;
+  }

+MaintenanceBanner.propTypes = {
+  maintenance: PropTypes.shape({
+    isActive: PropTypes.bool.isRequired,
+    startDate: PropTypes.string.isRequired,
+    endDate: PropTypes.string.isRequired,
+    message: PropTypes.string.isRequired,
+  }),
+};
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const MaintenanceBanner = ({ maintenance }) => {
if (!maintenance?.isActive) return null;
import PropTypes from 'prop-types';
import { isValid } from 'date-fns';
const MaintenanceBanner = ({ maintenance }) => {
if (!maintenance?.isActive || !maintenance?.startDate || !maintenance?.endDate) return null;
const startDate = new Date(maintenance.startDate);
const endDate = new Date(maintenance.endDate);
if (!isValid(startDate) || !isValid(endDate)) {
console.warn('Invalid maintenance dates provided');
return null;
}
MaintenanceBanner.propTypes = {
maintenance: PropTypes.shape({
isActive: PropTypes.bool.isRequired,
startDate: PropTypes.string.isRequired,
endDate: PropTypes.string.isRequired,
message: PropTypes.string.isRequired,
}),
};

return (
<div className="bg-yellow-50 border-b border-yellow-200 mb-2">
<div className="py-3 px-3 sm:px-6 lg:px-8">
<div className="flex items-center justify-center">
<div className="flex items-center">
<span className="flex">
<IoWarningOutline
className="h-6 w-6 text-yellow-600"
aria-hidden="true"
/>
</span>
<p className="ml-3 font-medium text-yellow-700">
{maintenance.message}
<span className="ml-2 text-yellow-600">
(Estimated downtime:{' '}
{dayjs(maintenance.startDate).format('D MMM, YYYY hh:mm A')} -{' '}
{dayjs(maintenance.endDate).format('D MMM, YYYY hh:mm A')})
</span>
</p>
</div>
</div>
</div>
</div>
);
};

export default MaintenanceBanner;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Add test coverage for MaintenanceBanner component

The component is currently used in the Layout component and serves a clear purpose, but lacks test coverage. A test file should be added to ensure the component's reliability, especially since it handles maintenance state display.

  • Add test file at platform/src/common/components/MaintenanceBanner/MaintenanceBanner.test.jsx
  • Include test cases for:
    • Rendering with maintenance prop
    • Proper display of maintenance message
    • Component behavior when maintenance prop is empty/false
🔗 Analysis chain

Verify component usage and add tests

Let's ensure the component is properly integrated and tested:

  1. Verify all usage locations
  2. Add unit tests for the component
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for MaintenanceBanner usage
echo "Searching for MaintenanceBanner usage..."
rg "MaintenanceBanner" --type jsx --type tsx

# Check for existing test file
echo "Checking for test file..."
fd "MaintenanceBanner.test" --type file

Length of output: 264

Script:

#!/bin/bash
# Search for MaintenanceBanner usage with correct file extensions
echo "Searching for MaintenanceBanner usage..."
rg "MaintenanceBanner" -g "*.js" -g "*.jsx" -g "*.ts" -g "*.tsx"

# Check for test files in a more comprehensive way
echo "Checking for test files..."
fd -e test.js -e test.jsx -e spec.js -e spec.jsx "MaintenanceBanner"

# Look for the component definition to understand its location
echo "Finding component definition..."
rg "class MaintenanceBanner|function MaintenanceBanner|const MaintenanceBanner" -g "*.js" -g "*.jsx" -g "*.ts" -g "*.tsx"

Length of output: 1001

11 changes: 11 additions & 0 deletions platform/src/core/apis/Account.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
USER_CHECKLISTS_URL,
FORGOT_PWD_URL,
RESET_PWD_URL,
MAINTENANCE_STATUS_URL,
} from '../urls/authentication';
import axios from 'axios';
import createAxiosInstance from './axiosConfig';
Expand Down Expand Up @@ -221,3 +222,13 @@ export const updateGroupDetailsApi = async (groupID, data) => {
.put(`${GROUPS_URL}/${groupID}`, data)
.then((response) => response.data);
};

export const getMaintenanceStatus = async () => {
try {
const response = await createAxiosInstance().get(MAINTENANCE_STATUS_URL);
return response.data;
} catch (error) {
console.error('Error fetching maintenance status:', error);
throw error;
}
};
Comment on lines +226 to +234
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Enhance error handling and reliability

While the basic implementation is functional, consider these improvements:

  1. Add specific error handling for different failure scenarios
  2. Implement a retry mechanism for transient failures
  3. Specify response type for better type safety
 export const getMaintenanceStatus = async () => {
   try {
-    const response = await createAxiosInstance().get(MAINTENANCE_STATUS_URL);
+    const response = await createAxiosInstance().get<MaintenanceStatusResponse>(
+      MAINTENANCE_STATUS_URL,
+      {
+        retry: 3,
+        retryDelay: 1000,
+      }
+    );
     return response.data;
   } catch (error) {
-    console.error('Error fetching maintenance status:', error);
+    if (axios.isAxiosError(error)) {
+      if (error.response?.status === 404) {
+        console.error('Maintenance status endpoint not found');
+      } else if (error.response?.status >= 500) {
+        console.error('Server error while fetching maintenance status');
+      } else {
+        console.error('Network error:', error.message);
+      }
+    }
     throw error;
   }
 };

Also, consider defining a type for the maintenance status response:

interface MaintenanceStatusResponse {
  active: boolean;
  message?: string;
  startTime?: string;
  endTime?: string;
}

38 changes: 38 additions & 0 deletions platform/src/core/hooks/useMaintenanceStatus.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
import { useState, useEffect } from 'react';
import { getMaintenanceStatus } from '../apis/Account';

const POLLING_INTERVAL = 5 * 60 * 1000; // 5 minutes

const useMaintenanceStatus = () => {
const [maintenance, setMaintenance] = useState(null);
const [loading, setLoading] = useState(true);
const [error, setError] = useState(null);

const fetchMaintenanceStatus = async () => {
try {
const response = await getMaintenanceStatus();
if (response.success && response.maintenance?.length > 0) {
setMaintenance(response.maintenance[0]);
} else {
setMaintenance(null);
}
setError(null);
} catch (err) {
setError(err);
console.error('Error fetching maintenance status:', err);
} finally {
setLoading(false);
}
};
Comment on lines +11 to +26
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Enhance error handling and add type validation

The error handling could be more robust:

  1. No specific error types are being handled
  2. No validation of response data structure
  3. Console error in production code

Consider this enhanced implementation:

 const fetchMaintenanceStatus = async () => {
   try {
     const response = await getMaintenanceStatus();
+    // Validate response structure
+    if (!response || typeof response.success !== 'boolean') {
+      throw new Error('Invalid response format');
+    }
     if (response.success && response.maintenance?.length > 0) {
+      // Validate maintenance object structure
+      const maintenance = response.maintenance[0];
+      if (!maintenance.startDate || !maintenance.endDate || typeof maintenance.isActive !== 'boolean') {
+        throw new Error('Invalid maintenance data format');
+      }
       setMaintenance(response.maintenance[0]);
     } else {
       setMaintenance(null);
     }
     setError(null);
   } catch (err) {
     setError(err);
-    console.error('Error fetching maintenance status:', err);
+    // Use a proper logging service in production
+    if (process.env.NODE_ENV === 'development') {
+      console.error('Error fetching maintenance status:', err);
+    }
   } finally {
     setLoading(false);
   }
 };
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const fetchMaintenanceStatus = async () => {
try {
const response = await getMaintenanceStatus();
if (response.success && response.maintenance?.length > 0) {
setMaintenance(response.maintenance[0]);
} else {
setMaintenance(null);
}
setError(null);
} catch (err) {
setError(err);
console.error('Error fetching maintenance status:', err);
} finally {
setLoading(false);
}
};
const fetchMaintenanceStatus = async () => {
try {
const response = await getMaintenanceStatus();
// Validate response structure
if (!response || typeof response.success !== 'boolean') {
throw new Error('Invalid response format');
}
if (response.success && response.maintenance?.length > 0) {
// Validate maintenance object structure
const maintenance = response.maintenance[0];
if (!maintenance.startDate || !maintenance.endDate || typeof maintenance.isActive !== 'boolean') {
throw new Error('Invalid maintenance data format');
}
setMaintenance(response.maintenance[0]);
} else {
setMaintenance(null);
}
setError(null);
} catch (err) {
setError(err);
// Use a proper logging service in production
if (process.env.NODE_ENV === 'development') {
console.error('Error fetching maintenance status:', err);
}
} finally {
setLoading(false);
}
};


useEffect(() => {
fetchMaintenanceStatus();
const interval = setInterval(fetchMaintenanceStatus, POLLING_INTERVAL);

return () => clearInterval(interval);
}, []);
Comment on lines +28 to +33
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add cleanup for in-flight requests

Use AbortController to clean up pending requests when the component unmounts.

 useEffect(() => {
+  const abortController = new AbortController();
+
-  fetchMaintenanceStatus();
+  const fetchWithAbort = async () => {
+    try {
+      await fetchMaintenanceStatus({ signal: abortController.signal });
+    } catch (error) {
+      if (!error.name === 'AbortError') {
+        throw error;
+      }
+    }
+  };
+
+  fetchWithAbort();
   const interval = setInterval(fetchMaintenanceStatus, POLLING_INTERVAL);

-  return () => clearInterval(interval);
+  return () => {
+    clearInterval(interval);
+    abortController.abort();
+  };
 }, []);

Committable suggestion skipped: line range outside the PR's diff.


return { maintenance, loading, error };
};

export default useMaintenanceStatus;
7 changes: 3 additions & 4 deletions platform/src/core/urls/authentication.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,4 @@
import {
NEXT_PUBLIC_API_BASE_URL,
NEXT_PUBLIC_API_TOKEN,
} from '../../lib/envConstants';
import { NEXT_PUBLIC_API_BASE_URL } from '../../lib/envConstants';
import { stripTrailingSlash } from '../utils/strings';

const BASE_AUTH_URL = stripTrailingSlash(NEXT_PUBLIC_API_BASE_URL);
Expand Down Expand Up @@ -41,3 +38,5 @@ export const GENERATE_TOKEN_URI = `${AUTH_URL}/tokens`;
export const ACTIVATE_USER_CLIENT = `${CLIENT_URI}/activate`;

export const ACTIVATION_REQUEST_URI = `${CLIENT_URI}/activate-request`;

export const MAINTENANCE_STATUS_URL = `${AUTH_URL}/maintenances/analytics`;
Loading