Closed n8kim1 closed 2 months ago
Is there a reason this is urgent? If not, would rather wait for cleaner code before considering this for merge. In particular am curious whether it's possible to avoid hardcoded naming patterns (which have unintended consequences, such as failing "bc24-very-cool-tournament").
Just don't write silly names
but yeah, was thinking about it and not sure what to do, cuz this has to be part of the PK ultimately. FWIW none of our current PK for tour have hyphens and it seems like we're sticking w that
Other ideas:
but also, mostly cuz if sprint tour fails, then this PR would be great for safety in copying it over, as well as time/convenience
I agree with jerry on waiting for cleaner code before merging this - right now it seems like we're introducing some code that may break if future devs decide to change up the naming schemes
can user input string? (via an admin action, this would require some other django code, but maybe it's worth introducing) I think this would be the cleanest solution.
declining this PR --
abandoning a tournament, in the way this PR does, is problematic
thoughts on prioritizing the other things first- main objection to dupe tournament is that it mainstreams “give up on the tournament whenever anything goes wrong and leave junk behind in db” the fix feels more like a “dirty fix” which was ok last year considering we were still in alpha, but feels less ok for something we want to enshrine in stable prod (for a full discussion: i think it’s this combination of give-up + junk that i object to, not give-up alone. for example if there was no junk left behind, then i wouldn’t feel so irked)
and solving the problem posed here can be done in other ways. for example, consider this outline
rescind match result on challonge prevent requeueing a tournament match that’s already in db (regardless of its state: even if it’s errored, it should be re-queued or deleted+recreated) change challonge participant list (in case seeds or eligibility was wrong) (should pre-condition complete rescind before allowing this)
Blocked on #723 for safety
The code quality -- especially computation of new name -- is a bit scuffed but it does the job for now. I don't think this needs clean error handling yet, so I'd rather add it later as needed, but happy to do so now.
Does part of #594