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.

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

Add queue cutoff line and queue join cooldown (resolves #29 and resolves #31) #40

Open
wants to merge 19 commits into
base: main
Choose a base branch
from

Conversation

NicholasBottone
Copy link

@NicholasBottone NicholasBottone commented Jun 5, 2022

Resolves #29 and resolves #31

Requires backend changes proposed in nthnluu/hours-backend#14

  • modify the queue struct model to include a field representing the ticket ID of the last ticket before the cutoff (or null if the cutoff line is at the top / before any tickets)
  • modify the cutoff request to take in a ticket ID for the position the cutoff point should be
  • modify the backend endpoints to update new models
    • starting cutoff mode should set the queue's cutoff ticket to be the id of the last ticket in the queue
    • when a ticket is deleted, check if the ticket id matches the cutoff ticket - if this is the case, set the cutoff ticket to the ticket id of the ticket before this one (or null if this is the first ticket in the queue)
    • when the queue is shuffled, disable the cutoff line
    • when a ticket is marked as completed, do the same thing as when a ticket is deleted
  • modify the frontend to send the position the cutoff point should be with the cutoff request
  • display cutoff line on the frontend using the new information provided in the queue object

@vercel
Copy link

vercel bot commented Jun 5, 2022

Someone is attempting to deploy a commit to a Personal Account owned by @nthnluu on Vercel.

@nthnluu first needs to authorize it.

@vercel
Copy link

vercel bot commented Jun 16, 2022

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

Name Status Preview Updated
hours-staging ✅ Ready (Inspect) Visit Preview Jun 17, 2022 at 6:34PM (UTC)

@nthnluu nthnluu requested a review from n-young June 16, 2022 17:14
n-young
n-young previously approved these changes Jun 17, 2022
Copy link
Collaborator

@n-young n-young left a comment

Choose a reason for hiding this comment

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

lgtm, it'd be nice to see screenshots though. i'll approve now, could someone send me screenshots via slack before merging? tks

src/components/queue/QueueList/QueueList.tsx Outdated Show resolved Hide resolved
src/components/queue/QueueList/QueueList.tsx Outdated Show resolved Hide resolved
@NicholasBottone NicholasBottone changed the title Add queue cutoff line (resolves #29) Add queue cutoff line and queue join cooldown (resolves #29 and resolves #31) Jun 17, 2022
src/util/queue/api.ts Outdated Show resolved Hide resolved
src/util/shared/constants.ts Outdated Show resolved Hide resolved
src/util/queue/api.ts Outdated Show resolved Hide resolved
src/components/queue/QueueList/QueueList.tsx Outdated Show resolved Hide resolved
src/components/queue/QueueList/QueueList.tsx Outdated Show resolved Hide resolved
@NicholasBottone
Copy link
Author

@n-young @nthnluu @neilramaswamy I think I've resolved any issues, please review again and see if there is anything else that needs changing before release

Copy link
Collaborator

@neilramaswamy neilramaswamy left a comment

Choose a reason for hiding this comment

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

Mostly style comments; I didn't actually run this but most of this looks like it'll work. I also expect that the time-limit will be enforced on the backend, correct?

src/components/queue/QueueList/QueueList.tsx Outdated Show resolved Hide resolved
src/components/queue/QueueList/QueueList.tsx Show resolved Hide resolved
src/components/queue/QueueList/QueueList.tsx Outdated Show resolved Hide resolved
src/components/queue/QueueList/QueueList.tsx Outdated Show resolved Hide resolved
src/components/queue/QueueList/QueueList.tsx Outdated Show resolved Hide resolved
src/components/queue/QueueList/QueueList.tsx Show resolved Hide resolved
@NicholasBottone
Copy link
Author

Not sure why github isn't letting me request re-review, but addressed PR feedback again @nthnluu @neilramaswamy

@NicholasBottone NicholasBottone requested review from neilramaswamy and removed request for nthnluu October 20, 2022 20:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants