-
Notifications
You must be signed in to change notification settings - Fork 1
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
Alex/bug bash: add alert when pick is updated #415
Changes from 11 commits
5f705d6
138a3d1
1c5c905
d78d63f
2066acf
b59489c
e154977
de6cad1
a264ede
29ee3ce
9e0f3c6
0b0e05d
e4ee42d
1e89efb
11d5a02
1236a94
1f26a02
f885dfe
9b95801
d739aa1
4e9475b
94eb8cb
8bd9efb
cd074b3
baa8836
4933a40
1473a7a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,137 @@ | ||||||
import React from 'react'; | ||||||
import Alert from '@/components/AlertNotification/AlertNotification'; | ||||||
import { AlertVariants } from '@/components/AlertNotification/Alerts.enum'; | ||||||
import { toast } from 'react-hot-toast'; | ||||||
import { onWeeklyPickChange } from './WeekHelper'; | ||||||
import { createWeeklyPicks } from '../../../../../../api/apiFunctions'; | ||||||
import { parseUserPick } from '../../../../../../utils/utils'; | ||||||
|
||||||
jest.mock('../../../../../../api/apiFunctions', () => ({ | ||||||
createWeeklyPicks: jest.fn(), | ||||||
})); | ||||||
|
||||||
jest.mock('react-hot-toast', () => ({ | ||||||
toast: { | ||||||
custom: jest.fn(), | ||||||
}, | ||||||
})); | ||||||
|
||||||
describe('Week', () => { | ||||||
const data = { | ||||||
target: { value: 'Browns' }, | ||||||
preventDefault: jest.fn(), | ||||||
stopPropagation: jest.fn(), | ||||||
} as any; | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is just a test, but we should have a type for this already. |
||||||
const NFLTeams = [{ teamName: 'Browns', teamId: 'mockId' }]; | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For the ID, use a string number, just like the DB. It doens't have to be long, but it should be numbers. |
||||||
const user = { id: 'mockUserId', email: '[email protected]', leagues: [] }; | ||||||
const entry = 'mockEntry'; | ||||||
const league = 'mockLeague'; | ||||||
const week = 'mockWeek'; | ||||||
const updateWeeklyPicks = jest.fn(); | ||||||
const setUserPick = jest.fn(); | ||||||
interface IUserResults { | ||||||
[key: string]: any; // Adjust the 'any' to match the structure of your user results | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here. This is opening up a can of worms. Because we strictly type everything, there should be a type that matches this. |
||||||
} | ||||||
|
||||||
interface IWeeklyPicks { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why are we recreating the interfaces in a test? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I had to create mock ones to define everything else in the tests There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. oh sorry i understand now and fixed it. I keep thinking that in tests you have to explain EVERYTHING to it. I didn't realize you can easily import IWeeklyPicks and use it inside the test easily |
||||||
leagueId: string; | ||||||
gameWeekId: string; | ||||||
userResults: IUserResults; | ||||||
} | ||||||
|
||||||
const weeklyPicks: IWeeklyPicks = { | ||||||
leagueId: 'mockId', | ||||||
gameWeekId: 'mockGameId', | ||||||
userResults: {}, | ||||||
}; | ||||||
|
||||||
const teamID = NFLTeams[0].teamName; | ||||||
|
||||||
const updatedWeeklyPicks = { | ||||||
...weeklyPicks.userResults, | ||||||
[user.id]: { | ||||||
...weeklyPicks.userResults[user.id], | ||||||
[entry]: { | ||||||
...weeklyPicks.userResults[user.id]?.[entry], | ||||||
...parseUserPick(user.id, entry, teamID || '')[user.id][entry], | ||||||
}, | ||||||
}, | ||||||
}; | ||||||
beforeEach(() => { | ||||||
jest.clearAllMocks(); | ||||||
}); | ||||||
|
||||||
test('should show success notification after changing your team pick', async () => { | ||||||
(createWeeklyPicks as jest.Mock).mockResolvedValue({}); | ||||||
|
||||||
const mockParseUserPick = jest.fn().mockReturnValue({ | ||||||
[user.id]: { | ||||||
[entry]: { | ||||||
teamName: 'Browns', | ||||||
}, | ||||||
}, | ||||||
}); | ||||||
|
||||||
jest.mock('../../../../../../utils/utils', () => ({ | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why not put this in the parent so other test can use it too? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I had it inside the test itself because it was only used for that test and defined inside it. I changed things and moved it all in the parent |
||||||
parseUserPick: mockParseUserPick, | ||||||
})); | ||||||
|
||||||
const currentUserPick = mockParseUserPick(user.id, entry, teamID || ''); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
|
||||||
await onWeeklyPickChange({ | ||||||
data, | ||||||
NFLTeams, | ||||||
user, | ||||||
entry, | ||||||
weeklyPicks, | ||||||
league, | ||||||
week, | ||||||
updateWeeklyPicks, | ||||||
setUserPick, | ||||||
}); | ||||||
|
||||||
expect(createWeeklyPicks).toHaveBeenCalledWith({ | ||||||
leagueId: league, | ||||||
gameWeekId: week, | ||||||
userResults: updatedWeeklyPicks, | ||||||
}); | ||||||
|
||||||
expect(mockParseUserPick).toHaveBeenCalledWith( | ||||||
user.id, | ||||||
entry, | ||||||
teamID || '', | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
); | ||||||
|
||||||
expect(toast.custom).toHaveBeenCalledWith( | ||||||
<Alert | ||||||
variant={AlertVariants.Success} | ||||||
message={`You have successfully pick the ${ | ||||||
currentUserPick[user.id][entry].teamName | ||||||
} for your team!`} | ||||||
/>, | ||||||
); | ||||||
}); | ||||||
|
||||||
test('should show error notification when changing your team fails', async () => { | ||||||
(createWeeklyPicks as jest.Mock).mockRejectedValue(new Error('error')); | ||||||
|
||||||
await onWeeklyPickChange({ | ||||||
data, | ||||||
NFLTeams, | ||||||
user, | ||||||
entry, | ||||||
weeklyPicks, | ||||||
league, | ||||||
week, | ||||||
updateWeeklyPicks, | ||||||
setUserPick, | ||||||
}); | ||||||
|
||||||
expect(toast.custom).toHaveBeenCalledWith( | ||||||
<Alert | ||||||
variant={AlertVariants.Error} | ||||||
message="There was an error processing your request." | ||||||
/>, | ||||||
); | ||||||
}); | ||||||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,8 +12,6 @@ import { | |
import { FormProvider, Control, useForm } from 'react-hook-form'; | ||
import { z } from 'zod'; | ||
import { IWeekProps } from './Week.interface'; | ||
import { createWeeklyPicks } from '@/api/apiFunctions'; | ||
import { parseUserPick } from '@/utils/utils'; | ||
import { zodResolver } from '@hookform/resolvers/zod'; | ||
import { useDataStore } from '@/store/dataStore'; | ||
import { ISchedule } from './WeekTeams.interface'; | ||
|
@@ -22,6 +20,7 @@ import { ChevronLeft } from 'lucide-react'; | |
import { getCurrentLeague } from '@/api/apiFunctions'; | ||
import { ILeague } from '@/api/apiFunctions.interface'; | ||
import WeekTeams from './WeekTeams'; | ||
import { onWeeklyPickChange } from './WeekHelper'; | ||
|
||
/** | ||
* Renders the weekly picks page. | ||
|
@@ -82,52 +81,25 @@ const Week = ({ entry, league, NFLTeams, week }: IWeekProps): JSX.Element => { | |
}); | ||
|
||
/** | ||
* Handles the form submission. | ||
* @param data - The form data. | ||
* Handles the weekly picks | ||
* @param data - data of the pick | ||
* @returns {void} | ||
*/ | ||
const onWeeklyPickChange = async ( | ||
const handleWeeklyPickChange = async ( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why was this name changed? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i changed the name because the onWeeklyPickChange was decoupled and then imported. i didnt want to have: const onWeeklyPickChange = async () => { won't that cause confusion to vs code? i did the same with loginAccount I even changed it back to 'onWeeklyPickChange' and it just breaks things. I have to name the parent something different than the decoupled function I have inside I can do a few things depending on what you want. Keep it as onWeeklyPickChange BUT I would have to change the decoupled function name. or I can leave it as it is There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Makes sense. |
||
data: ChangeEvent<HTMLInputElement>, | ||
): Promise<void> => { | ||
try { | ||
const teamSelect = data.target.value; | ||
const teamID = NFLTeams.find( | ||
(team) => team.teamName === teamSelect, | ||
)?.teamName; | ||
|
||
const currentUserPick = parseUserPick(user.id, entry, teamID || ''); | ||
|
||
// combines current picks and the user pick into one object. | ||
// if the user pick exists then it overrides the pick of the user. | ||
const updatedWeeklyPicks = { | ||
...weeklyPicks.userResults, | ||
[user.id]: { | ||
...weeklyPicks.userResults[user.id], | ||
[entry]: { | ||
...weeklyPicks.userResults[user.id]?.[entry], | ||
...currentUserPick[user.id][entry], | ||
}, | ||
}, | ||
}; | ||
|
||
// update weekly picks in the database | ||
await createWeeklyPicks({ | ||
leagueId: league, | ||
gameWeekId: week, | ||
userResults: updatedWeeklyPicks, | ||
}); | ||
|
||
// update weekly picks in the data store | ||
updateWeeklyPicks({ | ||
leagueId: league, | ||
gameWeekId: week, | ||
userResults: updatedWeeklyPicks, | ||
}); | ||
|
||
setUserPick(currentUserPick[user.id][entry].teamName); | ||
} catch (error) { | ||
console.error('Submission error:', error); | ||
} | ||
const params = { | ||
data, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Alphabetize the list |
||
NFLTeams, | ||
user, | ||
entry, | ||
weeklyPicks, | ||
league, | ||
week, | ||
updateWeeklyPicks, | ||
setUserPick, | ||
}; | ||
await onWeeklyPickChange(params); | ||
}; | ||
|
||
useEffect(() => { | ||
|
@@ -173,7 +145,7 @@ const Week = ({ entry, league, NFLTeams, week }: IWeekProps): JSX.Element => { | |
schedule={schedule} | ||
field={field} | ||
userPick={userPick} | ||
onWeeklyPickChange={onWeeklyPickChange} | ||
onWeeklyPickChange={handleWeeklyPickChange} | ||
/> | ||
</FormControl> | ||
<FormMessage /> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,103 @@ | ||
// Copyright (c) Gridiron Survivor. | ||
// Licensed under the MIT License. | ||
|
||
import React, { ChangeEvent } from 'react'; | ||
import { createWeeklyPicks } from '@/api/apiFunctions'; | ||
import { parseUserPick } from '@/utils/utils'; | ||
import Alert from '@/components/AlertNotification/AlertNotification'; | ||
import { AlertVariants } from '@/components/AlertNotification/Alerts.enum'; | ||
import { toast } from 'react-hot-toast'; | ||
import { INFLTeam } from '@/api/apiFunctions.interface'; | ||
import { IUser } from '@/api/apiFunctions.interface'; | ||
import { IWeeklyPicks } from '@/api/apiFunctions.interface'; | ||
|
||
/** | ||
* Handles the form submission. | ||
* @param props - data, NFLTeams, user, entry, weeklyPicks, league, week, updateWeeklyPicks, setUserPick | ||
* @param props.data - The form data. | ||
* @param props.NFLTeams - Props for NFL teams | ||
* @param props.user - Props for user | ||
* @param props.entry - Prop for the entry string | ||
* @param props.weeklyPicks - Props for the weeklyPicks | ||
* @param props.league - Prop value for the leagueId in createWeeklyPicks | ||
* @param props.week - Prop value for gameWeekId in updateWeeklyPicks | ||
* @param props.updateWeeklyPicks - Prop for the updateWeeklyPicks function | ||
* @param props.setUserPick - Prop for the setUserPick function | ||
* @returns {void} | ||
*/ | ||
export const onWeeklyPickChange = async ({ | ||
data, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Alphabetize the list |
||
NFLTeams, | ||
user, | ||
entry, | ||
weeklyPicks, | ||
league, | ||
week, | ||
updateWeeklyPicks, | ||
setUserPick, | ||
}: { | ||
data: ChangeEvent<HTMLInputElement>; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Make this an Interface |
||
NFLTeams: INFLTeam[]; | ||
user: IUser; | ||
entry: string; | ||
weeklyPicks: IWeeklyPicks; | ||
league: string; | ||
week: string; | ||
updateWeeklyPicks: ({}: IWeeklyPicks) => void; | ||
setUserPick: React.Dispatch<React.SetStateAction<string>>; | ||
}): Promise<void> => { | ||
try { | ||
const teamSelect = data.target.value; | ||
const teamID = NFLTeams.find( | ||
(team) => team.teamName === teamSelect, | ||
)?.teamName; | ||
|
||
const currentUserPick = parseUserPick(user.id, entry, teamID || ''); | ||
|
||
// combines current picks and the user pick into one object. | ||
// if the user pick exists then it overrides the pick of the user. | ||
const updatedWeeklyPicks = { | ||
...weeklyPicks.userResults, | ||
[user.id]: { | ||
...weeklyPicks.userResults[user.id], | ||
[entry]: { | ||
...weeklyPicks.userResults[user.id]?.[entry], | ||
...currentUserPick[user.id][entry], | ||
}, | ||
}, | ||
}; | ||
|
||
// update weekly picks in the database | ||
await createWeeklyPicks({ | ||
leagueId: league, | ||
gameWeekId: week, | ||
userResults: updatedWeeklyPicks, | ||
}); | ||
|
||
// update weekly picks in the data store | ||
updateWeeklyPicks({ | ||
leagueId: league, | ||
gameWeekId: week, | ||
userResults: updatedWeeklyPicks, | ||
}); | ||
|
||
setUserPick(currentUserPick[user.id][entry].teamName); | ||
|
||
toast.custom( | ||
<Alert | ||
variant={AlertVariants.Success} | ||
message={`You have successfully pick the ${ | ||
currentUserPick[user.id][entry].teamName | ||
} for your team!`} | ||
/>, | ||
); | ||
} catch (error) { | ||
console.error('Submission error:', error); | ||
toast.custom( | ||
<Alert | ||
variant={AlertVariants.Error} | ||
message="There was an error processing your request." | ||
/>, | ||
); | ||
} | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should work. Test it out for all paths.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sweet it works now for that path