-
Notifications
You must be signed in to change notification settings - Fork 180
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
Rename registration_opened?
to registration_open?
#9709
Rename registration_opened?
to registration_open?
#9709
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As much as I agree about the confusing logic of the competition method names, I'm really afraid about this change because it
(a) renames an existing method and
(b) introduces a new method that is named exactly like the old one
where especially point (b) is dangerous. Some parts of the code that you did not catch might still reference the old registration_opened?
implementation which now "accidentally" gets rerouted to a semantically different implementation.
I'd feel much more comfortable if you split htis up into a separate PR that only does renames (basically, a glorified version of point (a) above, maybe also considering the nomenclature of other registration_foo?
methods) and then after that passes and can be merged, pick this up again with only an implementation for (b)
registration_opened?
to registration_open?
Ok this is ready to review again - I will add a method to check if registration has been open in a follow up PR |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the sake of what's to come, might I suggest registration_currently_open?
?
I was 50/50 between this and |
This has been requested as a pre-requisite for auto-accepts. Basically, in some regions, organizers/delegates don't currently pre-register for their competitions because they don't want to show up on the Competitor list before the competition actually opens (causing confusion for competitors/generally looking a bit weird.
So, I've suggested that we only show the "Competitors" tab when registration for a competition is actually open.
Implementation
The flaw in this implementation is that if registration closes, and has another
registration_open
date set, then until registration opens again, the Competitor list will not be visible.To overcome this, we could add a boolean to the competition table which tracks whether registration has opened at least once. But that would be yet another setting in the competition table, which isn't ideal. I'll ask WCAT about the relevance of this edge case.