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

Auto Accept PR 2: Frontend & Waiting List Integration #10416

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

Conversation

dunkOnIT
Copy link
Contributor

@dunkOnIT dunkOnIT commented Dec 11, 2024

Frontend Changes

  • Give organizers a way to toggle auto-accept off
  • Add disable auto accept button for organizers when auto accept is enabled
  • Add enable/disable auto accept settings in the competition form
  • Make the "Disable Auto Accept" button actually do something when it is clicked, to indicate whether or not auto accept was successfully disabled

Backend Changes

  • Auto-accept users onto waiting list when Accepted is full
    • Should they be accepted onto the waiting list if the auto-accept-cutff has been reached? No - consulted people on discord
  • Auto-accept users from waiting list if a spot becomes available in the accepted list (ie, if an attendee's status changes away from accepted
  • Add bulk auto accept class method
  • Add bulk auto accept route
  • Test if registration history and state change emails get created/sent as expected

Future PR

  • Move waiting list to above the accepted list if it has at least one registration || competitor_limit has been reached | auto_accept is enabled and auto_accept_disable_threshold has been reached
  • Warn an organizer if they're trying to accept a user from the Pending list while there are registrations on the Waiting List
  • Give organizers a way to toggle auto-accept on
  • Give organizers a way to run a bulk auto-accept
  • Automatically run bulk auto-accept when auto-accept is toggled on
    • Warn organizers this will happen, and give them an "Are you sure?" dialogue, where saying "No" will

@dunkOnIT
Copy link
Contributor Author

dunkOnIT commented Dec 12, 2024

Waiting list:

  • Accept a user from the waiting list if a spot becomes available on the Accepted list
  • Model Tests
    • allow auto-accepting the first position on the waiting list
  • Integration Tests
    • Trigger an auto_accept job from the competing lane if a user's status changes away from accepted

@dunkOnIT
Copy link
Contributor Author

Add to waiting list:

  • If the Accepted list is full, accept users onto the Waiting List
    • What if the auto_accept_disable threshold is reached?

@dunkOnIT
Copy link
Contributor Author

Bulk auto accept class method tests:

  • Accepts competitors from the waiting list
    • in order of waiting list
  • Accepts pending competitors
    • in order of payment
  • Accepts up to but not exceeding the competition limit
    • those not accepted follow the expected order (later on waiting list = not accepted, later payment = not accepted)

@dunkOnIT dunkOnIT changed the base branch from v3/auto-accept to main December 19, 2024 10:35
@dunkOnIT dunkOnIT marked this pull request as ready for review January 16, 2025 05:44
@@ -128,6 +130,8 @@ export default function RegistrationAdministrationList({ competitionInfo }) {

const actionsRef = useRef();

const [autoAcceptEnabled, setAutoAcceptEnabled] = useState(competitionInfo.auto_accept_registrations)
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we use competitionInfo as the source of truth?

Copy link
Contributor Author

@dunkOnIT dunkOnIT Jan 16, 2025

Choose a reason for hiding this comment

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

Currently it is being hydrated on render (not queried via API), so there's no way to refetch it currently. Assuming we could refetch it, how would you suggest we then update whether or not the "Disable auto accept" button is rendered - declare all of competitionInfo a state? (my understanding is that React doesn't detect changes in the state of variables)

Copy link
Member

Choose a reason for hiding this comment

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

if we would fetch it, we would rely on the react Query to handle the state being up to date.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah it would be pretty simple if we switch to fetching it with react query, I think.

Having a situation where there are 2 sources of truth (competitionInfo.auto_accept_registrations and autoAcceptEnabled), and which could disagree, is not good - leaves room for future bugs, etc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants