Skip to content

Commit dda336b

Browse files
krulis-martinkrulis-martin
committed
Adding tests for new download (via link) endpoints and fixing related bugs in the process.
1 parent 931a54b commit dda336b

File tree

8 files changed

+374
-56
lines changed

8 files changed

+374
-56
lines changed

‎app/V1Module/presenters/ExerciseFilesPresenter.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -558,6 +558,7 @@ public function checkUpdateFileLink(string $id, string $linkId)
558558
throw new ForbiddenRequestException("You cannot update exercise file links for this exercise.");
559559
}
560560
}
561+
561562
/**
562563
* Update a specific exercise-file link. Missing arguments are not changed.
563564
* @POST

‎app/V1Module/presenters/UploadedFilesPresenter.php‎

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
use App\Model\Entity\UploadedFile;
3535
use App\Model\Entity\UploadedPartialFile;
3636
use App\Security\Roles;
37+
use App\Security\ACL\IAssignmentPermissions;
3738
use App\Security\ACL\IAssignmentSolutionPermissions;
3839
use App\Security\ACL\IExercisePermissions;
3940
use App\Security\ACL\IUploadedFilePermissions;
@@ -111,6 +112,12 @@ class UploadedFilesPresenter extends BasePresenter
111112
*/
112113
public $assignmentSolutionAcl;
113114

115+
/**
116+
* @var IAssignmentPermissions
117+
* @inject
118+
*/
119+
public $assignmentAcl;
120+
114121
/**
115122
* @var IExercisePermissions
116123
* @inject
@@ -656,7 +663,10 @@ private function checkExerciseFileLink(ExerciseFileLink $link): void
656663
}
657664

658665
// for logged-in users, check exercise access (this is additional check on top of role requirement)
659-
if (!$this->exerciseAcl->canViewDetail($link->getExercise())) {
666+
if ($link->getExercise() !== null && !$this->exerciseAcl->canViewDetail($link->getExercise())) {
667+
throw new ForbiddenRequestException("You cannot download exercise file for this exercise.");
668+
}
669+
if ($link->getAssignment() !== null && !$this->assignmentAcl->canViewDetail($link->getAssignment())) {
660670
throw new ForbiddenRequestException("You cannot download exercise file for this exercise.");
661671
}
662672

@@ -681,14 +691,9 @@ private function downloadFileByLink(ExerciseFileLink $link): void
681691
$this->sendStorageFileResponse($file, $link->getSaveName() ?? $fileEntity->getName());
682692
}
683693

684-
public function checkDownloadExerciseFileByLink(string $id, string $linkId)
694+
public function checkDownloadExerciseFileByLink(string $id)
685695
{
686-
$link = $this->fileLinks->findOrThrow($linkId);
687-
688-
if ($link->getExercise()?->getId() !== $id) {
689-
throw new BadRequestException("The exercise file link is not associated with the given exercise.");
690-
}
691-
696+
$link = $this->fileLinks->findOrThrow($id);
692697
$this->checkExerciseFileLink($link);
693698
}
694699

@@ -697,11 +702,10 @@ public function checkDownloadExerciseFileByLink(string $id, string $linkId)
697702
* This endpoint is deliberately placed in UploadedFilesPresenter so it works for non-logged-in users as well.
698703
* @GET
699704
*/
700-
#[Path("id", new VUuid(), "of exercise", required: true)]
701-
#[Path("linkId", new VUuid(), "of the exercise file link entity", required: true)]
702-
public function actionDownloadExerciseFileByLink(string $id, string $linkId)
705+
#[Path("id", new VUuid(), "of the exercise file link entity", required: true)]
706+
public function actionDownloadExerciseFileByLink(string $id)
703707
{
704-
$link = $this->fileLinks->findOrThrow($linkId);
708+
$link = $this->fileLinks->findOrThrow($id);
705709
$this->downloadFileByLink($link);
706710
}
707711

‎app/V1Module/router/RouterFactory.php‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,6 @@ private static function createExercisesRoutes(string $prefix): RouteList
173173
$router[] = new PostRoute("$prefix/<id>/file-links", "ExerciseFiles:createFileLink");
174174
$router[] = new PostRoute("$prefix/<id>/file-links/<linkId>", "ExerciseFiles:updateFileLink");
175175
$router[] = new DeleteRoute("$prefix/<id>/file-links/<linkId>", "ExerciseFiles:deleteFileLink");
176-
$router[] = new GetRoute("$prefix/<id>/file-links/<linkId>", "UploadedFiles:downloadExerciseFileByLink");
177176

178177
// special download route for file link by its key
179178
$router[] = new GetRoute("$prefix/<id>/file-download/<linkKey>", "UploadedFiles:downloadExerciseFileLinkByKey");
@@ -475,6 +474,8 @@ private static function createUploadedFilesRoutes(string $prefix): RouteList
475474
$router[] = new DeleteRoute("$prefix/partial/<id>", "UploadedFiles:cancelPartial");
476475
$router[] = new PostRoute("$prefix/partial/<id>", "UploadedFiles:completePartial");
477476

477+
$router[] = new GetRoute("$prefix/link/<id>", "UploadedFiles:downloadExerciseFileByLink");
478+
478479
$router[] = new PostRoute("$prefix", "UploadedFiles:upload");
479480
$router[] = new GetRoute("$prefix/<id>", "UploadedFiles:detail");
480481
$router[] = new GetRoute("$prefix/<id>/download", "UploadedFiles:download");

‎fixtures/demo/20-groups.neon‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,11 @@ App\Model\Entity\User:
117117
__construct: ["submitUser1@example.com", "<firstName()>", "<lastName()>", "", "", student, @demoInstance]
118118
makeStudentOf: @demoGroup
119119
setVerified: true
120+
121+
nonmemberStudent:
122+
__construct: ["nonmemberStudent@example.com", "<firstName()>", "<lastName()>", "", "", student, @demoInstance]
123+
setVerified: true
124+
120125

121126
App\Model\Entity\Login:
122127
submitUser1Login:

‎tests/Presenters/ExerciseFilesPresenter.phpt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -714,7 +714,7 @@ class TestExerciseFilesPresenter extends Tester\TestCase
714714
$link = $exercise->getFileLinks()->first();
715715
Assert::truthy($link);
716716

