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

feat: add lockout period #526

Closed
wants to merge 9 commits into from
Closed

feat: add lockout period #526

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
50256fe
start making lockout work
ryandotfurrer Sep 12, 2024
5e399f6
rework of make pick/change pick button into CustomLink and expanded C…
ryandotfurrer Sep 12, 2024
b65d83d
move checkLockout logic to LeagueEntries component
ryandotfurrer Sep 13, 2024
796689f
begin working on updating tests
ryandotfurrer Sep 13, 2024
bd2f277
Merge branch 'develop' into ryan/add-lockout-period
choir241 Sep 16, 2024
82a127e
fix linkCustom test
ryandotfurrer Sep 17, 2024
5c326ec
add prop to customlink for data-test-id and fix a test on the league …
ryandotfurrer Sep 17, 2024
335b30b
Fix LeagueEntries test
ryandotfurrer Sep 18, 2024
6ea046c
update now variable name to currentDateAndTime
ryandotfurrer Sep 18, 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
8 changes: 4 additions & 4 deletions app/(main)/league/[leagueId]/entry/all/page.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,7 @@ describe('League entries page (Entry Component)', () => {
screen.queryByTestId('add-new-entry-button'),
).not.toBeInTheDocument();
});
it('should display "Make Pick" button when no pick is set', async () => {
it('should display "Make Pick" link when no pick is set', async () => {
mockGetCurrentUserEntries.mockResolvedValueOnce([
{
$id: '123',
Expand All @@ -293,13 +293,13 @@ describe('League entries page (Entry Component)', () => {
render(<Entry params={{ leagueId: '123' }} />);

await waitFor(() => {
expect(screen.getByTestId('league-entry-pick-button')).toHaveTextContent(
expect(screen.getByTestId('league-entry-pick-link')).toHaveTextContent(
'Make Pick',
);
});
});

it('should render team logo and change button to "Change Pick" when a pick is made', async () => {
it('should render team logo and change link to "Change Pick" when a pick is made', async () => {
mockUseDataStore.mockReturnValue({
...mockUseDataStore(),
currentWeek: 1,
Expand All @@ -321,7 +321,7 @@ describe('League entries page (Entry Component)', () => {
'team-a-logo.png',
);

expect(screen.getByTestId('league-entry-pick-button')).toHaveTextContent(
expect(screen.getByTestId('league-entry-pick-link')).toHaveTextContent(
'Change Pick',
);
});
Expand Down
3 changes: 2 additions & 1 deletion app/(main)/league/[leagueId]/entry/all/page.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,10 +205,11 @@ const Entry = ({
return (
<section key={entry.$id}>
<LeagueEntries
key={entry.$id}
entryName={entry.name}
isEliminated={entry.eliminated}
isLockedOutProp={false}
isPickSet={isPickSet}
key={entry.$id}
linkUrl={linkUrl}
teamLogo={teamLogo}
/>
Expand Down
1 change: 1 addition & 0 deletions components/LeagueEntries/LeagueEntries.interface.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,6 @@ export interface ILeagueEntriesProps {
linkUrl: string;
isEliminated?: boolean;
isPickSet?: boolean;
isLockedOutProp: boolean;
teamLogo?: string;
}
35 changes: 20 additions & 15 deletions components/LeagueEntries/LeagueEntries.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,46 +4,52 @@ import React from 'react';

describe('LeagueEntries', () => {
it(`renders 'default' state without a pick made`, () => {
render(<LeagueEntries entryName="Entry 1" linkUrl="" />);
render(
<LeagueEntries entryName="Entry 1" isLockedOutProp={false} linkUrl="" />,
);

const leagueEntryContainerCard = screen.getByTestId(
'league-entry-container-card',
);
const leagueEntryNumber = screen.getByTestId('league-entry-number');
const entryStatus = screen.getByTestId('entry-status');
const leagueEntryPickButton = screen.getByTestId(
'league-entry-pick-button',
);
const leagueEntryPickLink = screen.getByTestId('league-entry-pick-link');

expect(entryStatus).toHaveTextContent('alive');
expect(leagueEntryContainerCard).toBeInTheDocument();
expect(leagueEntryNumber).toHaveTextContent('Entry 1');
expect(leagueEntryPickButton).toHaveTextContent('Make Pick');
expect(leagueEntryPickLink).toHaveTextContent('Make Pick');
});

it('renders as if the user made a pick', () => {
render(<LeagueEntries entryName="Entry 2" linkUrl="" isPickSet={true} />);
render(
<LeagueEntries
entryName="Entry 2"
isLockedOutProp={false}
linkUrl=""
isPickSet={true}
/>,
);

const leagueEntryContainerCard = screen.getByTestId(
'league-entry-container-card',
);
const leagueEntryNumber = screen.getByTestId('league-entry-number');
const entryStatus = screen.getByTestId('entry-status');
const leagueEntryPickButton = screen.getByTestId(
'league-entry-pick-button',
);
const leagueEntryPickLink = screen.getByTestId('league-entry-pick-link');

expect(entryStatus).toHaveTextContent('alive');
expect(leagueEntryContainerCard).toBeInTheDocument();
expect(leagueEntryNumber).toHaveTextContent('Entry 2');
expect(leagueEntryPickButton).toHaveTextContent('Change Pick');
expect(leagueEntryPickLink).toHaveTextContent('Change Pick');
});

it('renders as if the user is eliminated', () => {
render(
<LeagueEntries
entryName="Entry 3"
isEliminated
isLockedOutProp={false}
isPickSet={false}
linkUrl=""
/>,
Expand All @@ -67,6 +73,7 @@ describe('LeagueEntries', () => {
render(
<LeagueEntries
entryName="Entry 2"
isLockedOutProp={false}
isPickSet={true}
linkUrl={linkUrl}
teamLogo={teamLogoUrl}
Expand All @@ -78,15 +85,13 @@ describe('LeagueEntries', () => {
);
const leagueEntryNumber = screen.getByTestId('league-entry-number');
const entryStatus = screen.getByTestId('entry-status');
const leagueEntryPickButton = screen.getByTestId(
'league-entry-pick-button',
);
const leagueLink = screen.getByTestId('league-entry-pick-button-link');
const leagueEntryPickLink = screen.getByTestId('league-entry-pick-link');
const leagueLink = screen.getByTestId('league-entry-pick-link');
const leagueEntryLogo = screen.getByTestId('league-entry-logo');

expect(entryStatus).toHaveTextContent('alive');
expect(leagueEntryNumber).toHaveTextContent('Entry 2');
expect(leagueEntryPickButton).toHaveTextContent('Change Pick');
expect(leagueEntryPickLink).toHaveTextContent('Change Pick');
expect(leagueLink).toHaveAttribute('href', linkUrl);
expect(leagueEntryLogo).toBeInTheDocument();
expect(leagueEntryLogo).toHaveAttribute('src', teamLogoUrl);
Expand Down
153 changes: 99 additions & 54 deletions components/LeagueEntries/LeagueEntries.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
// Copyright (c) Gridiron Survivor.
// Licensed under the MIT License.

import { Button } from '../Button/Button';
import { cn } from '@/utils/utils';
import { EntryStatus } from '../EntryStatus/EntryStatus';
import { ILeagueEntriesProps } from './LeagueEntries.interface';
import React, { JSX } from 'react';
import Link from 'next/link';
import LinkCustom from '../LinkCustom/LinkCustom';
import React, { JSX, useEffect, useState } from 'react';

/**
* A card that contains information on the user's entry for this league. Contains the entry number, their entry status (alive or eliminated), team logo once a pick is set, and a button to make a pick or change their pick
Expand All @@ -16,67 +15,113 @@ import Link from 'next/link';
* @param props.isEliminated - If true, the user is flagged as eliminat4ed
* @param props.isPickSet - if true, the team logo of the picked team shows up on the LeagueEntries card and the button changes from "make a pick" to "chagne pick"
* @param props.teamLogo - the team logo
* @param props.lockout - if true, the user is locked out from making a pick
* @returns {React.JSX.Element} - A div element that contains the user's entry information
*/

/**
* Display all entries for a league.
* @param {string} leagueId - The league id.
* @returns {JSX.Element} The rendered entries component.
*/
const LeagueEntries = ({
entryName,
linkUrl,
isEliminated = false,
isPickSet = false,
linkUrl,
isLockedOutProp = false,
teamLogo = '',
}: ILeagueEntriesProps): JSX.Element => (
<div
data-testid="league-entry-container-card"
className={cn(
'league-entry-container-card grid h-20 min-w-fit grid-cols-2 justify-between rounded-lg border border-border bg-card p-4 text-card-foreground shadow-sm',
isEliminated ? 'bg-muted' : 'transparent',
)}
>
<section
className="league-entry-header flex items-center gap-12"
data-testid="league-entry-header"
}: ILeagueEntriesProps): JSX.Element => {
const [isLockedOut, setLockedOut] = useState<boolean>(isLockedOutProp);

useEffect(() => {
/**
* Checks if the user is locked out from making a pick
*/
const checkLockout = (): void => {
choir241 marked this conversation as resolved.
Show resolved Hide resolved
const currentDateAndTime = new Date();
const day = currentDateAndTime.getUTCDay();
const hours = currentDateAndTime.getUTCHours();
if (
(day === 4 && hours >= 0) ||
(day > 4 && day < 2) ||
(day === 2 && hours < 12)
) {
setLockedOut(true);
} else {
setLockedOut(false);
}
Comment on lines +45 to +53
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: We could add a return in the if statement and remove the else block entirely

Copy link
Member Author

@ryandotfurrer ryandotfurrer Sep 18, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I'm getting a TypeScript Parsing error when I try that:

The parser expected to find a ')' to match the '(' token here.

const checkLockout = (): void => {
      const currentDateAndTime = new Date();
      const day = currentDateAndTime.getUTCDay();
      const hours = currentDateAndTime.getUTCHours();
      if (
        (day === 4 && hours >= 0) ||
        (day > 4 && day < 2) ||
        (day === 2 && hours < 12)

        return setLockedOut(true);
      ) else {
        setLockedOut(false);
      }
    };

Copy link
Contributor

@choir241 choir241 Sep 18, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

You're missing curly braces for your if statement here, also I meant something along the lines of the following:

const checkLockout = (): void => {
      const currentDateAndTime = new Date();
      const day = currentDateAndTime.getUTCDay();
      const hours = currentDateAndTime.getUTCHours();
      if (
        (day === 4 && hours >= 0) ||
        (day > 4 && day < 2) ||
        (day === 2 && hours < 12)){

        setLockedOut(true);
        return;
      }
        
    setLockedOut(false);
      
    };

};

checkLockout();

const intervalId = setInterval(checkLockout, 60 * 60 * 1000);

return (): void => clearInterval(intervalId);
choir241 marked this conversation as resolved.
Show resolved Hide resolved
}, []);

return (
<div
data-testid="league-entry-container-card"
className={cn(
'league-entry-container-card grid h-20 min-w-fit grid-cols-2 justify-between rounded-lg border border-border bg-card p-4 text-card-foreground shadow-sm',
isEliminated ? 'bg-muted' : 'transparent',
)}
>
<h4
data-testid="league-entry-number"
className={cn(
'league-entry-number text-xl font-semibold leading-none tracking-tight',
isEliminated ? 'opacity-50' : '',
)}
<section
className="league-entry-header flex items-center gap-12"
data-testid="league-entry-header"
>
{entryName}
</h4>
<EntryStatus isEliminated={isEliminated} />
</section>
<section
className="league-entry-footer flex items-center justify-end gap-12"
data-testid="league-entry-footer"
>
{isPickSet && (
<img
className="league-entry-logo h-12 w-12"
data-testid="league-entry-logo"
src={teamLogo}
alt="teamLogo"
/>
)}

<div
className="league-entry-pick-button-container"
data-testid="league-entry-pick-button-container"
<h4
data-testid="league-entry-number"
className={cn(
'league-entry-number text-xl font-semibold leading-none tracking-tight',
isEliminated ? 'opacity-50' : '',
)}
>
{entryName}
</h4>
<EntryStatus isEliminated={isEliminated} />
</section>
<section
className="league-entry-footer flex items-center justify-end gap-12"
data-testid="league-entry-footer"
>
{!isEliminated && (
<Link href={linkUrl} data-testid="league-entry-pick-button-link">
<Button
className="league-entry-pick-button"
data-testid="league-entry-pick-button"
label={isPickSet ? 'Change Pick' : 'Make Pick'}
variant={isPickSet ? 'secondary' : 'default'}
/>
</Link>
{isPickSet && (
<img
className="league-entry-logo h-12 w-12"
data-testid="league-entry-logo"
src={teamLogo}
alt="teamLogo"
/>
)}
</div>
</section>
</div>
);

<div
className="league-entry-pick-button-container"
data-testid="league-entry-pick-button-container"
>
{!isEliminated && (
<LinkCustom
aria-disabled={isLockedOut === true ? 'true' : 'false'}
className={cn(
'league-entry-pick-link',
isLockedOut === true ? 'opacity-50 cursor-not-allowed' : '',
)}
dataTestidProp="league-entry-pick-link"
href={linkUrl}
onClick={(e: { preventDefault: () => unknown }) =>
isLockedOut === true && e.preventDefault()
}
size={'defaultButton'}
variant={isPickSet ? 'secondaryButton' : 'primaryButton'}
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are we using isLockedOut === true in line 112, but here we're checking for truthy/falsy values?

Copy link
Member Author

Choose a reason for hiding this comment

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

I could not get the functionally working without explicitly checking if isLockedOut === true. When I built the LeagueEntries component previously I was able to make it work with the truthy/falsy values.

I'm all for making it more explicit if that is the consensus.

Copy link
Contributor

Choose a reason for hiding this comment

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

@shashilo what are your thoughts on this?

Copy link
Contributor

Choose a reason for hiding this comment

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

@ryandotfurrer @choir27 I think it has to do with how the prop is being brought in and the useEffect is changing the state of isLockedOut.

Copy link
Contributor

Choose a reason for hiding this comment

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

@ryandotfurrer we don't need to bring this in as a prop as the parent component calling it is not doing anything to change that value.

Remove the prop, set the default state to false. and then see if that allows you to do isLockedOut without the check if it === true.

>
{isPickSet ? 'Change Pick' : 'Make Pick'}
</LinkCustom>
)}
</div>
</section>
</div>
);
};

export { LeagueEntries };
6 changes: 5 additions & 1 deletion components/LinkCustom/LinkCustom.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,11 @@ import LinkCustom from './LinkCustom';
describe('LinkCustom Component', () => {
it('renders with default props', () => {
render(
<LinkCustom children="Test link" href="https://example.com"></LinkCustom>,
<LinkCustom
children="Test link"
dataTestidProp="linkCustom"
href="https://example.com"
></LinkCustom>,
);
Copy link
Contributor

Choose a reason for hiding this comment

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

Q: Do we need to individually test each properties?

const link = screen.getByTestId('linkCustom');
expect(link).toBeInTheDocument();
Expand Down
Loading