Exclude samples for interactive problems. #2718
Conversation
(cherry picked from commit 0cc0da0)
🔍 Existing Issues For ReviewYour pull request is modifying functions with the following pre-existing issues: 📄 File: webapp/src/Service/DOMJudgeService.php
Did you find this useful? React with a 👍 or 👎 |
|
Wouldn't it be nicer if we do not allow downloading a sample zip for interactive problems, instead of supplying an empty zip? |
|
This code path is the overall zip with all samples, and we still include the attachments, so it is not empty. |
| @@ -881,7 +881,10 @@ public function getSamplesZipForContest(Contest $contest): StreamedResponse | |||
|
|
|||
| /** @var ContestProblem $problem */ | |||
| foreach ($contest->getProblems() as $problem) { | |||
| $this->addSamplesToZip($zip, $problem, $problem->getShortname()); | |||
| // We don't include the samples for interactive problems. | |||
| if (!$problem->getProblem()->getCombinedRunCompare()) { | |||
There was a problem hiding this comment.
I'm apparantly in the minority here, but I don't like that we have to add a comment to explain why this function indicates that this is an interactive problem.
Can we make it a TODO to fix this in the future? Either by using a function alias or actually storing if something is interactive?
There was a problem hiding this comment.
Who says you are in the minority :-)
I think it is a good idea to add a function alias.
There was a problem hiding this comment.
I mentioned IRL that I don't like adding that alias, as it hides that under the hood this CombinedRunCompare, which I think is not necessarily exactly the same as an interactive compare script. I think it's better to leave that explicit than make an alias function look like a clean expression of the problem being an inteactive problem. OTOH, if this really 1-1 means that this is an interactive compare script and thus problem, then I think we should just rename that table column and this associated getter.
There was a problem hiding this comment.
Let's address/discuss this outside of this PR.
(cherry picked from commit 0cc0da0)