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. |
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.
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.
…t samples.zip" This reverts commit d83c09eac76fecd52841e6d8d8c3eec74ee8fe95.
Probably we can even do this with one by using leftJoins and checking if any of the columns we need become NON NULL but that makes it a lot harder to get right in the next iteration.
vmcj
force-pushed
the
all_samples_team
branch
from
75f5988 to
77928f7
Compare
vmcj
force-pushed
the
all_samples_team
branch
from
da7b0e8 to
01b754d
Compare
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.