Archive contest - Issue/176 #322
Conversation
|
please let me know when this PR is ready for review |
|
have you found second reviewer? |
oioioi/forum/views.py
Outdated
| @@ -131,6 +134,7 @@ def category_view(request, category_id): | |||
| 'msgs': get_msgs(request), | |||
| 'can_interact_with_users': can_interact_with_users(request), | |||
| 'can_interact_with_admins': can_interact_with_admins(request), | |||
| 'is_archived': is_archived(request), | |||
There was a problem hiding this comment.
Inconsistency in naming the variables: see line 86 in the same file
| @@ -5,6 +5,11 @@ | |||
|
|
|||
| {% block main-content %} | |||
| {% for header in headers %}{{ header }}{% endfor %} | |||
| {% if contest.is_archived %} | |||
| <div class="alert alert-info"> | |||
There was a problem hiding this comment.
I wonder if yellow wouldn't be a better choice for the div's color, as it would be harder to miss... But it is a personal choice after all.
oioioi/contests/utils.py
Outdated
|
|
||
| @make_request_condition | ||
| @request_cached | ||
| def is_not_archived(request): |
There was a problem hiding this comment.
Is there a reason why there're two almost the same functions and "not is_archived" is used? Is that because it is used in decorators, or just for readability?
There was a problem hiding this comment.
I didn't know you can negate condition in enforce_condition. Fixed!
| return not is_archived(request) | ||
|
|
||
|
|
||
| def get_inline_for_contest(inline, contest): |
There was a problem hiding this comment.
Clever! A short comment with explanation could be useful though.
|
|
||
|
|
||
| @enforce_condition(contest_exists & is_contest_basicadmin) | ||
| def confirm_archive_contest(request): |
There was a problem hiding this comment.
I like it, clever again!
| @@ -58,6 +58,9 @@ def categories(self, obj): | |||
| categories.short_description = _("Categories") | |||
|
|
|||
| def add_category(self, obj): | |||
| if obj.contest.is_archived: | |||
| return _("Unarchive the contest to add category.") | |||
There was a problem hiding this comment.
I wonder if it could look a bit more clear and visible? Easier to distinguish? The text could be made bold, or underlined, or maybe in a box? Just some ideas.
Looking at the code I'm not sure it's an easy fix, so maybe the effort wouldn't be worth the outcome.
| @@ -230,6 +234,10 @@ def edit_post_view(request, category_id, thread_id, post_id): | |||
| if not (post.author == request.user or is_admin): | |||
| raise PermissionDenied | |||
|
|
|||
| # Only admins can edit posts when contest is archived. | |||
There was a problem hiding this comment.
Comments are always a plus!
oioioi/forum/views.py
Outdated
| @@ -300,7 +309,9 @@ def delete_post_view(request, category_id, thread_id, post_id): | |||
| ) | |||
|
|
|||
|
|
|||
| @enforce_condition(not_anonymous & contest_exists & can_enter_contest) | |||
| @enforce_condition( | |||
There was a problem hiding this comment.
Why is it in 3 lines, not in 1? Inconsistency with formatting above.
| @@ -24,6 +24,7 @@ def registration_notice_fragment(request): | |||
| and request.user.is_authenticated | |||
| and not is_contest_admin(request) | |||
| and not is_participant(request) | |||
| and rc.can_register(request) | |||
There was a problem hiding this comment.
I don't think this change in code is connected with this issue - good that you fixed that, though!
There was a problem hiding this comment.
You're right, it's only indirectly related with our issue. Without this, archived contest would still have the registration alert.
fix inconsistent formatting
|
Improved naming, formatting, added some comment. Changed the archived contest alert background to yellow. |
apparently, you can negate condition in enforce_condition
Added archive functionality to contests.
Participants cannot send submissions, ask questions, write on forum while contest is archived.