Alex/bug bash: add alert when pick is updated #415
Conversation
|
Gridiron Survivor Application
Project name: Gridiron Survivor Application
Only deployments on the production branch are activated automatically. If you'd like to activate this deployment, navigate to your deployments. Learn more about Appwrite Function deployments.
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
alexappleget
changed the title
| * @returns {void} | ||
| */ | ||
| const onWeeklyPickChange = async ( | ||
| const handleWeeklyPickChange = async ( |
There was a problem hiding this comment.
Why was this name changed?
There was a problem hiding this comment.
i changed the name because the onWeeklyPickChange was decoupled and then imported. i didnt want to have:
const onWeeklyPickChange = async () => {
await onWeeklyPickChange(params);
}
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
| console.error('Submission error:', error); | ||
| } | ||
| const params = { | ||
| data, |
| * @returns {void} | ||
| */ | ||
| export const onWeeklyPickChange = async ({ | ||
| data, |
| updateWeeklyPicks, | ||
| setUserPick, | ||
| }: { | ||
| data: ChangeEvent<HTMLInputElement>; |
| import { AlertVariants } from '@/components/AlertNotification/Alerts.enum'; | ||
| import { toast } from 'react-hot-toast'; | ||
| import { onWeeklyPickChange } from './WeekHelper'; | ||
| import { createWeeklyPicks } from '../../../../../../api/apiFunctions'; |
There was a problem hiding this comment.
This should work. Test it out for all paths.
| import { createWeeklyPicks } from '../../../../../../api/apiFunctions'; | |
| import { createWeeklyPicks } from '@/api/apiFunctions'; |
There was a problem hiding this comment.
sweet it works now for that path
| [key: string]: any; // Adjust the 'any' to match the structure of your user results | ||
| } | ||
|
|
||
| interface IWeeklyPicks { |
There was a problem hiding this comment.
Why are we recreating the interfaces in a test?
There was a problem hiding this comment.
I had to create mock ones to define everything else in the tests
There was a problem hiding this comment.
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
| preventDefault: jest.fn(), | ||
| stopPropagation: jest.fn(), | ||
| } as any; | ||
| const NFLTeams = [{ teamName: 'Browns', teamId: 'mockId' }]; |
There was a problem hiding this comment.
For the ID, use a string number, just like the DB. It doens't have to be long, but it should be numbers.
| }, | ||
| }); | ||
|
|
||
| jest.mock('../../../../../../utils/utils', () => ({ |
There was a problem hiding this comment.
Why not put this in the parent so other test can use it too?
There was a problem hiding this comment.
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.
| const currentUserPick = mockParseUserPick(user.id, entry, teamID || ''); | |
| const currentUserPick = mockParseUserPick(user.id, entry, teamID); |
| expect(mockParseUserPick).toHaveBeenCalledWith( | ||
| user.id, | ||
| entry, | ||
| teamID || '', |
There was a problem hiding this comment.
| teamID || '', | |
| teamID, |
app/(main)/league/[leagueId]/entry/[entryId]/week/Week.test.tsx
Outdated
Show resolved
Hide resolved
…://github.com/LetsGetTechnical/gridiron-survivor into alex/414-bug-bash-add-alert-when-pick-is-upda
…add-alert-when-pick-is-upda
| IUser, | ||
| } from '@/api/apiFunctions.interface'; | ||
|
|
||
| export interface IData { |
There was a problem hiding this comment.
Make this more specific. IData is a generic name that could be for anything. Match the data structure. IWeekData is a much clearer name.
| updateWeeklyPicks: ({}: IWeeklyPicks) => void; | ||
| user: IUser; | ||
| weeklyPicks: IWeeklyPicks; | ||
| week: string; |
There was a problem hiding this comment.
Do we have an ENUM for the weeks?
There was a problem hiding this comment.
we do not. when i looked into it, it is just something that gets passed in as a string
There was a problem hiding this comment.
nit: We should look to adding it later. There's only week 1-18 available.
| weeklyPicks, | ||
| week, | ||
| }; | ||
| await onWeeklyPickChange(params); |
| />, | ||
| ); | ||
| } catch (error) { | ||
| console.error('Submission error:', error); |
There was a problem hiding this comment.
Change this to throw a new error and allow the function to catch the error.
There was a problem hiding this comment.
Is this basically so that we can more easily catch where the issue lies when it errors? So, the trycatch() being added into handleWeeklyPickChange() and then changing this console.error() will allow us to figure out where an issue lies?
…r inside onWeeklyPickChange(), and updated IData to IWeekData
| updateWeeklyPicks: ({}: IWeeklyPicks) => void; | ||
| user: IUser; | ||
| weeklyPicks: IWeeklyPicks; | ||
| week: string; |
There was a problem hiding this comment.
nit: We should look to adding it later. There's only week 1-18 available.
Added in Alert notifications for when users pick/update their teams
Closes: #414