Alex+Ryan/Add Lockout Period by alexappleget · Pull Request #563 · LetsGetTechnical/gridiron-survivor

alexappleget · 2024-09-25T20:27:32Z

Closes #525

Took over Ryan's task and cherry picked his commits over.

Added lockout period to disable pick buttons and then added lockout to the onWeeklyPickChange api function to throw a notification error saying picks are locked.

UPDATE: Created a custom useHook for the lockout period as useLockout()

Everything works as it should. With this new useHook, you can import the useLockout hook anywhere and just do const lockedOut = useLockout() and it'll know if lockedOut is true or false.

… branch

appwrite · 2024-09-25T20:27:35Z

Gridiron Survivor Application 6616ea581ef9f5521c7d

Function	ID	Status	Action
Your function has been successfully deployed.

Project name: Gridiron Survivor Application
Project ID: 6616ea581ef9f5521c7d

Function	ID	Status	Action
userAuth	`6626fef885a9f630442b`	Ready	View Logs

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.

💡 Did you know?
You can use Avatars API to generate QR code for any text or URLs

vercel · 2024-09-25T20:27:36Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
gridiron-survivor	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Oct 15, 2024 11:27pm
gridiron-survivor-storybook	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Oct 15, 2024 11:27pm

…d branch.

ryanfurrer · 2024-10-08T21:22:26Z

-              />
-            </Link>
+            <LinkCustom
+              aria-disabled={isLockedOut === true ? 'true' : 'false'}


@alexappleget This was a question previously brought up on my PR. Chris had suggested removing isLockedOut as a prop from the parent component, settings the default state to false, and seeing if that would allow us to remove the truthy/falsy values.

The truthy/falsy values being isLockedOut === true

So instead it could be rewritten as a true ternary: aria-disabled={isLockedOut ? 'true' : 'false'}

ryanfurrer · 2024-10-08T21:22:57Z

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


@alexappleget that is correct.

ryanfurrer · 2024-10-09T17:30:33Z

-              />
-            </Link>
+            <LinkCustom
+              aria-disabled={isLockedOut === true ? 'true' : 'false'}


@Danielle254 Yes, I too agree. That is how I originally tried developing it but it was not working as intended.

As per my previous comment, @chris-nowicki thought this may be to it being in the parent component as a prop.

…out-period

braydoncoyer · 2024-10-12T14:42:58Z

-      await onWeeklyPickChange(params);
-      setUserPick(teamSelect);
-      router.push(`/league/${league}/entry/all`);
+      if (lockedOut) {


Do we need to nest this if statement in the try block?

Yes. So what it does is run the api functions inside the try block. But, if its during the lockdown period it will block the api functions from running and instead run the toast notification. It is a way to stop hackers from hitting the api's when they shouldn't.

@braydoncoyer please let me know if you need more info on this

@alexappleget @ryandotfurrer Yep, I get the purpose of the Try block. I think I was more asking if the try block could be moved inside the conditional to better represent which part of the code may cause/throw an error.

I'll leave this as a nit because as-is this likely works. However, I'm always a big fan of being more specific about which part of the code may need to have errors caught.

if (lockedOut) { ... code } else { try { await onWeeklyPickChange(params); setUserPick(teamSelect); ... } catch (error) { ... } console.error(params); }

braydoncoyer · 2024-10-12T14:45:48Z

+    afterEach(() => {
+        getUTCDaySpy.mockRestore();
+        getUTCHoursSpy.mockRestore();
+    })


nit: can we get a newline here? If Prettier or another formatter is run in a pipeline, feel free to ignore.

@braydoncoyer done and will be reflected in the next push.

braydoncoyer · 2024-10-12T14:47:57Z

+ * Function to constantly check the current time every hour and set the lockout accordingly.
+ * @returns - The value of lockedOut.
+ */
+const useLockout = (): boolean => {


Since this hook returns a binary value to indicate a user's ability to perform a task, would a more specific name be effective?

Ex: useIsUserLockedOut

I actually like this name. I moreso just used useLockout to match everything. I'll change it :)

@braydoncoyer this has been updated by @alexappleget. Please resolve this convo when you can; thanks!

braydoncoyer · 2024-10-12T14:48:19Z

+      const day = currentDateAndTime.getUTCDay();
+      const hours = currentDateAndTime.getUTCHours();
+      if (
+        (day === 5 && hours >= 0) || // Thursday at 9pm EST


Do we care about time zones? I'm still ramping up and this may be a well-known answer but one I think is worth me asking.

these numbers are UTC time which is 4 hours ahead of me. I left the comments for easy visual understanding. It handles when the lockdown period is.

Do we care about time zones? I'm still ramping up and this may be a well-known answer but one I think is worth me asking.

@braydoncoyer I used UTC so we don't have to worry about time zones. I only left the comments in there for reference, like Alex said.

Not to mention, however, that Appwrite also uses UTC. So if we need to tie this in with that we don't need to make changes again.

braydoncoyer · 2024-10-12T14:51:33Z

+        day === 1 || // Monday
+        (day === 2 && hours < 12) // Tuesday at 9am EST
+      ) {
+        setLockedOut(true);


Looks like we may be updating this state regardless if the value has changed.

I wonder if a check could be made to ensure we only update state if the value has changed.

const isLockedOut = (day === 5 && hours >= 0) || // Thursday at 9pm EST day > 5 || // Friday and Saturday day === 0 || // Sunday day === 1 || // Monday (day === 2 && hours < 12); // Tuesday at 9am EST if (isLockedOut !== lockedOut) { setLockedOut(isLockedOut); }

I apologize that I dont follow. When Ryan made this function, it is to handle the buttons being disabled, etc. During the specific days/times itll keep lockedOut as true which will lock everything down. Then outside of those days/times itll have it set to false which will unlock everything for users to use. And with the interval it checks the time every hour.

@braydoncoyer like @alexappleget said, this is simply checking several statements and, if any of them are true, it will change the state. Are you saying you think this is changing the state multiple times?

Below is a screenshot of what it is achieving; notice the user cannot click and the links are disabled.

@ryandotfurrer Thanks for the clarification!

My original comment could have been more clear. I think that we may set state to a value (let's say true) even if the value is already true. This seems redundant. It may not actually work like this -- I'd have to pull down and test locally, but perhaps you know the answer off the top of your head!

…out-period

shashilo · 2024-10-14T16:55:26Z

+      const day = currentDateAndTime.getUTCDay();
+      const hours = currentDateAndTime.getUTCHours();
+      if (
+        (day === 5 && hours >= 0) || // Thursday at 9pm EST


Since we have the other change to not allow picks if a game has been played, let's update the lockdown period to Sat midnight. 95% of the NFL games are on Sun/Mon.

updated in latest commit

shashilo · 2024-10-14T16:55:58Z

+ */
+const useIsUserLockedOut = (): boolean => {
+  const [lockedOut, setLockedOut] = useState<boolean>(false);
+  useEffect(() => {


Why does this have to be a useEffect?

This is a useEffect as it is sets up the setInterval which checks the time every hour.

But why is it needed? Looks like the code can function without it. useEffects will cause unnecessary re-renders if one uses it incorrectly. Figure out if the code needs it, and if it renders too many times.

shashilo · 2024-10-14T16:56:54Z

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


Again, what is 60, 60, and 1000? Put these into CONST instead, so anyone can read and understand what each value means. Also, what's the reasoning behind useing setInterval?

60 * 60 * 1000 creates a setInterval that checks the time once per hour. The reasoning behind the setInterval is so that it checks the time every hour and can appropriately update the state of setLockedOut.

What do you mean by COST?

It was a long week. I meant CONST. A constant variable. What represents these individual numbers?

shashilo · 2024-10-14T16:57:23Z

-      await onWeeklyPickChange(params);
-      setUserPick(teamSelect);
-      router.push(`/league/${league}/entry/all`);
+      if (lockedOut) {


I don't see any new unit tests for this.

I will add these in ASAP.

@shashilo can you elaborate what test exactly you're looking for?

It's a conditional statement, so you need to have a positive and negative unit tests for this. Checking that both conditions are accurate when their conditions are met.

…nent code

shashilo · 2024-10-21T14:52:23Z

+ */
+const useIsUserLockedOut = (): boolean => {
+  const [lockedOut, setLockedOut] = useState<boolean>(false);
+  useEffect(() => {


But why is it needed? Looks like the code can function without it. useEffects will cause unnecessary re-renders if one uses it incorrectly. Figure out if the code needs it, and if it renders too many times.

shashilo · 2024-10-21T14:53:24Z

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


It was a long week. I meant CONST. A constant variable. What represents these individual numbers?

Fix: Updated page.test file to match Ryan's file in his "add-lockout"…

4e6700e

… branch

vercel Bot deployed to Preview – gridiron-survivor September 25, 2024 20:28 View deployment

vercel Bot deployed to Preview – gridiron-survivor-storybook September 25, 2024 20:28 View deployment

Fix: Made changes in entry/all page.tsx to match Ryan's past branch.

0c5ebe8

vercel Bot deployed to Preview – gridiron-survivor-storybook September 25, 2024 20:57 View deployment

vercel Bot had a problem deploying to Preview – gridiron-survivor September 25, 2024 20:58 Failure

Fix: Updated LeagueEntries interface file to match Ryan's old branch

c93a773

vercel Bot deployed to Preview – gridiron-survivor September 25, 2024 21:02 View deployment

vercel Bot deployed to Preview – gridiron-survivor-storybook September 25, 2024 21:02 View deployment

Fix: Updated League entry tests to take in Ryan's changes from his ol…

9973ae7

…d branch.

vercel Bot deployed to Preview – gridiron-survivor-storybook September 25, 2024 21:56 View deployment

vercel Bot deployed to Preview – gridiron-survivor September 25, 2024 21:57 View deployment

Fix: Updated LeagueEntries to bring in Ryan's old branch changes.

6fdee85

vercel Bot deployed to Preview – gridiron-survivor-storybook September 25, 2024 22:47 View deployment

vercel Bot had a problem deploying to Preview – gridiron-survivor September 25, 2024 22:48 Failure

Fix: Updated LinkCustom test file to match Ryan's.

ad04f2c

vercel Bot had a problem deploying to Preview – gridiron-survivor September 25, 2024 22:51 Failure

vercel Bot deployed to Preview – gridiron-survivor-storybook September 25, 2024 22:51 View deployment

Fix: Updated LinkCustom file to match Ryan's

988de13

vercel Bot had a problem deploying to Preview – gridiron-survivor September 25, 2024 23:07 Failure

vercel Bot deployed to Preview – gridiron-survivor-storybook September 25, 2024 23:08 View deployment

Fix: Finalized changes from Ryan's branch. All tests pass.

a576cab

vercel Bot deployed to Preview – gridiron-survivor-storybook September 26, 2024 00:22 View deployment

vercel Bot deployed to Preview – gridiron-survivor September 26, 2024 00:23 View deployment

Fix: Adjusted lockout logic.

5c69b6e

vercel Bot deployed to Preview – gridiron-survivor-storybook September 26, 2024 01:28 View deployment

vercel Bot deployed to Preview – gridiron-survivor September 26, 2024 01:29 View deployment

Fix: Adjusted lockout times and added comments.

7f9a693

Fix: deleted prop from tests as it no longer exists.

303f8a4

vercel Bot deployed to Preview – gridiron-survivor October 8, 2024 20:48 View deployment

vercel Bot deployed to Preview – gridiron-survivor-storybook October 8, 2024 20:49 View deployment

ryanfurrer reviewed Oct 9, 2024

View reviewed changes

Merge remote-tracking branch 'origin/develop' into Alex+Ryan/add-lock…

a8ff6a1

…out-period

vercel Bot deployed to Preview – gridiron-survivor October 10, 2024 18:59 View deployment

vercel Bot deployed to Preview – gridiron-survivor-storybook October 10, 2024 18:59 View deployment

Fix: Handled PR comments

0996cd7

vercel Bot deployed to Preview – gridiron-survivor-storybook October 10, 2024 19:34 View deployment

vercel Bot deployed to Preview – gridiron-survivor October 10, 2024 19:35 View deployment

braydoncoyer reviewed Oct 12, 2024

View reviewed changes

Merge remote-tracking branch 'origin/develop' into Alex+Ryan/add-lock…

b99ebb0

…out-period

vercel Bot deployed to Preview – gridiron-survivor October 12, 2024 15:13 View deployment

vercel Bot deployed to Preview – gridiron-survivor-storybook October 12, 2024 15:14 View deployment

Fix: Handled Braydon's PR comments and updated tests

bd81e39

vercel Bot deployed to Preview – gridiron-survivor October 12, 2024 15:23 View deployment

vercel Bot deployed to Preview – gridiron-survivor-storybook October 12, 2024 15:24 View deployment

shashilo requested changes Oct 14, 2024

View reviewed changes

ryanfurrer self-assigned this Oct 14, 2024

Merge branch 'develop' into Alex+Ryan/add-lockout-period

220d865

vercel Bot deployed to Preview – gridiron-survivor-storybook October 14, 2024 19:23 View deployment

vercel Bot deployed to Preview – gridiron-survivor October 14, 2024 19:25 View deployment

refactor: sorted imports and added additional line break in test.

d32c663

vercel Bot deployed to Preview – gridiron-survivor-storybook October 14, 2024 21:38 View deployment

vercel Bot deployed to Preview – gridiron-survivor October 14, 2024 21:39 View deployment

test: remove isLockedOutProp from LeagueEntries test to reflect compo…

52beb96

…nent code

vercel Bot deployed to Preview – gridiron-survivor-storybook October 14, 2024 22:30 View deployment

ryanfurrer added 2 commits October 14, 2024 19:58

feat: adjust lockout period to start Saturday @ 12:00am

5cbe943

test: adjust test for testing lockout from Friday to Saturday.

0b0ec0b

shashilo requested changes Oct 21, 2024

View reviewed changes


		checkLockout();

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

Uh oh!

Conversation

alexappleget commented Sep 25, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

appwrite Bot commented Sep 25, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vercel Bot commented Sep 25, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

braydoncoyer Oct 23, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ryanfurrer Oct 14, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ryanfurrer Oct 14, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shashilo Oct 14, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

alexappleget commented Sep 25, 2024 •

edited

Loading

appwrite Bot commented Sep 25, 2024 •

edited

Loading

vercel Bot commented Sep 25, 2024 •

edited

Loading

braydoncoyer Oct 23, 2024 •

edited

Loading

ryanfurrer Oct 14, 2024 •

edited

Loading

ryanfurrer Oct 14, 2024 •

edited

Loading

shashilo Oct 14, 2024 •

edited

Loading