Skip to content

Commit

Permalink
Refactor contest join checks
Browse files Browse the repository at this point in the history 
As suggested, merge the access checks into one. Some tests had to be
changed, because the old checks didn't check the dates.
  • Loading branch information
@Riolku
Riolku committed May 23, 2022
1 parent c93db56 commit f9d811e
Show file tree
Hide file tree
Showing 5 changed files with 66 additions and 80 deletions.
37 changes: 17 additions & 20 deletions judge/models/contest.py
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
from typing import Optional

from django.core.exceptions import ValidationError
from django.core.validators import MaxValueValidator, MinValueValidator, RegexValidator
from django.db import models, transaction
Expand Down Expand Up @@ -379,35 +381,30 @@ def access_check(self, user):
return
raise self.PrivateContest()

# Assumes the user can access, to avoid the cost again
def is_live_joinable_by(self, user):
if not self.started:
return False
def get_join_type(self, user) -> Optional[int]:
if self.ended:
return None # Virtual Join should not be LIVE or SPECTATE

if not user.is_authenticated:
return False
return None

if user.profile.id in self.editor_ids or user.profile.id in self.tester_ids:
return False
return ContestParticipation.SPECTATE

if self.has_completed_contest(user):
return False
if not self.started:
return None

if self.limit_join_organizations:
return self.join_organizations.filter(id__in=user.profile.organizations.all()).exists()
return True
if user.profile.id in self.spectator_ids:
return ContestParticipation.SPECTATE

# Also skips access check
def is_spectatable_by(self, user):
if not user.is_authenticated:
return False
if (self.limit_join_organizations and
not self.join_organizations.filter(id__in=user.profile.organizations.all()).exists()):
return None

if user.profile.id in self.editor_ids or user.profile.id in self.tester_ids:
return True
if self.has_completed_contest(user):
return ContestParticipation.SPECTATE

if self.limit_join_organizations:
return self.join_organizations.filter(id__in=user.profile.organizations.all()).exists()
return True
return ContestParticipation.LIVE

def is_accessible_by(self, user):
try:
Expand Down
85 changes: 35 additions & 50 deletions judge/models/tests/test_contest.py
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
from typing import Optional