717-
$payload = PresenterTestHelper::performPresenterRequest(
717+
PresenterTestHelper::performPresenterRequest(
718718
$this->presenter,
719719
"V1:ExerciseFiles",
720720
'DELETE',

‎tests/Presenters/GroupsPresenter.phpt‎

Lines changed: 19 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -26,16 +26,7 @@ $_SERVER['REMOTE_ADDR'] = '127.0.0.1';
2626
class TestGroupsPresenter extends Tester\TestCase
2727
{
2828
private $userLogin = "user2@example.com";
29-
private $userPassword = "password2";
30-
3129
private $adminLogin = "admin@admin.com";
32-
private $adminPassword = "admin";
33-
34-
private $groupSupervisorLogin = "demoGroupSupervisor@example.com";
35-
private $groupSupervisorPassword = "password";
36-
37-
private $groupSupervisor2Login = "demoGroupSupervisor2@example.com";
38-
private $groupSupervisor2Password = "password";
3930

4031
/** @var GroupsPresenter */
4132
protected $presenter;
@@ -152,10 +143,7 @@ class TestGroupsPresenter extends Tester\TestCase
152143

153144
public function testListAllGroupsBySupervisor()
154145
{
155-
$token = PresenterTestHelper::login(
156-
$this->container,
157-
$this->groupSupervisorLogin,
158-
);
146+
PresenterTestHelper::login($this->container, PresenterTestHelper::GROUP_SUPERVISOR_LOGIN);
159147
$payload = PresenterTestHelper::performPresenterRequest(
160148
$this->presenter,
161149
'V1:Groups',
@@ -167,7 +155,7 @@ class TestGroupsPresenter extends Tester\TestCase
167155

168156
public function testListAllGroupsByAdmin()
169157
{
170-
$token = PresenterTestHelper::login($this->container, $this->adminLogin);
158+
PresenterTestHelper::loginDefaultAdmin($this->container);
171159
$payload = PresenterTestHelper::performPresenterRequest(
172160
$this->presenter,
173161
'V1:Groups',
@@ -179,7 +167,7 @@ class TestGroupsPresenter extends Tester\TestCase
179167

180168
public function testSearchGroupByName()
181169
{
182-
$token = PresenterTestHelper::login($this->container, $this->adminLogin);
170+
PresenterTestHelper::loginDefaultAdmin($this->container);
183171
$payload = PresenterTestHelper::performPresenterRequest(
184172
$this->presenter,
185173
'V1:Groups',
@@ -191,7 +179,7 @@ class TestGroupsPresenter extends Tester\TestCase
191179

192180
public function testSearchGroupByNameIncludingAncestors()
193181
{
194-
$token = PresenterTestHelper::login($this->container, $this->adminLogin);
182+
PresenterTestHelper::loginDefaultAdmin($this->container);
195183
$payload = PresenterTestHelper::performPresenterRequest(
196184
$this->presenter,
197185
'V1:Groups',
@@ -203,7 +191,7 @@ class TestGroupsPresenter extends Tester\TestCase
203191

204192
public function testListGroupIncludingArchived()
205193
{
206-
$token = PresenterTestHelper::login($this->container, $this->adminLogin);
194+
PresenterTestHelper::loginDefaultAdmin($this->container);
207195
$payload = PresenterTestHelper::performPresenterRequest(
208196
$this->presenter,
209197
'V1:Groups',
@@ -215,7 +203,7 @@ class TestGroupsPresenter extends Tester\TestCase
215203

216204
public function testListGroupOnlyArchived()
217205
{
218-
$token = PresenterTestHelper::login($this->container, $this->adminLogin);
206+
PresenterTestHelper::loginDefaultAdmin($this->container);
219207
$payload = PresenterTestHelper::performPresenterRequest(
220208
$this->presenter,
221209
'V1:Groups',
@@ -390,7 +378,7 @@ class TestGroupsPresenter extends Tester\TestCase
390378

391379
public function testAddGroup()
392380
{
393-
$token = PresenterTestHelper::login($this->container, $this->adminLogin);
381+
PresenterTestHelper::loginDefaultAdmin($this->container);
394382
$admin = $this->container->getByType(Users::class)->getByEmail($this->adminLogin);
395383

396384
/** @var Instance $instance */
@@ -445,7 +433,7 @@ class TestGroupsPresenter extends Tester\TestCase
445433

446434
public function testAddOrganizationalGroup()
447435
{
448-
$token = PresenterTestHelper::login($this->container, $this->adminLogin);
436+
PresenterTestHelper::loginDefaultAdmin($this->container);
449437
$admin = $this->container->getByType(Users::class)->getByEmail($this->adminLogin);
450438

451439
/** @var Instance $instance */
@@ -501,7 +489,7 @@ class TestGroupsPresenter extends Tester\TestCase
501489

502490
public function testAddExamGroup()
503491
{
504-
$token = PresenterTestHelper::login($this->container, $this->adminLogin);
492+
PresenterTestHelper::loginDefaultAdmin($this->container);
505493
$admin = $this->container->getByType(Users::class)->getByEmail($this->adminLogin);
506494

507495
/** @var Instance $instance */
@@ -558,7 +546,7 @@ class TestGroupsPresenter extends Tester\TestCase
558546

559547
public function testAddGroupNoAdmin()
560548
{
561-
$token = PresenterTestHelper::login($this->container, $this->adminLogin);
549+
PresenterTestHelper::loginDefaultAdmin($this->container);
562550
$admin = $this->container->getByType(Users::class)->getByEmail($this->adminLogin);
563551

564552
/** @var Instance $instance */
@@ -613,7 +601,7 @@ class TestGroupsPresenter extends Tester\TestCase
613601

614602
public function testValidateAddGroupData()
615603
{
616-
$token = PresenterTestHelper::login($this->container, $this->adminLogin);
604+
PresenterTestHelper::loginDefaultAdmin($this->container);
617605

618606
$instance = $this->presenter->instances->findAll()[0];
619607

@@ -755,7 +743,7 @@ class TestGroupsPresenter extends Tester\TestCase
755743

756744
public function testRemoveGroup()
757745
{
758-
$token = PresenterTestHelper::login($this->container, $this->adminLogin);
746+
PresenterTestHelper::loginDefaultAdmin($this->container);
759747

760748
$instance = $this->presenter->instances->findAll()[0];
761749
$groups = $this->presenter->groups->findAll();
@@ -779,7 +767,7 @@ class TestGroupsPresenter extends Tester\TestCase
779767

780768
public function testDetail()
781769
{
782-
$token = PresenterTestHelper::login($this->container, $this->adminLogin);
770+
PresenterTestHelper::loginDefaultAdmin($this->container);
783771

784772
$groups = $this->presenter->groups->findAll();
785773
$group = array_pop($groups);
@@ -800,7 +788,7 @@ class TestGroupsPresenter extends Tester\TestCase
800788

801789
public function testSubgroups()
802790
{
803-
$token = PresenterTestHelper::login($this->container, $this->adminLogin);
791+
PresenterTestHelper::loginDefaultAdmin($this->container);
804792

805793
$groups = $this->presenter->groups->findAll();
806794
$group = array_pop($groups);
@@ -821,7 +809,7 @@ class TestGroupsPresenter extends Tester\TestCase
821809

822810
public function testMembers()
823811
{
824-
$token = PresenterTestHelper::login($this->container, $this->adminLogin);
812+
PresenterTestHelper::loginDefaultAdmin($this->container);
825813

826814
$groups = $this->presenter->groups->findAll();
827815
$group = array_pop($groups);
@@ -851,7 +839,7 @@ class TestGroupsPresenter extends Tester\TestCase
851839

852840
public function testAssignments()
853841
{
854-
$token = PresenterTestHelper::login($this->container, $this->adminLogin);
842+
PresenterTestHelper::loginDefaultAdmin($this->container);
855843

856844
$groups = $this->presenter->groups->findAll();
857845
foreach ($groups as $group) {
@@ -871,7 +859,7 @@ class TestGroupsPresenter extends Tester\TestCase
871859

872860
public function testShadowAssignments()
873861
{
874-
$token = PresenterTestHelper::login($this->container, $this->adminLogin);
862+
PresenterTestHelper::loginDefaultAdmin($this->container);
875863

876864
$groups = $this->presenter->groups->findAll();
877865
foreach ($groups as $group) {
@@ -1450,8 +1438,8 @@ class TestGroupsPresenter extends Tester\TestCase
14501438

14511439
private function prepExamGroup(): Group
14521440
{
1453-
PresenterTestHelper::login($this->container, $this->groupSupervisor2Login);
1454-
$admin = $this->presenter->users->getByEmail($this->groupSupervisor2Login);
1441+
PresenterTestHelper::login($this->container, PresenterTestHelper::GROUP_SUPERVISOR2_LOGIN);
1442+
$admin = $this->presenter->users->getByEmail(PresenterTestHelper::GROUP_SUPERVISOR2_LOGIN);
14551443
$groups = $this->getAllGroupsInDepth(
14561444
2,
14571445
function (Group $g) {

0 commit comments

Comments
 (0)