From 6225c458390700b56049cd083b8bf96cd9c5133f Mon Sep 17 00:00:00 2001 From: Dev Singh Date: Mon, 27 Jan 2025 12:11:30 -0600 Subject: [PATCH 1/8] enable edit profile support --- src/common/types/msGraphApi.ts | 13 ++++++++ src/ui/Router.tsx | 5 +++ src/ui/components/AppShell/index.tsx | 4 +-- src/ui/components/AuthGuard/index.tsx | 21 ++++++------- src/ui/components/ProfileDropdown/index.tsx | 12 +++++++ src/ui/config.ts | 20 +++++++++++- src/ui/pages/events/ViewEvents.page.tsx | 5 ++- src/ui/pages/profile/ManageProfile.page.tsx | 25 +++++++++++++++ .../pages/profile/ManageProfileComponent.tsx | 31 +++++++++++++++++++ 9 files changed, 120 insertions(+), 16 deletions(-) create mode 100644 src/common/types/msGraphApi.ts create mode 100644 src/ui/pages/profile/ManageProfile.page.tsx create mode 100644 src/ui/pages/profile/ManageProfileComponent.tsx diff --git a/src/common/types/msGraphApi.ts b/src/common/types/msGraphApi.ts new file mode 100644 index 00000000..7f009f12 --- /dev/null +++ b/src/common/types/msGraphApi.ts @@ -0,0 +1,13 @@ +export interface UserProfileData { + id?: string; + userPrincipalName: string; + displayName?: string; + givenName?: string; + surname?: string; + mail?: string; + jobTitle?: string; + mobilePhone?: string; + officeLocation?: string; + preferredLanguage?: string; + businessPhones?: string[]; +} diff --git a/src/ui/Router.tsx b/src/ui/Router.tsx index 1258db86..9c68c7a5 100644 --- a/src/ui/Router.tsx +++ b/src/ui/Router.tsx @@ -17,6 +17,7 @@ import { ScanTicketsPage } from './pages/tickets/ScanTickets.page'; import { SelectTicketsPage } from './pages/tickets/SelectEventId.page'; import { ViewTicketsPage } from './pages/tickets/ViewTickets.page'; import { ManageIamPage } from './pages/iam/ManageIam.page'; +import { ManageProfilePage } from './pages/profile/ManageProfile.page'; // Component to handle redirects to login with return path const LoginRedirect: React.FC = () => { @@ -87,6 +88,10 @@ const authenticatedRouter = createBrowserRouter([ path: '/logout', element: , }, + { + path: '/profile', + element: , + }, { path: '/home', element: , diff --git a/src/ui/components/AppShell/index.tsx b/src/ui/components/AppShell/index.tsx index c5a4abaa..867184f6 100644 --- a/src/ui/components/AppShell/index.tsx +++ b/src/ui/components/AppShell/index.tsx @@ -25,7 +25,7 @@ import { HeaderNavbar } from '../Navbar/index.js'; import { AuthenticatedProfileDropdown } from '../ProfileDropdown/index.js'; import { getCurrentRevision } from '@ui/util/revision.js'; -interface AcmAppShellProps { +export interface AcmAppShellProps { children: ReactNode; active?: string; showLoader?: boolean; @@ -164,7 +164,7 @@ const AcmAppShell: React.FC = ({ padding="md" header={{ height: 60 }} navbar={{ - width: 200, + width: showSidebar ? 200 : 0, breakpoint: 'sm', collapsed: { mobile: !opened }, }} diff --git a/src/ui/components/AuthGuard/index.tsx b/src/ui/components/AuthGuard/index.tsx index 0634013f..76b58b55 100644 --- a/src/ui/components/AuthGuard/index.tsx +++ b/src/ui/components/AuthGuard/index.tsx @@ -1,7 +1,7 @@ import { Card, Text, Title } from '@mantine/core'; import React, { ReactNode, useEffect, useState } from 'react'; -import { AcmAppShell } from '@ui/components/AppShell'; +import { AcmAppShell, AcmAppShellProps } from '@ui/components/AppShell'; import FullScreenLoader from '@ui/components/AuthContext/LoadingScreen'; import { getRunEnvironmentConfig, ValidService } from '@ui/config'; import { useApi } from '@ui/util/api'; @@ -60,11 +60,13 @@ export const clearAuthCache = () => { } }; -export const AuthGuard: React.FC<{ - resourceDef: ResourceDefinition; - children: ReactNode; - isAppShell?: boolean; -}> = ({ resourceDef, children, isAppShell = true }) => { +export const AuthGuard: React.FC< + { + resourceDef: ResourceDefinition; + children: ReactNode; + isAppShell?: boolean; + } & AcmAppShellProps +> = ({ resourceDef, children, isAppShell = true, ...appShellProps }) => { const { service, validRoles } = resourceDef; const { baseEndpoint, authCheckRoute, friendlyName } = getRunEnvironmentConfig().ServiceConfiguration[service]; @@ -163,12 +165,7 @@ export const AuthGuard: React.FC<{ } if (isAppShell) { - return ( - - {friendlyName} - {children} - - ); + return {children}; } return <>{children}; diff --git a/src/ui/components/ProfileDropdown/index.tsx b/src/ui/components/ProfileDropdown/index.tsx index 1fe8f0e6..71760e8b 100644 --- a/src/ui/components/ProfileDropdown/index.tsx +++ b/src/ui/components/ProfileDropdown/index.tsx @@ -18,6 +18,7 @@ import { useState } from 'react'; import { AuthContextData, useAuth } from '../AuthContext/index.js'; import classes from '../Navbar/index.module.css'; +import { useNavigate } from 'react-router-dom'; interface ProfileDropdownProps { userData?: AuthContextData; @@ -26,6 +27,7 @@ interface ProfileDropdownProps { const AuthenticatedProfileDropdown: React.FC = ({ userData }) => { const [opened, setOpened] = useState(false); const theme = useMantineTheme(); + const navigate = useNavigate(); const { logout } = useAuth(); if (!userData) { return null; @@ -111,6 +113,16 @@ const AuthenticatedProfileDropdown: React.FC = ({ userData + + + + )), + ...toAdd.map((email) => ( + + + + +
+ + {email.split('@')[0]} + + + {email} + +
+ + + + + Queued for addition + + + + + + + )), + ]; + return ( - - +
+ Exec Council Group Management - {/* Member List */} - - - Current Members - - - {isLoading && } - {!isLoading && ( - - {members.map((member) => ( - - - - - {member.name} ({member.email}) - - {toRemove.includes(member.email) && ( - - Queued for removal - - )} - - handleRemoveMember(member.email)} - data-testid={`remove-exec-member-${member.email}`} - > - - - - - ))} - {toAdd.map((member) => ( - - - - {member} - - Queued for addition - - - - - ))} - - )} - - + {isLoading ? ( + + ) : ( + + + + Member + Status + Actions + + + {rows} +
+ )} - {/* Add Member */} - - setEmail(e.currentTarget.value)} - placeholder="Enter email" - label="Add Member" - /> - - + setEmail(e.currentTarget.value)} + placeholder="Enter email" + label="Add Member" + mt="md" + /> + - {/* Save Changes Button */} - {/* Confirmation Modal */} setConfirmationModal(false)} title="Confirm Changes" > - +
{toAdd.length > 0 && ( - - +
+ Members to Add: - - {toAdd.map((email) => ( - {email} - ))} - - + {toAdd.map((email) => ( + {email} + ))} +
)} {toRemove.length > 0 && ( - - +
+ Members to Remove: - - {toRemove.map((email) => ( - {email} - ))} - - + {toRemove.map((email) => ( + {email} + ))} +
)} - +
- +
); }; diff --git a/src/ui/pages/iam/ManageIam.page.tsx b/src/ui/pages/iam/ManageIam.page.tsx index e0e3b6a4..5a60d2e4 100644 --- a/src/ui/pages/iam/ManageIam.page.tsx +++ b/src/ui/pages/iam/ManageIam.page.tsx @@ -10,6 +10,7 @@ import { GroupMemberGetResponse, GroupModificationPatchRequest, } from '@common/types/iam'; +import { transformCommaSeperatedName } from '@common/utils'; import { getRunEnvironmentConfig } from '@ui/config'; export const ManageIamPage = () => { @@ -37,7 +38,13 @@ export const ManageIamPage = () => { const getExecMembers = async () => { try { const response = await api.get(`/api/v1/iam/groups/${groupId}`); - return response.data as GroupMemberGetResponse; + const responseMapped = response.data + .map((x: any) => ({ + ...x, + name: transformCommaSeperatedName(x.name), + })) + .sort((x: any, y: any) => (x.name > y.name ? 1 : x.name < y.name ? -1 : 0)); + return responseMapped as GroupMemberGetResponse; } catch (error: any) { console.error('Failed to get users:', error); return []; From 4ed132145f410b2405d58e5a714294326abb77c6 Mon Sep 17 00:00:00 2001 From: Dev Singh Date: Mon, 27 Jan 2025 14:30:00 -0600 Subject: [PATCH 3/8] force reroute to profile if we dont know their first and last name --- src/common/types/msGraphApi.ts | 13 +- src/ui/Router.tsx | 42 +++++- src/ui/components/AuthContext/index.tsx | 35 +++-- src/ui/components/AuthGuard/index.tsx | 4 + src/ui/pages/Login.page.tsx | 24 ++- src/ui/pages/profile/ManageProfile.page.tsx | 41 ++++- .../pages/profile/ManageProfileComponent.tsx | 141 ++++++++++++++++-- 7 files changed, 252 insertions(+), 48 deletions(-) diff --git a/src/common/types/msGraphApi.ts b/src/common/types/msGraphApi.ts index 7f009f12..934034d3 100644 --- a/src/common/types/msGraphApi.ts +++ b/src/common/types/msGraphApi.ts @@ -1,13 +1,12 @@ -export interface UserProfileData { - id?: string; +export interface UserProfileDataBase { userPrincipalName: string; displayName?: string; givenName?: string; surname?: string; mail?: string; - jobTitle?: string; - mobilePhone?: string; - officeLocation?: string; - preferredLanguage?: string; - businessPhones?: string[]; + otherMails?: string[] +} + +export interface UserProfileData extends UserProfileDataBase { + discordUsername?: string; } diff --git a/src/ui/Router.tsx b/src/ui/Router.tsx index 9c68c7a5..0ff4bd48 100644 --- a/src/ui/Router.tsx +++ b/src/ui/Router.tsx @@ -19,6 +19,28 @@ import { ViewTicketsPage } from './pages/tickets/ViewTickets.page'; import { ManageIamPage } from './pages/iam/ManageIam.page'; import { ManageProfilePage } from './pages/profile/ManageProfile.page'; +const ProfileRediect: React.FC = () => { + const location = useLocation(); + + // Don't store login-related paths and ALLOW the callback path + const excludedPaths = [ + '/login', + '/logout', + '/force_login', + '/a', + '/auth/callback', // Add this to excluded paths + ]; + + if (excludedPaths.includes(location.pathname)) { + return ; + } + + // Include search params and hash in the return URL if they exist + const returnPath = location.pathname + location.search + location.hash; + const loginUrl = `/profile?returnTo=${encodeURIComponent(returnPath)}&firstTime=true`; + return ; +}; + // Component to handle redirects to login with return path const LoginRedirect: React.FC = () => { const location = useLocation(); @@ -57,6 +79,18 @@ const commonRoutes = [ }, ]; +const profileRouter = createBrowserRouter([ + ...commonRoutes, + { + path: '/profile', + element: , + }, + { + path: '*', + element: , + }, +]); + const unauthenticatedRouter = createBrowserRouter([ ...commonRoutes, { @@ -67,7 +101,6 @@ const unauthenticatedRouter = createBrowserRouter([ path: '/login', element: , }, - // Catch-all route that preserves the attempted path { path: '*', element: , @@ -168,7 +201,12 @@ const ErrorBoundary: React.FC = ({ children }) => { export const Router: React.FC = () => { const { isLoggedIn } = useAuth(); - const router = isLoggedIn ? authenticatedRouter : unauthenticatedRouter; + console.log(isLoggedIn); + const router = isLoggedIn + ? authenticatedRouter + : isLoggedIn === null + ? profileRouter + : unauthenticatedRouter; return ( diff --git a/src/ui/components/AuthContext/index.tsx b/src/ui/components/AuthContext/index.tsx index c805b401..1f0251c9 100644 --- a/src/ui/components/AuthContext/index.tsx +++ b/src/ui/components/AuthContext/index.tsx @@ -19,6 +19,8 @@ import { CACHE_KEY_PREFIX } from '../AuthGuard/index.js'; import FullScreenLoader from './LoadingScreen.js'; import { getRunEnvironmentConfig, ValidServices } from '@ui/config.js'; +import { transformCommaSeperatedName } from '@common/utils.js'; +import { useApi } from '@ui/util/api.js'; interface AuthContextDataWrapper { isLoggedIn: boolean; @@ -28,6 +30,7 @@ interface AuthContextDataWrapper { getToken: CallableFunction; logoutCallback: CallableFunction; getApiToken: CallableFunction; + setLoginStatus: CallableFunction; } export type AuthContextData = { @@ -53,7 +56,6 @@ export const clearAuthCache = () => { export const AuthProvider: React.FC = ({ children }) => { const { instance, inProgress, accounts } = useMsal(); - const [userData, setUserData] = useState(null); const [isLoggedIn, setIsLoggedIn] = useState(false); @@ -67,11 +69,9 @@ export const AuthProvider: React.FC = ({ children }) => { if (response) { handleMsalResponse(response); } else if (accounts.length > 0) { - // User is already logged in, set the state - const [lastName, firstName] = accounts[0].name?.split(',') || []; setUserData({ email: accounts[0].username, - name: `${firstName} ${lastName}`, + name: transformCommaSeperatedName(accounts[0].name || ''), }); setIsLoggedIn(true); } @@ -94,10 +94,9 @@ export const AuthProvider: React.FC = ({ children }) => { }) .then((silentResponse) => { if (silentResponse?.account?.name) { - const [lastName, firstName] = silentResponse.account.name.split(','); setUserData({ - email: silentResponse.account.username, - name: `${firstName} ${lastName}`, + email: accounts[0].username, + name: transformCommaSeperatedName(accounts[0].name || ''), }); setIsLoggedIn(true); } @@ -105,18 +104,16 @@ export const AuthProvider: React.FC = ({ children }) => { .catch(console.error); return; } - - // Use response.account instead of accounts[0] - const [lastName, firstName] = response.account.name?.split(',') || []; setUserData({ - email: response.account.username, - name: `${firstName} ${lastName}`, + email: accounts[0].username, + name: transformCommaSeperatedName(accounts[0].name || ''), }); setIsLoggedIn(true); } }, [accounts, instance] ); + const getApiToken = useCallback( async (service: ValidServices) => { if (!userData) { @@ -194,6 +191,9 @@ export const AuthProvider: React.FC = ({ children }) => { }, [instance] ); + const setLoginStatus = useCallback((val: boolean) => { + setIsLoggedIn(val); + }, []); const logout = useCallback(async () => { try { @@ -209,7 +209,16 @@ export const AuthProvider: React.FC = ({ children }) => { }; return ( {inProgress !== InteractionStatus.None ? ( diff --git a/src/ui/components/AuthGuard/index.tsx b/src/ui/components/AuthGuard/index.tsx index 76b58b55..070aa631 100644 --- a/src/ui/components/AuthGuard/index.tsx +++ b/src/ui/components/AuthGuard/index.tsx @@ -82,6 +82,10 @@ export const AuthGuard: React.FC< setIsAuthenticated(true); return; } + if (validRoles.length === 0) { + setIsAuthenticated(true); + return; + } // Check for cached response first const cachedData = getCachedResponse(service, authCheckRoute); diff --git a/src/ui/pages/Login.page.tsx b/src/ui/pages/Login.page.tsx index b75dc5be..b6a28add 100644 --- a/src/ui/pages/Login.page.tsx +++ b/src/ui/pages/Login.page.tsx @@ -5,19 +5,33 @@ import { Center, Alert } from '@mantine/core'; import { IconAlertCircle, IconAlertTriangle } from '@tabler/icons-react'; import { useEffect } from 'react'; import { useNavigate, useSearchParams } from 'react-router-dom'; +import { useApi } from '@ui/util/api'; export function LoginPage() { const navigate = useNavigate(); - const { isLoggedIn } = useAuth(); + const graphApi = useApi('msGraphApi'); + const { isLoggedIn, setLoginStatus } = useAuth(); const [searchParams] = useSearchParams(); const showLogoutMessage = searchParams.get('lc') === 'true'; const showLoginMessage = !showLogoutMessage && searchParams.get('li') === 'true'; useEffect(() => { - if (isLoggedIn) { - const returnTo = searchParams.get('returnTo'); - navigate(returnTo || '/home'); - } + const evalState = async () => { + if (isLoggedIn) { + const returnTo = searchParams.get('returnTo'); + const me = (await graphApi.get('/v1.0/me?$select=givenName,surname')).data as { + givenName?: string; + surname?: string; + }; + if (!me.givenName || !me.surname) { + setLoginStatus(null); + navigate(`/profile?firstTime=true${returnTo ? `&returnTo=${returnTo}` : ''}`); + } else { + navigate(returnTo || '/home'); + } + } + }; + evalState(); }, [navigate, isLoggedIn, searchParams]); return ( diff --git a/src/ui/pages/profile/ManageProfile.page.tsx b/src/ui/pages/profile/ManageProfile.page.tsx index 7afbcada..4be9f730 100644 --- a/src/ui/pages/profile/ManageProfile.page.tsx +++ b/src/ui/pages/profile/ManageProfile.page.tsx @@ -1,25 +1,52 @@ import React from 'react'; -import { Title } from '@mantine/core'; +import { Container, Title } from '@mantine/core'; import { AuthGuard } from '@ui/components/AuthGuard'; import { useApi } from '@ui/util/api'; -import { UserProfileData } from '@common/types/msGraphApi'; +import { UserProfileData, UserProfileDataBase } from '@common/types/msGraphApi'; import { ManageProfileComponent } from './ManageProfileComponent'; +import { useSearchParams } from 'react-router-dom'; export const ManageProfilePage: React.FC = () => { const api = useApi('msGraphApi'); - + const [searchParams] = useSearchParams(); + const returnTo = searchParams.get('returnTo') || undefined; + const firstTime = searchParams.get('firstTime') === 'true' || false; const getProfile = async () => { - return (await api.get('/v1.0/me')).data as UserProfileData; + const raw = ( + await api.get( + '/v1.0/me?$select=userPrincipalName,givenName,surname,displayName,otherMails,mail' + ) + ).data as UserProfileDataBase; + const discordUsername = raw.otherMails?.filter((x) => x.endsWith('@discord')); + const enhanced = raw as UserProfileData; + if (discordUsername?.length === 1) { + enhanced.discordUsername = discordUsername[0].replace('@discord', ''); + enhanced.otherMails = enhanced.otherMails?.filter((x) => !x.endsWith('@discord')); + } + return enhanced; }; const setProfile = async (data: UserProfileData) => { + const newOtherEmails = [data.mail || data.userPrincipalName]; + if (data.discordUsername && data.discordUsername !== '') { + newOtherEmails.push(`${data.discordUsername}@discord`); + } + data.otherMails = newOtherEmails; + delete data.discordUsername; return (await api.patch('/v1.0/me', data)).data; }; return ( - - Edit Profile - + + + Edit Profile + + ); }; diff --git a/src/ui/pages/profile/ManageProfileComponent.tsx b/src/ui/pages/profile/ManageProfileComponent.tsx index f9001c52..7cb7136e 100644 --- a/src/ui/pages/profile/ManageProfileComponent.tsx +++ b/src/ui/pages/profile/ManageProfileComponent.tsx @@ -1,31 +1,144 @@ import React, { useEffect, useState } from 'react'; -import { Title, SimpleGrid, Container } from '@mantine/core'; -import { AuthGuard } from '@ui/components/AuthGuard'; -import { useApi } from '@ui/util/api'; -import { AppRoles } from '@common/roles'; -import { getRunEnvironmentConfig } from '@ui/config'; +import { TextInput, Button, Group, Box, LoadingOverlay, Alert } from '@mantine/core'; import { UserProfileData } from '@common/types/msGraphApi'; +import { useAuth } from '@ui/components/AuthContext'; +import { notifications } from '@mantine/notifications'; +import { useNavigate } from 'react-router-dom'; +import { IconMoodSmileBeam, IconQuestionMark } from '@tabler/icons-react'; interface ManageProfileComponentProps { getProfile: () => Promise; setProfile: (data: UserProfileData) => Promise; + firstTime: boolean; + returnTo?: string; } export const ManageProfileComponent: React.FC = ({ getProfile, setProfile, + firstTime, + returnTo, }) => { + const { userData, setLoginStatus } = useAuth(); + const navigate = useNavigate(); const [userProfile, setUserProfile] = useState(undefined); + const [loading, setLoading] = useState(false); + const fetchProfile = async () => { + setLoading(true); + try { + const profile = await getProfile(); + setUserProfile(profile); + } catch (e) { + console.error(e); + setUserProfile(null); + notifications.show({ + color: 'red', + message: 'Failed to load user profile', + }); + } finally { + setLoading(false); + } + }; useEffect(() => { - const fetchProfile = async () => { - try { - setUserProfile(await getProfile()); - } catch (e) { - console.error(e); - setUserProfile(null); - } - }; fetchProfile(); }, [getProfile]); - return null; + + const handleSubmit = async () => { + if (!userProfile) return; + setLoading(true); + try { + await setProfile(userProfile); + notifications.show({ + color: 'green', + title: 'Profile updated successfully', + message: 'Changes may take some time to reflect.', + }); + setLoginStatus(true); + if (returnTo) { + return navigate(returnTo); + } + await fetchProfile(); + } catch (e) { + console.error(e); + notifications.show({ + color: 'red', + message: 'Failed to update profile', + }); + } finally { + setLoading(false); + } + }; + + if (userProfile === undefined) { + return ; + } + + return ( + <> + {firstTime && ( + } + title="Welcome to ACM @ UIUC Management Portal" + color="yellow" + > + Your profile is incomplete. Please provide us with the information below and click Save. + + )} + +
{ + e.preventDefault(); + handleSubmit(); + }} + > + + setUserProfile((prev) => prev && { ...prev, displayName: e.target.value }) + } + placeholder={userData?.name} + required + /> + + setUserProfile((prev) => prev && { ...prev, givenName: e.target.value }) + } + placeholder={userProfile?.givenName} + required + /> + setUserProfile((prev) => prev && { ...prev, surname: e.target.value })} + placeholder={userProfile?.surname} + required + /> + setUserProfile((prev) => prev && { ...prev, mail: e.target.value })} + placeholder={userData?.email} + required + /> + + + setUserProfile((prev) => prev && { ...prev, discordUsername: e.target.value }) + } + /> + + + + + +
+ + ); }; From 451c7a8c7bd6d68343b95ecfd68bc139a40d1ed1 Mon Sep 17 00:00:00 2001 From: Dev Singh Date: Mon, 27 Jan 2025 14:36:59 -0600 Subject: [PATCH 4/8] keep the redir behavior outside of the component --- src/ui/pages/profile/ManageProfile.page.tsx | 15 ++++++++++++--- src/ui/pages/profile/ManageProfileComponent.tsx | 15 ++++----------- 2 files changed, 16 insertions(+), 14 deletions(-) diff --git a/src/ui/pages/profile/ManageProfile.page.tsx b/src/ui/pages/profile/ManageProfile.page.tsx index 4be9f730..b22da761 100644 --- a/src/ui/pages/profile/ManageProfile.page.tsx +++ b/src/ui/pages/profile/ManageProfile.page.tsx @@ -4,10 +4,13 @@ import { AuthGuard } from '@ui/components/AuthGuard'; import { useApi } from '@ui/util/api'; import { UserProfileData, UserProfileDataBase } from '@common/types/msGraphApi'; import { ManageProfileComponent } from './ManageProfileComponent'; -import { useSearchParams } from 'react-router-dom'; +import { useNavigate, useSearchParams } from 'react-router-dom'; +import { useAuth } from '@ui/components/AuthContext'; export const ManageProfilePage: React.FC = () => { const api = useApi('msGraphApi'); + const { setLoginStatus } = useAuth(); + const navigate = useNavigate(); const [searchParams] = useSearchParams(); const returnTo = searchParams.get('returnTo') || undefined; const firstTime = searchParams.get('firstTime') === 'true' || false; @@ -33,7 +36,14 @@ export const ManageProfilePage: React.FC = () => { } data.otherMails = newOtherEmails; delete data.discordUsername; - return (await api.patch('/v1.0/me', data)).data; + const response = await api.patch('/v1.0/me', data); + if (response.status < 299 && firstTime) { + setLoginStatus(true); + } + if (returnTo) { + return navigate(returnTo); + } + return response.data; }; return ( @@ -44,7 +54,6 @@ export const ManageProfilePage: React.FC = () => { getProfile={getProfile} setProfile={setProfile} firstTime={firstTime} - returnTo={returnTo} /> diff --git a/src/ui/pages/profile/ManageProfileComponent.tsx b/src/ui/pages/profile/ManageProfileComponent.tsx index 7cb7136e..e214fd08 100644 --- a/src/ui/pages/profile/ManageProfileComponent.tsx +++ b/src/ui/pages/profile/ManageProfileComponent.tsx @@ -1,25 +1,21 @@ import React, { useEffect, useState } from 'react'; import { TextInput, Button, Group, Box, LoadingOverlay, Alert } from '@mantine/core'; import { UserProfileData } from '@common/types/msGraphApi'; -import { useAuth } from '@ui/components/AuthContext'; import { notifications } from '@mantine/notifications'; import { useNavigate } from 'react-router-dom'; -import { IconMoodSmileBeam, IconQuestionMark } from '@tabler/icons-react'; +import { IconMoodSmileBeam } from '@tabler/icons-react'; interface ManageProfileComponentProps { getProfile: () => Promise; setProfile: (data: UserProfileData) => Promise; firstTime: boolean; - returnTo?: string; } export const ManageProfileComponent: React.FC = ({ getProfile, setProfile, firstTime, - returnTo, }) => { - const { userData, setLoginStatus } = useAuth(); const navigate = useNavigate(); const [userProfile, setUserProfile] = useState(undefined); const [loading, setLoading] = useState(false); @@ -53,10 +49,6 @@ export const ManageProfileComponent: React.FC = ({ title: 'Profile updated successfully', message: 'Changes may take some time to reflect.', }); - setLoginStatus(true); - if (returnTo) { - return navigate(returnTo); - } await fetchProfile(); } catch (e) { console.error(e); @@ -97,7 +89,7 @@ export const ManageProfileComponent: React.FC = ({ onChange={(e) => setUserProfile((prev) => prev && { ...prev, displayName: e.target.value }) } - placeholder={userData?.name} + placeholder={userProfile?.displayName} required /> = ({ label="Email" value={userProfile?.mail || ''} onChange={(e) => setUserProfile((prev) => prev && { ...prev, mail: e.target.value })} - placeholder={userData?.email} + placeholder={userProfile?.mail} required + disabled /> Date: Mon, 27 Jan 2025 14:51:33 -0600 Subject: [PATCH 5/8] tests that fail --- .../profile/ManageProfileComponent.test.tsx | 180 ++++++++++++++++++ .../pages/profile/ManageProfileComponent.tsx | 2 +- 2 files changed, 181 insertions(+), 1 deletion(-) create mode 100644 src/ui/pages/profile/ManageProfileComponent.test.tsx diff --git a/src/ui/pages/profile/ManageProfileComponent.test.tsx b/src/ui/pages/profile/ManageProfileComponent.test.tsx new file mode 100644 index 00000000..39f6e362 --- /dev/null +++ b/src/ui/pages/profile/ManageProfileComponent.test.tsx @@ -0,0 +1,180 @@ +import React from 'react'; +import { render, screen, act } from '@testing-library/react'; +import { MemoryRouter } from 'react-router-dom'; +import { MantineProvider } from '@mantine/core'; +import { notifications } from '@mantine/notifications'; +import userEvent from '@testing-library/user-event'; +import { vi } from 'vitest'; +import { ManageProfileComponent } from './ManageProfileComponent'; + +describe('ManageProfileComponent tests', () => { + const renderComponent = async ( + getProfile: () => Promise, + setProfile: (data: any) => Promise, + firstTime: boolean = false + ) => { + await act(async () => { + render( + + + + + + ); + }); + }; + + beforeEach(() => { + vi.clearAllMocks(); + }); + + it('renders loading overlay when fetching profile', async () => { + const getProfile = vi.fn().mockResolvedValue(new Promise(() => {})); // Never resolves + const setProfile = vi.fn(); + + await renderComponent(getProfile, setProfile); + + expect(screen.getByText(/Loading.../i)).toBeInTheDocument(); + }); + + it('renders profile form after successfully fetching profile', async () => { + const getProfile = vi.fn().mockResolvedValue({ + displayName: 'John Doe', + givenName: 'John', + surname: 'Doe', + mail: 'john.doe@example.com', + discordUsername: 'johndoe#1234', + }); + const setProfile = vi.fn(); + + await renderComponent(getProfile, setProfile); + + expect(screen.getByLabelText('Display Name')).toHaveValue('John Doe'); + expect(screen.getByLabelText('First Name')).toHaveValue('John'); + expect(screen.getByLabelText('Last Name')).toHaveValue('Doe'); + expect(screen.getByLabelText('Email')).toHaveValue('john.doe@example.com'); + expect(screen.getByLabelText('Discord Username')).toHaveValue('johndoe#1234'); + }); + + it('handles profile fetch failure gracefully', async () => { + const notificationsMock = vi.spyOn(notifications, 'show'); + const getProfile = vi.fn().mockRejectedValue(new Error('Failed to fetch profile')); + const setProfile = vi.fn(); + + await renderComponent(getProfile, setProfile); + + expect(screen.getByText(/Failed to load user profile/i)).toBeInTheDocument(); + expect(notificationsMock).toHaveBeenCalledWith( + expect.objectContaining({ + message: 'Failed to load user profile', + color: 'red', + }) + ); + + notificationsMock.mockRestore(); + }); + + it('allows editing profile fields and saving changes', async () => { + const notificationsMock = vi.spyOn(notifications, 'show'); + const getProfile = vi.fn().mockResolvedValue({ + displayName: 'John Doe', + givenName: 'John', + surname: 'Doe', + mail: 'john.doe@example.com', + discordUsername: '', + }); + const setProfile = vi.fn().mockResolvedValue({}); + + await renderComponent(getProfile, setProfile); + + const user = userEvent.setup(); + + // Edit fields + await user.clear(screen.getByLabelText('Display Name')); + await user.type(screen.getByLabelText('Display Name'), 'Jane Doe'); + await user.type(screen.getByLabelText('Discord Username'), 'janedoe#5678'); + + // Save changes + const saveButton = screen.getByRole('button', { name: 'Save' }); + await user.click(saveButton); + + expect(setProfile).toHaveBeenCalledWith({ + displayName: 'Jane Doe', + givenName: 'John', + surname: 'Doe', + mail: 'john.doe@example.com', + discordUsername: 'janedoe#5678', + }); + + expect(notificationsMock).toHaveBeenCalledWith( + expect.objectContaining({ + title: 'Profile updated successfully', + message: 'Changes may take some time to reflect.', + color: 'green', + }) + ); + + notificationsMock.mockRestore(); + }); + + it('shows first-time user alert when `firstTime` is true', async () => { + const getProfile = vi.fn().mockResolvedValue({ + displayName: '', + givenName: '', + surname: '', + mail: 'new.user@example.com', + discordUsername: '', + }); + const setProfile = vi.fn(); + + await renderComponent(getProfile, setProfile, true); + + expect( + screen.getByText( + 'Your profile is incomplete. Please provide us with the information below and click Save.' + ) + ).toBeInTheDocument(); + }); + + it('handles profile update failure gracefully', async () => { + const notificationsMock = vi.spyOn(notifications, 'show'); + const getProfile = vi.fn().mockResolvedValue({ + displayName: 'John Doe', + givenName: 'John', + surname: 'Doe', + mail: 'john.doe@example.com', + discordUsername: '', + }); + const setProfile = vi.fn().mockRejectedValue(new Error('Failed to update profile')); + + await renderComponent(getProfile, setProfile); + + const user = userEvent.setup(); + + // Attempt to save without any changes + const saveButton = screen.getByRole('button', { name: 'Save' }); + await user.click(saveButton); + + expect(notificationsMock).toHaveBeenCalledWith( + expect.objectContaining({ + message: 'Failed to update profile', + color: 'red', + }) + ); + + notificationsMock.mockRestore(); + }); + + it('disables the save button when no profile data is loaded', async () => { + const getProfile = vi.fn().mockResolvedValue(null); + const setProfile = vi.fn(); + + await renderComponent(getProfile, setProfile); + + expect(screen.getByRole('button', { name: 'Save' })).toBeDisabled(); + }); +}); diff --git a/src/ui/pages/profile/ManageProfileComponent.tsx b/src/ui/pages/profile/ManageProfileComponent.tsx index e214fd08..54204c27 100644 --- a/src/ui/pages/profile/ManageProfileComponent.tsx +++ b/src/ui/pages/profile/ManageProfileComponent.tsx @@ -126,7 +126,7 @@ export const ManageProfileComponent: React.FC = ({ /> - From 332bbf5a321fd90260a5f7da72f920a9c333111d Mon Sep 17 00:00:00 2001 From: Dev Singh Date: Mon, 27 Jan 2025 23:16:19 +0000 Subject: [PATCH 6/8] fix the tests --- src/ui/Router.tsx | 1 - .../AuthContext/AuthCallbackHandler.page.tsx | 5 ----- .../profile/ManageProfileComponent.test.tsx | 19 +++++++++---------- .../pages/profile/ManageProfileComponent.tsx | 7 ++++++- 4 files changed, 15 insertions(+), 17 deletions(-) diff --git a/src/ui/Router.tsx b/src/ui/Router.tsx index 0ff4bd48..c9a6e253 100644 --- a/src/ui/Router.tsx +++ b/src/ui/Router.tsx @@ -201,7 +201,6 @@ const ErrorBoundary: React.FC = ({ children }) => { export const Router: React.FC = () => { const { isLoggedIn } = useAuth(); - console.log(isLoggedIn); const router = isLoggedIn ? authenticatedRouter : isLoggedIn === null diff --git a/src/ui/components/AuthContext/AuthCallbackHandler.page.tsx b/src/ui/components/AuthContext/AuthCallbackHandler.page.tsx index 5eb6c49e..57ca19e8 100644 --- a/src/ui/components/AuthContext/AuthCallbackHandler.page.tsx +++ b/src/ui/components/AuthContext/AuthCallbackHandler.page.tsx @@ -34,11 +34,6 @@ export const AuthCallback: React.FC = () => { setTimeout(() => { handleCallback(); }, 100); - - // Cleanup function - return () => { - console.log('Callback component unmounting'); // Debug log 8 - }; }, [instance, navigate]); return ; diff --git a/src/ui/pages/profile/ManageProfileComponent.test.tsx b/src/ui/pages/profile/ManageProfileComponent.test.tsx index 39f6e362..f715f0f1 100644 --- a/src/ui/pages/profile/ManageProfileComponent.test.tsx +++ b/src/ui/pages/profile/ManageProfileComponent.test.tsx @@ -38,7 +38,7 @@ describe('ManageProfileComponent tests', () => { await renderComponent(getProfile, setProfile); - expect(screen.getByText(/Loading.../i)).toBeInTheDocument(); + expect(screen.getByTestId('profile-loading')).toBeInTheDocument(); }); it('renders profile form after successfully fetching profile', async () => { @@ -53,11 +53,11 @@ describe('ManageProfileComponent tests', () => { await renderComponent(getProfile, setProfile); - expect(screen.getByLabelText('Display Name')).toHaveValue('John Doe'); - expect(screen.getByLabelText('First Name')).toHaveValue('John'); - expect(screen.getByLabelText('Last Name')).toHaveValue('Doe'); - expect(screen.getByLabelText('Email')).toHaveValue('john.doe@example.com'); - expect(screen.getByLabelText('Discord Username')).toHaveValue('johndoe#1234'); + expect(screen.getByTestId('edit-displayName')).toHaveValue('John Doe'); + expect(screen.getByTestId('edit-firstName')).toHaveValue('John'); + expect(screen.getByTestId('edit-lastName')).toHaveValue('Doe'); + expect(screen.getByTestId('edit-email')).toHaveValue('john.doe@example.com'); + expect(screen.getByTestId('edit-discordUsername')).toHaveValue('johndoe#1234'); }); it('handles profile fetch failure gracefully', async () => { @@ -67,7 +67,6 @@ describe('ManageProfileComponent tests', () => { await renderComponent(getProfile, setProfile); - expect(screen.getByText(/Failed to load user profile/i)).toBeInTheDocument(); expect(notificationsMock).toHaveBeenCalledWith( expect.objectContaining({ message: 'Failed to load user profile', @@ -94,9 +93,9 @@ describe('ManageProfileComponent tests', () => { const user = userEvent.setup(); // Edit fields - await user.clear(screen.getByLabelText('Display Name')); - await user.type(screen.getByLabelText('Display Name'), 'Jane Doe'); - await user.type(screen.getByLabelText('Discord Username'), 'janedoe#5678'); + await user.clear(screen.getByTestId('edit-displayName')); + await user.type(screen.getByTestId('edit-displayName'), 'Jane Doe'); + await user.type(screen.getByTestId('edit-discordUsername'), 'janedoe#5678'); // Save changes const saveButton = screen.getByRole('button', { name: 'Save' }); diff --git a/src/ui/pages/profile/ManageProfileComponent.tsx b/src/ui/pages/profile/ManageProfileComponent.tsx index 54204c27..9f4423bd 100644 --- a/src/ui/pages/profile/ManageProfileComponent.tsx +++ b/src/ui/pages/profile/ManageProfileComponent.tsx @@ -62,7 +62,7 @@ export const ManageProfileComponent: React.FC = ({ }; if (userProfile === undefined) { - return ; + return ; } return ( @@ -91,6 +91,7 @@ export const ManageProfileComponent: React.FC = ({ } placeholder={userProfile?.displayName} required + data-testId="edit-displayName" /> = ({ } placeholder={userProfile?.givenName} required + data-testId="edit-firstName" /> = ({ onChange={(e) => setUserProfile((prev) => prev && { ...prev, surname: e.target.value })} placeholder={userProfile?.surname} required + data-testId="edit-lastName" /> = ({ placeholder={userProfile?.mail} required disabled + data-testId="edit-email" /> = ({ onChange={(e) => setUserProfile((prev) => prev && { ...prev, discordUsername: e.target.value }) } + data-testId="edit-discordUsername" /> From 244b2f32ffb5c888b2e3c0d61c148fbb1a7c6412 Mon Sep 17 00:00:00 2001 From: Dev Singh Date: Mon, 27 Jan 2025 18:35:06 -0600 Subject: [PATCH 7/8] fix test --- tests/e2e/events.spec.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tests/e2e/events.spec.ts b/tests/e2e/events.spec.ts index 2b6bad0b..c1b28ac7 100644 --- a/tests/e2e/events.spec.ts +++ b/tests/e2e/events.spec.ts @@ -9,9 +9,7 @@ describe("Events tests", () => { }) => { await becomeUser(page); await page.locator("a").filter({ hasText: "Events" }).click(); - await expect(page.getByRole("heading")).toContainText( - "Core Management Service (NonProd)", - ); + await expect(page.getByRole("heading")).toContainText("Event Management"); await expect( page.getByRole("button", { name: "New Calendar Event" }), ).toBeVisible(); From 2abb16ea3312040a5ebf1bb535dd3f14c04e7c08 Mon Sep 17 00:00:00 2001 From: Dev Singh Date: Mon, 27 Jan 2025 18:39:23 -0600 Subject: [PATCH 8/8] fix tests part 7! --- tests/e2e/login.spec.ts | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/tests/e2e/login.spec.ts b/tests/e2e/login.spec.ts index e0aeab52..b0fa3b27 100644 --- a/tests/e2e/login.spec.ts +++ b/tests/e2e/login.spec.ts @@ -20,14 +20,11 @@ describe("Login tests", () => { page.getByRole("link", { name: "ACM Logo Management Portal" }), ).toBeVisible(); await expect( - page.getByRole("link", { name: "P", exact: true }), + page.getByRole("link", { name: "PC", exact: true }), ).toBeVisible(); - await page.getByRole("link", { name: "P", exact: true }).click(); - await expect(page.getByLabel("PMy Account")).toContainText( - "Name Playwright Core User", - ); - await expect(page.getByLabel("PMy Account")).toContainText( - "Emailcore-e2e-testing@acm.illinois.edu", + await page.getByRole("link", { name: "PC", exact: true }).click(); + await expect(page.getByLabel("PCMy Account")).toContainText( + "NamePlaywright Core UserEmailcore-e2e-testing@acm.illinois.eduEdit ProfileLog Out", ); expect(page.url()).toEqual("https://manage.qa.acmuiuc.org/home"); });