from django.core.exceptions import ValidationError
from django.test import SimpleTestCase, TestCase
from django.utils import timezone
Expand Down Expand Up @@ -319,35 +321,31 @@ def test_basic_contest_methods(self):
'superuser': {
'can_see_own_scoreboard': self.assertTrue,
'can_see_full_scoreboard': self.assertTrue,
'is_live_joinable_by': self.assertFalse, # author
'is_spectatable_by': self.assertTrue,
'get_join_type': self.assertSpectate, # author
'is_accessible_by': self.assertTrue,
'is_editable_by': self.assertTrue,
'is_in_contest': self.assertFalse,
},
'staff_contest_edit_own': {
'can_see_own_scoreboard': self.assertTrue,
'can_see_full_scoreboard': self.assertTrue,
'is_live_joinable_by': self.assertFalse, # author
'is_spectatable_by': self.assertTrue,
'get_join_type': self.assertSpectate, # author
'is_accessible_by': self.assertTrue,
'is_editable_by': self.assertTrue,
'is_in_contest': self.assertFalse,
},
'staff_contest_see_all': {
'can_see_own_scoreboard': self.assertTrue,
'can_see_full_scoreboard': self.assertTrue,
'is_live_joinable_by': self.assertTrue,
'is_spectatable_by': self.assertTrue,
'get_join_type': self.assertLive,
'is_accessible_by': self.assertTrue,
'is_editable_by': self.assertFalse,
'is_in_contest': self.assertFalse,
},
'staff_contest_edit_all': {
'can_see_own_scoreboard': self.assertTrue,
'can_see_full_scoreboard': self.assertTrue,
'is_live_joinable_by': self.assertTrue,
'is_spectatable_by': self.assertTrue,
'get_join_type': self.assertLive,
'is_accessible_by': self.assertTrue,
'is_editable_by': self.assertTrue,
'is_in_contest': self.assertFalse,
Expand All @@ -356,26 +354,23 @@ def test_basic_contest_methods(self):
# scoreboard checks don't do accessibility checks
'can_see_own_scoreboard': self.assertTrue,
'can_see_full_scoreboard': self.assertTrue,
'is_live_joinable_by': self.assertTrue,
'is_spectatable_by': self.assertTrue,
'get_join_type': self.assertLive,
'is_accessible_by': self.assertFalse,
'is_editable_by': self.assertFalse,
'is_in_contest': self.assertFalse,
},
'non_staff_tester': {
'can_see_own_scoreboard': self.assertTrue,
'can_see_full_scoreboard': self.assertTrue,
'is_live_joinable_by': self.assertFalse,
'is_spectatable_by': self.assertTrue,
'get_join_type': self.assertSpectate,
'is_accessible_by': self.assertTrue,
'is_editable_by': self.assertFalse,
'is_in_contest': self.assertFalse,
},
'anonymous': {
'can_see_own_scoreboard': self.assertTrue,
'can_see_full_scoreboard': self.assertTrue,
'is_live_joinable_by': self.assertFalse,
'is_spectatable_by': self.assertFalse,
'get_join_type': self.assertCantJoin,
'is_accessible_by': self.assertFalse,
'is_editable_by': self.assertFalse,
'is_in_contest': self.assertFalse,
Expand Down Expand Up @@ -454,8 +449,7 @@ def test_contest_hidden_scoreboard_contest_methods(self):
'normal_before_window': {
'can_see_own_scoreboard': self.assertFalse,
'can_see_full_scoreboard': self.assertFalse,
'is_live_joinable_by': self.assertTrue,
'is_spectatable_by': self.assertTrue,
'get_join_type': self.assertLive,
'has_completed_contest': self.assertFalse,
},
'normal_during_window': {
Expand All @@ -466,15 +460,13 @@ def test_contest_hidden_scoreboard_contest_methods(self):
'normal_after_window': {
'can_see_own_scoreboard': self.assertTrue,
'can_see_full_scoreboard': self.assertFalse,
'is_live_joinable_by': self.assertFalse,
'is_spectatable_by': self.assertTrue,
'get_join_type': self.assertSpectate,
'has_completed_contest': self.assertTrue,
},
'non_staff_tester': {
'can_see_own_scoreboard': self.assertFalse,
'can_see_full_scoreboard': self.assertFalse,
'is_live_joinable_by': self.assertFalse,
'is_spectatable_by': self.assertTrue,
'get_join_type': self.assertSpectate,
'has_completed_contest': self.assertFalse,
},
}
Expand Down Expand Up @@ -588,32 +580,25 @@ def test_tester_see_scoreboard_contest_methods(self):
def test_public_limit_organization_join_contest(self):
data = {
'non_staff_tester': {
'is_live_joinable_by': self.assertFalse,
'is_spectatable_by': self.assertTrue,
'get_join_type': self.assertSpectate,
},
'non_staff_author': {
'is_live_joinable_by': self.assertFalse,
'is_spectatable_by': self.assertTrue,
'get_join_type': self.assertSpectate,
},
'staff_contest_edit_own': {
'is_live_joinable_by': self.assertFalse,
'is_spectatable_by': self.assertTrue, # curator
'get_join_type': self.assertSpectate, # curator
},
'staff_contest_edit_all': {
'is_live_joinable_by': self.assertFalse,
'is_spectatable_by': self.assertFalse,
'get_join_type': self.assertCantJoin,
},
'normal': {
'is_live_joinable_by': self.assertFalse,
'is_spectatable_by': self.assertFalse,
'get_join_type': self.assertCantJoin,
},
'normal_open_org': {
'is_live_joinable_by': self.assertTrue,
'is_spectatable_by': self.assertTrue,
'get_join_type': self.assertLive,
},
'normal_after_window': {
'is_live_joinable_by': self.assertFalse,
'is_spectatable_by': self.assertFalse, # not in org
'get_join_type': self.assertCantJoin,
},
}
self._test_object_methods_with_users(self.public_limit_organization_join_contest, data)
Expand Down Expand Up @@ -660,8 +645,6 @@ def test_private_contest_methods(self):
'normal_open_org': {
'can_see_own_scoreboard': self.assertTrue,
'can_see_full_scoreboard': self.assertTrue,
'is_live_joinable_by': self.assertTrue,
'is_spectatable_by': self.assertTrue,
'is_accessible_by': self.assertTrue,
'is_editable_by': self.assertFalse,
'is_in_contest': self.assertFalse,
Expand Down Expand Up @@ -717,8 +700,7 @@ def test_organization_private_contest_methods(self):
'normal': {
'can_see_own_scoreboard': self.assertTrue,
'can_see_full_scoreboard': self.assertTrue,
'is_live_joinable_by': self.assertTrue,
'is_spectatable_by': self.assertTrue,
'get_join_type': self.assertLive,
'is_accessible_by': self.assertTrue,
'is_editable_by': self.assertFalse,
'is_in_contest': self.assertFalse,
Expand All @@ -745,35 +727,31 @@ def test_future_organization_private_contest_methods(self):
'staff_contest_edit_own': {
'can_see_own_scoreboard': self.assertFalse,
'can_see_full_scoreboard': self.assertFalse,
'is_live_joinable_by': self.assertFalse,
'is_spectatable_by': self.assertTrue,
'get_join_type': self.assertCantJoin,
'is_accessible_by': self.assertFalse,
'is_editable_by': self.assertFalse,
'is_in_contest': self.assertFalse,
},
'staff_contest_see_all': {
'can_see_own_scoreboard': self.assertTrue,
'can_see_full_scoreboard': self.assertTrue,
'is_live_joinable_by': self.assertFalse,
'is_spectatable_by': self.assertTrue,
'get_join_type': self.assertCantJoin,
'is_accessible_by': self.assertTrue,
'is_editable_by': self.assertFalse,
'is_in_contest': self.assertFalse,
},
'staff_contest_edit_all': {
'can_see_own_scoreboard': self.assertTrue,
'can_see_full_scoreboard': self.assertTrue,
'is_live_joinable_by': self.assertFalse,
'is_spectatable_by': self.assertTrue,
'get_join_type': self.assertCantJoin,
'is_accessible_by': self.assertTrue,
'is_editable_by': self.assertTrue,
'is_in_contest': self.assertFalse,
},
'normal': {
'can_see_own_scoreboard': self.assertTrue,
'can_see_full_scoreboard': self.assertTrue,
'is_live_joinable_by': self.assertFalse,
'is_spectatable_by': self.assertTrue,
'get_join_type': self.assertCantJoin,
'is_accessible_by': self.assertTrue,
'is_editable_by': self.assertFalse,
'is_in_contest': self.assertFalse,
Expand All @@ -782,8 +760,7 @@ def test_future_organization_private_contest_methods(self):
# False because contest has not begun
'can_see_own_scoreboard': self.assertFalse,
'can_see_full_scoreboard': self.assertFalse,
'is_live_joinable_by': self.assertFalse,
'is_spectatable_by': self.assertTrue,
'get_join_type': self.assertSpectate,
'is_accessible_by': self.assertTrue,
'is_editable_by': self.assertFalse,
'is_in_contest': self.assertFalse,
Expand All @@ -792,8 +769,7 @@ def test_future_organization_private_contest_methods(self):
# False because contest has not begun
'can_see_own_scoreboard': self.assertFalse,
'can_see_full_scoreboard': self.assertFalse,
'is_live_joinable_by': self.assertFalse,
'is_spectatable_by': self.assertFalse,
'get_join_type': self.assertCantJoin,
'is_accessible_by': self.assertFalse,
'is_editable_by': self.assertFalse,
'is_in_contest': self.assertFalse,
Expand Down Expand Up @@ -1002,6 +978,15 @@ def test_virtual_participation(self):
self.assertEqual(participation.start, participation.real_start)
self.assertIsInstance(participation.end_time, timezone.datetime)

def assertLive(self, arg: Optional[int], msg: Optional[str] = None) -> None:
self.assertEqual(arg, ContestParticipation.LIVE, msg=msg)

def assertSpectate(self, arg: Optional[int], msg: Optional[str] = None) -> None:
self.assertEqual(arg, ContestParticipation.SPECTATE, msg=msg)

def assertCantJoin(self, arg: Optional[int], msg: Optional[str] = None) -> None:
self.assertIsNone(arg, msg=msg)


class ContestTagTestCase(TestCase):
@classmethod
Expand Down
15 changes: 9 additions & 6 deletions judge/views/contests.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,8 @@ def get_context_data(self, **kwargs):
context['current_contests'] = present
context['future_contests'] = future
context['finished_contests'] = finished
context['LIVE'] = ContestParticipation.LIVE
context['SPECTATE'] = ContestParticipation.SPECTATE
context['now'] = self._now
context['first_page_href'] = '.'
context['page_suffix'] = '#past-contests'
Expand Down Expand Up @@ -192,6 +194,11 @@ def get_context_data(self, **kwargs):
context['is_spectator'] = self.is_spectator
context['can_edit'] = self.can_edit

participation_type = self.object.get_join_type(self.request.user)
context['can_test'] = participation_type == ContestParticipation.TESTING
context['can_live_join'] = participation_type == ContestParticipation.LIVE
context['can_spectate'] = participation_type == ContestParticipation.SPECTATE

if not self.object.og_image or not self.object.summary:
metadata = generate_opengraph('generated-meta-contest:%d' % self.object.id,
self.object.description, 'contest')
Expand Down Expand Up @@ -380,13 +387,9 @@ def join_contest(self, request, access_code=None):
break
else:
SPECTATE = ContestParticipation.SPECTATE
LIVE = ContestParticipation.LIVE

if contest.is_live_joinable_by(request.user):
participation_type = LIVE
elif contest.is_spectatable_by(request.user):
participation_type = SPECTATE
else:
participation_type = contest.get_join_type(request.user)
if participation_type is None:
return generic_message(request, _('Cannot enter'),
_('You are not able to join this contest.'))
try:
Expand Down
4 changes: 2 additions & 2 deletions templates/contest/contest-tabs.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,15 +58,15 @@
{{- _('Leave contest') -}}
{% endif %}">
</form>
{% elif contest.is_live_joinable_by(request.user) %}
{% elif can_live_join %}
<form action="{{ url('contest_join', contest.key) }}" method="post"
class="contest-join-pseudotab unselectable button">
{% csrf_token %}
<input type="submit"
class="contest-join{% if not has_joined %} first-join{% endif %}"
value="{{ _('Join contest') }}">
</form>
{% elif contest.is_spectatable_by(request.user) %}
{% elif can_spectate %}
<form action="{{ url('contest_join', contest.key) }}" method="post"
class="contest-join-pseudotab unselectable button">
{% csrf_token %}
Expand Down
5 changes: 3 additions & 2 deletions templates/contest/list.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -131,13 +131,14 @@
{% macro contest_join(contest, request, finished_contests) %}
{% if not request.in_contest %}
<td>
{% if contest.is_live_joinable_by(request.user) %}
{% set participation_type = contest.get_join_type(request.user) %}
{% if participation_type == LIVE %}
<form action="{{ url('contest_join', contest.key) }}" method="post">
{% csrf_token %}
<input type="submit" class="unselectable button full participate-button join-warning"
value="{{ _('Join') }}">
</form>
{% elif contest.is_spectatable_by(request.user) %}
{% elif participation_type == SPECTATE %}
<form action="{{ url('contest_join', contest.key) }}" method="post">
{% csrf_token %}
<input type="submit" class="unselectable button full participate-button"
Expand Down

0 comments on commit f9d811e

Please sign in to comment.