-
Notifications
You must be signed in to change notification settings - Fork 363
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
New analysis spoiler attachments #1027
base: master
Are you sure you want to change the base?
Conversation
Introduce analysis mode spoilers (task attachments visible to contestants only in analysis mode). This can be used for providing additional (or all) test data, sample solutions or anything that contestants are not allowed to access during the contest, but only during analysis.
Hi! Interesting idea. I'm not sure if it's a feature that everyone would need, but maybe it can save some time to contest admins. At a first glance, however, there seem to be a lot of code duplication: do you need the |
@stefano-maggiolo and I were just discussing this. This could be implemented in several ways (distinct tables, boolean column, polymorphism), each with pros and cons. Stefano seems to prefer the current approach, I prefer polymorphism. My concern with a boolean column is that one needs to remember to always filter by it and, if one forgets, one could end up displaying "spoilers" during the contest. (Also, we're not fully happy with the name "spoiler", though we couldn't come up with anything better.) |
Yeah, I prefer it as it is, and possibly with the boring name "AnalysisAttachment". I've been sick, and now I'm off for a week or so, apologies for any (further) delay. Thanks @magula for the patch, I like the concept! |
@lerks in fact described pretty well the reason I decided not to use a boolean flag. Using a boolean flag instead of just duplicating the code wouldn't even eliminate all of the duplicate code, but I think it would result in code in which it's easier to make an error, and one would have to be more careful whenever one's changing anything about attachments or spoilers not to accidentally... spoil spoilers. Now, I don't have any strong preferences on whether to use polymorphism or stick to the current approach (I'm not sure about the benefits of using polymorphism here or how to do that best). About the name: We weren't too happy about it either, but it's the best we could come up with. :) "Analysis attachments" would certainly be a good option, but I was a bit afraid that it might be to easy to confuse with regular attachments at a first glance (that's just guessing, though). So I'm definitely fine with changing that name. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To make a decision:
- let's keep it simple and stick to separate tables, also no polymorphism solution to code duplication;
- rename to AnalysisAttachment(s).
You also need to update the DB schema (in db/__init__.py
) and provide an updater that adds analysis_attachment = {} to every task. See 5e48afe for an example.
Reviewed 13 of 13 files at r1.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @magula)
cms/db/init.py, line 109 at r1 (raw file):
from .user import User, Team, Participation, Message, Question from .task import Task, Statement, Attachment, Spoiler, Dataset, Manager, \ Testcase
Also add to __all__
cms/db/task.py, line 359 at r1 (raw file):
class Spoiler(Base): """Class to store additional files to give to the user together with the statement of the task during analysis mode.
Class to store additional attachments to show only during analysis mode
.
cms/server/contest/templates/task_description.html, line 202 at r1 (raw file):
{% if contest.analysis_enabled and actual_phase == +3 and task.spoilers|length > 0 %} <h2>{% trans %}Analysis Spoilers{% endtrans %}</h2>
From the point of view of the contestants I think it's a bit silly to make a name distinction between regular attachments and spoilers/analysis attachments. How about just having a single list, with analysis attachments at the beginning and in bold, for example?
Introduce analysis mode spoilers (task attachments visible to contestants only in analysis mode).
This can be used for providing additional (or all) test data, sample solutions or anything
that contestants are not allowed to access during the contest, but only during analysis.
This change is