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.

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

enhance: reporting tool enhancements and fixes #2176

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
4 changes: 4 additions & 0 deletions reports/next.config.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ const nextConfig = {
protocol: 'https',
hostname: 'res.cloudinary.com',
},
{
protocol: 'https',
hostname: 'res.cloudinary.com',
},
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Remove duplicate entry in remotePatterns array

Hey there! I noticed we've got a bit of a doppelganger situation in our remotePatterns array. It looks like we accidentally added an identical entry for the Cloudinary hostname. While I appreciate the enthusiasm for Cloudinary (it's pretty great, right?), we only need one entry for each unique pattern.

Let's clean this up by removing the duplicate. Here's a quick fix:

 images: {
   remotePatterns: [
     {
       protocol: 'https',
       hostname: 'res.cloudinary.com',
     },
-    {
-      protocol: 'https',
-      hostname: 'res.cloudinary.com',
-    },
   ],
 },

This should keep our config nice and tidy without changing any functionality. What do you think?

📝 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
{
protocol: 'https',
hostname: 'res.cloudinary.com',
},
{
protocol: 'https',
hostname: 'res.cloudinary.com',
},

],
},
async redirects() {
Expand Down
2 changes: 1 addition & 1 deletion reports/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "reports",
"version": "0.5.1",
"version": "0.5.2",
"private": true,
"scripts": {
"dev": "next dev",
Expand Down
Binary file added reports/public/images/section1.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added reports/public/images/section2.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added reports/public/images/section3.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
18 changes: 18 additions & 0 deletions reports/public/svgs/NoInternetSVG.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import React from 'react';

interface NoInternetSVGProps {
className?: string;
}

const NoInternetSVG: React.FC<NoInternetSVGProps> = ({ className }) => (
<svg
className={className}
xmlns="http://www.w3.org/2000/svg"
viewBox="0 0 512 512"
fill="currentColor"
>
<path d="M256 48C141.1 48 48 141.1 48 256s93.1 208 208 208 208-93.1 208-208S370.9 48 256 48zm0 384c-97.2 0-176-78.8-176-176S158.8 80 256 80s176 78.8 176 176-78.8 176-176 176zm95.8-272H160.2c-9.6 0-18.4 5.8-22.2 14.8s-1.8 19.6 5.4 26.6l95.8 95.8c7 7 16.2 10.8 25.8 10.8s18.8-3.8 25.8-10.8l95.8-95.8c7.2-7 9-17.6 5.4-26.6s-12.6-14.8-22.2-14.8zm-95.8 160c-13.3 0-24-10.7-24-24s10.7-24 24-24 24 10.7 24 24-10.7 24-24 24z" />
</svg>
);

export default NoInternetSVG;
36 changes: 17 additions & 19 deletions reports/src/app/AppProvider.tsx
Original file line number Diff line number Diff line change
@@ -1,39 +1,37 @@
'use client';
import { SessionProvider } from 'next-auth/react';
import { ThemeProvider as NextThemesProvider, useTheme } from 'next-themes';
import { ThemeProviderProps } from 'next-themes/dist/types';
import { useRef, useEffect, useState } from 'react';
import { Provider } from 'react-redux';

import { AppStore, makeStore } from '../lib/store';
import { SessionProvider } from 'next-auth/react';
import { ThemeProvider as NextThemesProvider } from 'next-themes';
import { type ThemeProviderProps } from 'next-themes/dist/types';
import { useTheme } from 'next-themes';
// import Loading from "./loading";

const AppProvider = ({
const AppProvider: React.FC<React.PropsWithChildren<ThemeProviderProps>> = ({
children,
...props
}: {
children: React.ReactNode;
} & ThemeProviderProps) => {
const storeRef = useRef<AppStore>();
if (!storeRef.current) {
storeRef.current = makeStore();
}

}) => {
const storeRef = useRef<AppStore | null>(null);
const { theme } = useTheme();
const [mounted, setMounted] = useState(false);

// Initialize store
useEffect(() => {
document.body.classList.toggle('dark', theme === 'dark');
}, [theme]);

useEffect(() => {
storeRef.current = makeStore();
setMounted(true);
}, []);

// Update body class based on theme
useEffect(() => {
if (mounted) {
document.body.classList.toggle('dark', theme === 'dark');
}
}, [theme, mounted]);

if (!mounted) return null;

return (
<Provider store={storeRef.current}>
<Provider store={storeRef.current!}>
<SessionProvider basePath="/reports/api/auth">
<NextThemesProvider {...props}>{children}</NextThemesProvider>
</SessionProvider>
Expand Down
69 changes: 69 additions & 0 deletions reports/src/app/NetworkStatus.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
'use client';
import React, { useState, useEffect } from 'react';

import { useNetworkChecker } from '@/hooks/useNetworkChecker';
import NoInternetSVG from '@/public/svgs/NoInternetSVG';

const NetworkStatus: React.FC<React.PropsWithChildren> = ({ children }) => {
const [retrying, setRetrying] = useState(false);
const [retryTimer, setRetryTimer] = useState<number | null>(null);
const isOnline = useNetworkChecker();

const handleRetry = () => {
setRetrying(true);
setRetryTimer(setTimeout(() => window.location.reload(), 5000) as unknown as number);
};

const cancelRetry = () => {
if (retryTimer) {
clearTimeout(retryTimer);
setRetryTimer(null);
setRetrying(false);
}
};
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 retry logic and improve type safety.

The retry logic is well-implemented, but there are a couple of areas for improvement:

  1. Consider using a constant for the retry delay to improve maintainability:

    const RETRY_DELAY_MS = 5000;
  2. The type casting in handleRetry can be avoided by using window.setTimeout instead of setTimeout:

    setRetryTimer(window.setTimeout(() => window.location.reload(), RETRY_DELAY_MS));

These changes will make the code more maintainable and type-safe.


useEffect(() => {
return () => {
if (retryTimer) clearTimeout(retryTimer);
};
}, [retryTimer]);

if (!isOnline) {
return (
<div className="flex justify-center items-center h-screen bg-gray-50 px-4">
<div className="text-center max-w-lg">
<NoInternetSVG className="w-64 h-64 mx-auto mb-6" />
<h1 className="text-3xl md:text-4xl text-blue-800 font-bold mb-4">
Oops! No Internet Connection
</h1>
<p className="text-lg mb-6 text-gray-700">
It seems you are offline. Please check your connection and try again.
</p>
<div className="space-x-4">
<button
onClick={handleRetry}
className="px-6 py-3 bg-blue-700 text-white rounded-md transition hover:bg-blue-800 focus:outline-none focus:ring-4 focus:ring-blue-700 focus:ring-opacity-50 disabled:opacity-50"
aria-label="Retry connection"
disabled={retrying}
>
{retrying ? 'Retrying in 5s...' : 'Retry Now'}
</button>
{retrying && (
<button
onClick={cancelRetry}
className="px-6 py-3 bg-gray-300 text-gray-800 rounded-md transition hover:bg-gray-400 focus:outline-none focus:ring-4 focus:ring-gray-400 focus:ring-opacity-50"
aria-label="Cancel retry"
>
Cancel
</button>
)}
</div>
</div>
</div>
);
}

return <>{children}</>;
};

export default NetworkStatus;
64 changes: 44 additions & 20 deletions reports/src/app/api/auth/[...nextauth]/options.ts
Original file line number Diff line number Diff line change
@@ -1,18 +1,31 @@
import axios from 'axios';
import { NextAuthOptions, User as NextAuthUser } from 'next-auth';
import CredentialsProvider from 'next-auth/providers/credentials';

export const options = {
// Define types for credentials, user, and token
interface Credentials {
email?: string;
password?: string;
}

Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Refine the 'Credentials' interface by requiring fields

Defining the Credentials interface clarifies the structure of the credentials object. However, since both email and password are required for authentication, it's better to make these fields mandatory.

Consider updating the interface to:

interface Credentials {
-  email?: string;
-  password?: string;
+  email: string;
+  password: string;
}

This change enforces at the type level that both fields must be provided.

📝 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
// Define types for credentials, user, and token
interface Credentials {
email?: string;
password?: string;
}
// Define types for credentials, user, and token
interface Credentials {
email: string;
password: string;
}

interface CustomUser extends NextAuthUser {
_id: string;
userName: string;
token: string;
}

export const options: NextAuthOptions = {
providers: [
CredentialsProvider({
id: 'credentials',
name: 'Credentials',
credentials: {},
async authorize(credentials: { email?: string; password?: string } | undefined) {
if (!credentials) {
throw new Error('No credentials provided');
async authorize(credentials: Credentials | undefined) {
if (!credentials || !credentials.email || !credentials.password) {
throw new Error('Please provide both email and password.');
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Avoid throwing errors directly in authentication flow

Throwing an error with throw new Error('Please provide both email and password.'); may expose internal error messages. Instead, returning null signals an authentication failure without revealing details.

Consider modifying the code:

if (!credentials || !credentials.email || !credentials.password) {
-  throw new Error('Please provide both email and password.');
+  return null;
}

Ensure the client handles the null response appropriately to inform the user.

📝 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
if (!credentials || !credentials.email || !credentials.password) {
throw new Error('Please provide both email and password.');
if (!credentials || !credentials.email || !credentials.password) {
return null;

}

const { email: userName = '', password = '' } = credentials;
const { email: userName, password } = credentials;

try {
const url = `${process.env.NEXT_PUBLIC_API_URL}/users/loginUser`;
Expand All @@ -22,12 +35,19 @@ export const options = {
});

if (response) {
return response;
return {
_id: response._id,
userName: response.userName,
email: response.email,
token: response.token,
} as CustomUser;
}

throw new Error('User not found');
return null;
} catch (error: any) {
throw new Error(error.response.data.message || error.message);
throw new Error(
error?.response?.data?.message || 'An error occurred during login. Please try again.',
);
}
},
}),
Expand All @@ -39,22 +59,26 @@ export const options = {
},
secret: process.env.NEXTAUTH_SECRET,
callbacks: {
jwt: async ({ token, user }: any) => {
jwt: async ({ token, user }) => {
if (user) {
token.id = user._id;
token.userName = user.userName;
token.email = user.email;
token.accessToken = user.token;
const customUser = user as CustomUser;
token.id = customUser._id;
token.userName = customUser.userName;
token.email = customUser.email;
token.accessToken = customUser.token;
}
return token;
},
session: async ({ session, token }: { session: any; token: any }) => {
session.user.id = token._id;
session.user.userName = token.userName;
session.user.email = token.email;
session.accessToken = token.accessToken;

return session;
session: async ({ session, token }) => {
return {
...session,
user: {
id: token.id as string,
userName: token.userName as string,
email: token.email as string,
},
accessToken: token.accessToken as string,
};
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Sensitive data in session object

Including accessToken in the session object means it's sent to the client, which may lead to security vulnerabilities. Tokens should be stored securely and not exposed unless absolutely necessary.

Consider removing accessToken from the session:

return {
  ...session,
  user: {
    id: token.id as string,
    userName: token.userName as string,
    email: token.email as string,
  },
-  accessToken: token.accessToken as string,
};

If the token is needed on the client, evaluate using HTTP-only cookies or other secure storage mechanisms to prevent unauthorized access.

📝 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
session: async ({ session, token }) => {
return {
...session,
user: {
id: token.id as string,
userName: token.userName as string,
email: token.email as string,
},
accessToken: token.accessToken as string,
};
session: async ({ session, token }) => {
return {
...session,
user: {
id: token.id as string,
userName: token.userName as string,
email: token.email as string,
},
};
},

},
},
};
1 change: 1 addition & 0 deletions reports/src/app/api/auth/[...nextauth]/route.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import NextAuth from 'next-auth';

import { options } from './options';

const handler = NextAuth(options);
Expand Down
40 changes: 27 additions & 13 deletions reports/src/app/error.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
'use client';
import React, { useEffect } from 'react';
import Image from 'next/image';
import { useRouter } from 'next/navigation';
import React, { useEffect } from 'react';

import { Button } from '@/components/ui/button';
import ErrorBoundaryImage from '@/public/images/ErrorBoundary.png';

Expand All @@ -11,34 +12,47 @@ interface ErrorProps {

const ErrorComponent: React.FC<ErrorProps> = ({ error }) => {
const router = useRouter();

useEffect(() => {
console.error(error);
logError(error);
}, [error]);

// Utility function to log errors, can be extended for error tracking services
const logError = (error: Error) => {
console.error(error);
// integrate error tracking services like Sentry, LogRocket, etc.
};

const handleRetry = () => {
router.push('/home');
};

return (
<div className="flex flex-col items-center justify-center min-h-screen bg-gray-100">
<div className="p-4 space-y-8 text-center">
<div className="flex items-center justify-center w-auto h-auto p-3 mx-auto bg-blue-100 rounded-full">
<div className="flex flex-col items-center justify-center min-h-screen px-4 bg-gray-50">
<div className="p-6 space-y-8 text-center bg-white rounded-md shadow-lg">
<div className="flex items-center justify-center w-48 h-48 p-3 mx-auto bg-blue-100 rounded-full">
<Image
src={ErrorBoundaryImage}
alt="icon"
alt="Error occurred"
width={200}
height={200}
className="mix-blend-multiply"
priority
/>
</div>
<div className="space-y-8">
<h2 className="text-2xl font-semibold text-gray-900">Oops! Something went wrong</h2>
<p className="text-lg text-gray-500 max-w-lg">
We&apos;re sorry for the inconvenience. Our team has been notified and we&apos;re
working on a fix. Please try again later.
<div className="space-y-4">
<h2 className="text-3xl font-bold text-gray-900">Oops! Something Went Wrong</h2>
<p className="text-lg text-gray-500 max-w-md mx-auto">
We&apos;re sorry for the inconvenience. Our team has been notified and is working on a
fix. Please try again later or return to the home page.
</p>
</div>
<div>
<Button
type="button"
onClick={() => router.push('/home')}
className="px-6 py-3 text-lg font-medium text-white bg-blue-700 rounded hover:bg-blue-800"
onClick={handleRetry}
className="px-6 py-3 text-lg font-medium text-white bg-blue-600 rounded-lg hover:bg-blue-700 focus:outline-none focus:ring-4 focus:ring-blue-300 transition ease-in-out"
aria-label="Go back to the home page"
>
Go Home
</Button>
Expand Down
6 changes: 6 additions & 0 deletions reports/src/app/files/page.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import Files from '@/components/pages/home/Files';

const Page = () => {
return <Files />;
};
export default Page;
12 changes: 12 additions & 0 deletions reports/src/app/help/page.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import HelpPage from '@/components/pages/help';
import MainLayout from '@/layout/MainLayout';

const Help = () => {
return (
<MainLayout>
<HelpPage />
</MainLayout>
);
};

export default Help;
Loading
Loading