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 Mar 7, 2022
1 parent c93db56 commit 9c79d6a
Show file tree
Hide file tree
Showing 5 changed files with 62 additions and 79 deletions.
36 changes: 16 additions & 20 deletions judge/models/contest.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
from jsonfield import JSONField
from lupa import LuaRuntime
from moss import MOSS_LANG_C, MOSS_LANG_CC, MOSS_LANG_JAVA, MOSS_LANG_PYTHON
from typing import Optional

from judge import contest_format
from judge.models.problem import Problem
Expand Down Expand Up @@ -379,35 +380,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
84 changes: 34 additions & 50 deletions judge/models/tests/test_contest.py
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from django.core.exceptions import ValidationError
from django.test import SimpleTestCase, TestCase
from django.utils import timezone
from typing import Optional

from judge.models import Contest, ContestParticipation, ContestTag
from judge.models.contest import MinValueOrNoneValidator
Expand Down Expand Up @@ -319,35 +320,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 +353,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 +448,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 +459,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 +579,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 +644,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 +699,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 +726,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 +759,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 +768,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 +977,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
12 changes: 7 additions & 5 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 @@ -276,6 +278,9 @@ def get_context_data(self, **kwargs):
problem_count=Count('id'),
),
)
participation_type = self.object.get_join_type(self.request.user)
context['can_live_join'] = participation_type == ContestParticipation.LIVE
context['can_spectate'] = participation_type == ContestParticipation.SPECTATE
return context


Expand Down Expand Up @@ -382,11 +387,8 @@ def join_contest(self, request, access_code=None):
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 9c79d6a

Please sign in to comment.