Allow teams to download all samples/attachments #3193
Conversation
vmcj
force-pushed
the
all_samples_team
branch
from
9b9cc50 to
64d2627
Compare
|
I would prefer adding a check if it exists, I don't think it's very nice to add buttons that don't always work. Even if it is very rare. |
We already allow this over the API so use the same URL.
…s.zip We always try to exit early and skip the actual adding. As checking and creation are more or less the same code we wrap the check inside the actual creation to prevent duplication.
vmcj
force-pushed
the
all_samples_team
branch
from
64d2627 to
75f5988
Compare
| if (!($tempFilename = tempnam($this->getDomjudgeTmpDir(), "export-"))) { | ||
| throw new ServiceUnavailableHttpException(null, 'Could not create temporary file.'); | ||
| } | ||
| $zip = null; |
There was a problem hiding this comment.
I get what you're doing, but I think just doing a simple query to count the number of sample testcases across all problems in the contest in checkIfSamplesZipForContest is cleaner than this (and more performant)
There was a problem hiding this comment.
Yes, but in case you have no samples but do have attachments you would not create the zip. But I wonder if this will ever return false, why would there be a contest with neither of:
- samples,
- problem statements
- attachments
All of those are possible, but having neither of those files really feels strange.
There was a problem hiding this comment.
The zip with all samples contains only samples or also attachment? In the latter case the button has a weird name maybe, not sure.
But still I would not mis-use a method like this.
There was a problem hiding this comment.
All of those are possible, but having neither of those files really feels strange.
When everything is provided on the team machine and/or as hardcopy problem statements. I don't think that's a far-fetched case.
3 participants
We already allow this over the API so use the same URL.
This doesn't check if there are actually samples as I think it will in 99% of the cases and in case there are none someone would just get a 404.