-
Notifications
You must be signed in to change notification settings - Fork 333
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
Allow tournament administrators to clone tournaments #2579
Allow tournament administrators to clone tournaments #2579
Conversation
Instead of using extra state to track whether the tournament, rounds, and players have loaded, detect it: - Stop setting `tournament.id`, then test for `tournament.id === tournament_id`. - Initialize `raw_rounds` to null, then test for non-null. - Introduce `raw_players and initialize to null, then test for non-null.
Add a "Clone Tournament" button to the tournament page that allows an existing tournament to be cloned. - Button shows up for anyone that can administer the tournament. - Takes the user to the "new tournament" page in the same group. - All tournament settings are pre-loaded to match the source tournament. This makes it easy to create new tournaments in a series, without the laborious and error-prone process of re-entering the settings.
Uffizzi Preview |
src/views/Tournament/Tournament.tsx
Outdated
// this is so anoek (user id 1) can quickly test tournaments | ||
React.useEffect(() => { | ||
if (user.id === 1 && tournament_id === 0) { | ||
setTournament({ | ||
...tournament, | ||
...default_tournament, |
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.
@anoek, for obvious reasons, I haven't been able to test this part of the code. I also suspect it blocks "Clone Tournament" from working for you.
Two options to consider:
- Delete this block entirely now that we have "Clone Tournament". It seems like the only thing you're gaining vs. clicking "Clone Tournament" is not having to set
time_start
. - Lift this up, changing how
default_tournament
gets set based onuser.id === 1
instead of modifying the tournament later viauseEffect
. This would be easier to reason about.
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.
I implemented and pushed the second option in 80e5e8f. But it might be nice to delete this entirely, or limit it just to a time_start
override, if you can use "Clone Tournament" instead.
One other consideration is what to do for the tournament title. If the tournament-to-clone's name was
I like the simplicity of just taking the old title verbatim. The user can fix/change it. |
Fix [sentry issues](online-go#2577 (comment)) by delaying computation of rounds until everything is loaded.
…rything-is-loaded' into HEAD
Slick, thanks! |
Add a "Clone Tournament" button to the tournament page that allows an existing tournament to be cloned.
This makes it easy to create new tournaments in a series, without the laborious and error-prone process of re-entering the settings.
Replaces #2573.
In case it's easier to look at them individually, there are three commits here, the first two of which are prep commits which would be nice to land even if this PR doesn't: