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

exclude Yourself player from Tournament set #30

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

abagali1
Copy link
Member

@abagali1 abagali1 commented Aug 4, 2022

cleaning up my fork - copied from previous PR

  • change include_users, bye_player, and add_users form field querysets from Submission.objects.latest() to Submission.objects.latest().exclude(user=get_user_model().objects.get(username="Yourself"))
  • remove ValidationErrors regarding inclusion of "Yourself" Submission as a Tournament player

@abagali1 abagali1 force-pushed the exclude-yourself branch 2 times, most recently from 1ead8a2 to 4ed9d5f Compare August 4, 2022 04:47
@coveralls
Copy link

coveralls commented Aug 4, 2022

Coverage Status

Coverage decreased (-0.04%) to 33.52% when pulling 07b53a4 on abagali1:exclude-yourself into 966f92b on tjcsl:master.

@abagali1
Copy link
Member Author

abagali1 commented Aug 4, 2022

CI was failing for a somewhat legitimate reason:

Referencing the User model, specifically to access the "Yourself" player, created a circular dependency which caused the initial db migration to fail (the "Yourself" player is created in a db migration but referencing "Yourself" in TournamentCreateForm before that migration is applied caused the CI fail since the CI reapplies all migrations every run)

This change worked locally since my local environment had already applied the necessary migrations.

I modified the code to achieve the same goal but this method is considerably jank and this change is not necessarily required because the form validator prohibits the "Yourself" player from being included in the set. This is purely a frontend clarification

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.

2 participants