Skip to content
This repository has been archived by the owner on Jan 3, 2025. It is now read-only.

Add Public Waiting List Tab #295

Merged
merged 34 commits into from
Dec 15, 2023
Merged

Add Public Waiting List Tab #295

merged 34 commits into from
Dec 15, 2023

Conversation

FinnIckler
Copy link
Member

This needs still some way to move people up and down the waiting list so I will continue to work on this after some more pressing manners are done

@FinnIckler FinnIckler marked this pull request as ready for review November 28, 2023 15:01
@FinnIckler
Copy link
Member Author

I added basic waiting list management in the edit registration section. It's still as manual as currently on the monolith, but at least it's natively built in

@FinnIckler
Copy link
Member Author

@thewca-bot deploy staging

Copy link
Contributor

Choose a reason for hiding this comment

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

Thoughts on using "waitlist" everywhere? I think that's quite standard, at least in North America. It also makes all the file and variable names a bit shorter and consistent (sometimes it's waiting, sometimes waiting_list, etc. at the moment).

Frontend/src/pages/waiting/components/WaitingList.jsx Outdated Show resolved Hide resolved
@dunkOnIT
Copy link
Collaborator

dunkOnIT commented Dec 4, 2023

Review questions:

lane_factory.rb

  • Not sure if having waiting_list_position here is the best thing to do - but otherwise it's a bit awkward to set the waiting_list_position in testing
    • alternative: use a model function that only updates the waiting list position (or generalized to only update a certain property?)

lane.rb

  • Method for incrementing/decrementing waiting list positions - this could possible all be one function?
  • Competition_id - it's pretty gross needing to pass around the competition_id everywhere - can we create an instance variable somewhere? Or should we store something like `parent_attendee_id' for cross-referencing?

registration.rb

  • We have a lot of accessor methods - do we want to use a different approach? idk if there is one
  • How should we update the registration - do we really need to loop through all the lanes + provide all the parameters to update_attributes! if we're just updating one field?

@dunkOnIT
Copy link
Collaborator

dunkOnIT commented Dec 6, 2023

Duncan changes:

  • Change all tests to submit waiting list positions as strings
  • Add a test that registration can't be moved to a position > max or < min
  • Check that putting users on waiting list sets position as expected
  • Test all functions in frontend and make sure they all work as expected
  • Move get_registrations_by_status to be class method of Registration class
  • Move multiple competitors to waitlist at once
  • Work through all TODOs/notes
  • Update cache invalidations (and check cache is working by querying redis?)
  • Try out caching tests - enable/disable caching within the test, and then do Rails.cache.read to check cache value
  • Figure out why cache is giving weird issues

Finn changes:

  • Add a "show true waiting list index" option somewhere in UI (maybe call it "dev mode"? and/or only show it to people with certain permission levels?
  • Make waiting list table draggable

@dunkOnIT
Copy link
Collaborator

dunkOnIT commented Dec 7, 2023

Issues:

  • Moving multiple competitors to waiting list causes duplicate waiting list positions (assuming because they happen simultaneously?)
  • Cache is just returning the expiry time, not the cached object?

@dunkOnIT
Copy link
Collaborator

dunkOnIT commented Dec 15, 2023

  • Add redis to the test docker-compose file

@FinnIckler FinnIckler merged commit 072b266 into main Dec 15, 2023
1 check passed
@dunkOnIT dunkOnIT deleted the feature/public-waiting-list branch December 15, 2023 11:19
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants