Skip to content

Commit 427a010

Browse files
krulis-martinkrulis-martin
committed
Moving updates of localized texts of an assignment to a separate endpoint. Student hints remain in the update-detail.
1 parent 0f873b8 commit 427a010

File tree

3 files changed

+140
-75
lines changed

3 files changed

+140
-75
lines changed

‎app/V1Module/presenters/AssignmentsPresenter.php‎

Lines changed: 85 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -211,7 +211,11 @@ public function checkUpdateDetail(string $id)
211211
*/
212212
#[Post("version", new VInt(), "Version of the edited assignment")]
213213
#[Post("isPublic", new VBool(), "Is the assignment ready to be displayed to students?")]
214-
#[Post("localizedTexts", new VArray(), "A description of the assignment")]
214+
#[Post(
215+
"localizedStudentHints",
216+
new VArray(),
217+
"Additional localized hint texts for students (locale => hint text)",
218+
)]
215219
#[Post("firstDeadline", new VTimestamp(), "First deadline for submission of the assignment")]
216220
#[Post(
217221
"maxPointsBeforeFirstDeadline",
@@ -307,10 +311,6 @@ public function actionUpdateDetail(string $id)
307311
);
308312
}
309313

310-
// localized texts cannot be empty
311-
if (count($req->getPost("localizedTexts")) == 0) {
312-
throw new InvalidApiArgumentException('localizedTexts', "No entry for localized texts given.");
313-
}
314314

315315
if ($this->isRequestJson()) {
316316
$disabledRuntimeIds = $req->getPost("disabledRuntimeEnvironmentIds");
@@ -425,49 +425,17 @@ public function actionUpdateDetail(string $id)
425425
$this->solutionEvaluations->flush();
426426
}
427427

428-
// go through localizedTexts and construct database entities
429-
$localizedTexts = [];
428+
// go through localized hints and construct database entities
430429
$localizedAssignments = [];
431-
foreach ($req->getPost("localizedTexts") as $localization) {
432-
$lang = $localization["locale"];
433-
434-
if (array_key_exists($lang, $localizedTexts)) {
435-
throw new InvalidApiArgumentException('localizedTexts', "Duplicate entry for language '$lang'");
436-
}
437-
438-
// create all new localized texts
439-
$assignmentExercise = $assignment->getExercise();
440-
$localizedExercise = $assignmentExercise ? $assignmentExercise->getLocalizedTextByLocale($lang) : null;
441-
$externalAssignmentLink = trim(Arrays::get($localization, "link", ""));
442-
if ($externalAssignmentLink !== "" && !Validators::isUrl($externalAssignmentLink)) {
443-
throw new InvalidApiArgumentException('link', "External assignment link is not a valid URL");
444-
}
445-
446-
$localizedTexts[$lang] = new LocalizedExercise(
430+
foreach ($req->getPost("localizedStudentHints") ?? [] as $lang => $hintText) {
431+
$localizedAssignments[$lang] = new LocalizedAssignment(
447432
$lang,
448-
trim(Arrays::get($localization, "name", "")),
449-
trim(Arrays::get($localization, "text", "")),
450-
$localizedExercise ? $localizedExercise->getDescription() : "",
451-
$externalAssignmentLink ?: null
433+
trim($hintText)
452434
);
453-
454-
if (array_key_exists("studentHint", $localization)) {
455-
$localizedAssignments[$lang] = new LocalizedAssignment(
456-
$lang,
457-
trim(Arrays::get($localization, "studentHint", ""))
458-
);
459-
}
460-
}
461-
462-
// make changes to database
463-
Localizations::updateCollection($assignment->getLocalizedTexts(), $localizedTexts);
464-
465-
foreach ($assignment->getLocalizedTexts() as $localizedText) {
466-
$this->assignments->persist($localizedText, false);
467435
}
468436

437+
// save changes to database (if any)
469438
Localizations::updateCollection($assignment->getLocalizedAssignments(), $localizedAssignments);
470-
471439
foreach ($assignment->getLocalizedAssignments() as $localizedAssignment) {
472440
$this->assignments->persist($localizedAssignment, false);
473441
}
@@ -501,7 +469,81 @@ public function actionUpdateDetail(string $id)
501469
}
502470
}
503471

504-
$this->assignments->flush();
472+
$this->assignments->persist($assignment);
473+
$this->sendSuccessResponse($this->assignmentViewFactory->getAssignment($assignment));
474+
}
475+
476+
public function checkUpdateLocalizedTexts(string $id)
477+
{
478+
$assignment = $this->assignments->findOrThrow($id);
479+
if (!$this->assignmentAcl->canUpdate($assignment)) {
480+
throw new ForbiddenRequestException("You cannot update this assignment.");
481+
}
482+
}
483+
484+
/**
485+
* Update (only) the localized texts of an assignment.
486+
* This is a separate operations since the texts are taken over from the exercise.
487+
* Updating them is an override of the exercise specification and needs to be handled carefully.
488+
* @POST
489+
*/
490+
#[Post("version", new VInt(), "Version of the edited assignment")]
491+
#[Post("localizedTexts", new VArray(), "Localized texts with exercise/assignment specification")]
492+
#[Path("id", new VUuid(), "Identifier of the updated assignment", required: true)]
493+
public function actionUpdateLocalizedTexts(string $id)
494+
{
495+
$assignment = $this->assignments->findOrThrow($id);
496+
497+
$req = $this->getRequest();
498+
$version = (int)$req->getPost("version");
499+
if ($version !== $assignment->getVersion()) {
500+
$newVer = $assignment->getVersion();
501+
throw new BadRequestException(
502+
"The assignment was edited in the meantime and the version has changed. Current version is $newVer.",
503+
FrontendErrorMappings::E400_010__ENTITY_VERSION_TOO_OLD,
504+
[
505+
'entity' => 'assignment',
506+
'id' => $id,
507+
'version' => $newVer
508+
]
509+
);
510+
}
511+
512+
// go through localizedTexts and construct database entities
513+
$localizedTexts = [];
514+
foreach ($req->getPost("localizedTexts") as $localization) {
515+
$lang = $localization["locale"];
516+
517+
if (array_key_exists($lang, $localizedTexts)) {
518+
throw new InvalidApiArgumentException('localizedTexts', "Duplicate entry for language '$lang'");
519+
}
520+
521+
// create all new localized texts
522+
$assignmentExercise = $assignment->getExercise();
523+
$localizedExercise = $assignmentExercise ? $assignmentExercise->getLocalizedTextByLocale($lang) : null;
524+
$externalAssignmentLink = trim(Arrays::get($localization, "link", ""));
525+
if ($externalAssignmentLink !== "" && !Validators::isUrl($externalAssignmentLink)) {
526+
throw new InvalidApiArgumentException('link', "External assignment link is not a valid URL");
527+
}
528+
529+
$localizedTexts[$lang] = new LocalizedExercise(
530+
$lang,
531+
trim(Arrays::get($localization, "name", "")),
532+
trim(Arrays::get($localization, "text", "")),
533+
$localizedExercise ? $localizedExercise->getDescription() : "",
534+
$externalAssignmentLink ?: null
535+
);
536+
}
537+
538+
// make changes to database
539+
Localizations::updateCollection($assignment->getLocalizedTexts(), $localizedTexts);
540+
foreach ($assignment->getLocalizedTexts() as $localizedText) {
541+
$this->assignments->persist($localizedText, false);
542+
}
543+
544+
$assignment->incrementVersion();
545+
$assignment->updatedNow();
546+
$this->assignments->persist($assignment);
505547
$this->sendSuccessResponse($this->assignmentViewFactory->getAssignment($assignment));
506548
}
507549

‎app/V1Module/router/RouterFactory.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -236,6 +236,7 @@ private static function createAssignmentsRoutes(string $prefix): RouteList
236236
$router[] = new PostRoute("$prefix", "Assignments:create");
237237
$router[] = new GetRoute("$prefix/<id>", "Assignments:detail");
238238
$router[] = new PostRoute("$prefix/<id>", "Assignments:updateDetail");
239+
$router[] = new PostRoute("$prefix/<id>/localized-texts", "Assignments:updateLocalizedTexts");
239240
$router[] = new DeleteRoute("$prefix/<id>", "Assignments:remove");
240241
$router[] = new GetRoute("$prefix/<id>/solutions", "Assignments:solutions");
241242
$router[] = new GetRoute("$prefix/<id>/best-solutions", "Assignments:bestSolutions");

‎tests/Presenters/AssignmentsPresenter.phpt‎

Lines changed: 54 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -145,8 +145,8 @@ class TestAssignmentsPresenter extends Tester\TestCase
145145
$this->presenter->solutionEvaluations = $mockEvaluations;
146146

147147
$isPublic = true;
148-
$localizedTexts = [
149-
["locale" => "locA", "text" => "descA", "name" => "nameA"]
148+
$localizedStudentHints = [
149+
"en" => "Use the force!",
150150
];
151151
$firstDeadline = (new DateTime())->getTimestamp();
152152
$maxPointsBeforeFirstDeadline = 123;
@@ -171,7 +171,7 @@ class TestAssignmentsPresenter extends Tester\TestCase
171171
[
172172
'isPublic' => $isPublic,
173173
'version' => 1,
174-
'localizedTexts' => $localizedTexts,
174+
'localizedStudentHints' => $localizedStudentHints,
175175
'firstDeadline' => $firstDeadline,
176176
'maxPointsBeforeFirstDeadline' => $maxPointsBeforeFirstDeadline,
177177
'submissionsCountLimit' => $submissionsCountLimit,
@@ -218,10 +218,10 @@ class TestAssignmentsPresenter extends Tester\TestCase
218218

219219
// check localized texts
220220
Assert::count(1, $updatedAssignment["localizedTexts"]);
221-
$localized = current($localizedTexts);
221+
$localized = current($localizedStudentHints);
222222
$updatedLocalized = $updatedAssignment["localizedTexts"][0];
223-
Assert::equal($updatedLocalized["locale"], $localized["locale"]);
224-
Assert::equal($updatedLocalized["text"], $localized["text"]);
223+
Assert::equal("en", $updatedLocalized["locale"]);
224+
Assert::equal($localized, $updatedLocalized["studentHint"]);
225225
}
226226

227227
public function testUpdateDetailWithoutNotifications()
@@ -249,9 +249,7 @@ class TestAssignmentsPresenter extends Tester\TestCase
249249
'isPublic' => true,
250250
'version' => 1,
251251
'sendNotification' => false,
252-
'localizedTexts' => [
253-
["locale" => "locA", "text" => "descA", "name" => "nameA"]
254-
],
252+
'localizedStudentHints' => ["en" => "Use the force!"],
255253
'firstDeadline' => (new DateTime())->getTimestamp(),
256254
'maxPointsBeforeFirstDeadline' => 42,
257255
'submissionsCountLimit' => 10,
@@ -303,9 +301,7 @@ class TestAssignmentsPresenter extends Tester\TestCase
303301
$this->presenter->solutionEvaluations = $mockEvaluations;
304302

305303
$isPublic = true;
306-
$localizedTexts = [
307-
["locale" => "locA", "text" => "descA", "name" => "nameA"]
308-
];
304+
$localizedStudentHints = ["en" => "Use the force!"];
309305
$firstDeadline = (new DateTime())->getTimestamp() + 100;
310306
$maxPointsBeforeFirstDeadline = 123;
311307
$submissionsCountLimit = 32;
@@ -330,7 +326,7 @@ class TestAssignmentsPresenter extends Tester\TestCase
330326
'isPublic' => $isPublic,
331327
'version' => 1,
332328
'sendNotification' => true,
333-
'localizedTexts' => $localizedTexts,
329+
'localizedStudentHints' => $localizedStudentHints,
334330
'firstDeadline' => $firstDeadline,
335331
'maxPointsBeforeFirstDeadline' => $maxPointsBeforeFirstDeadline,
336332
'submissionsCountLimit' => $submissionsCountLimit,
@@ -377,10 +373,10 @@ class TestAssignmentsPresenter extends Tester\TestCase
377373

378374
// check localized texts
379375
Assert::count(1, $updatedAssignment["localizedTexts"]);
380-
$localized = current($localizedTexts);
376+
$localized = current($localizedStudentHints);
381377
$updatedLocalized = $updatedAssignment["localizedTexts"][0];
382-
Assert::equal($updatedLocalized["locale"], $localized["locale"]);
383-
Assert::equal($updatedLocalized["text"], $localized["text"]);
378+
Assert::equal("en", $updatedLocalized["locale"]);
379+
Assert::equal($localized, $updatedLocalized["studentHint"]);
384380
}
385381

386382
public function testAddStudentHints()
@@ -399,9 +395,7 @@ class TestAssignmentsPresenter extends Tester\TestCase
399395
[
400396
'isPublic' => true,
401397
'version' => 1,
402-
'localizedTexts' => [
403-
["locale" => "locA", "text" => "descA", "name" => "nameA", "studentHint" => "Try hard"]
404-
],
398+
'localizedStudentHints' => ["en" => "Try hard"],
405399
'firstDeadline' => (new DateTime())->getTimestamp(),
406400
'maxPointsBeforeFirstDeadline' => 123,
407401
'submissionsCountLimit' => 32,
@@ -424,7 +418,7 @@ class TestAssignmentsPresenter extends Tester\TestCase
424418
$response = $this->presenter->run($request);
425419
$updatedAssignment = PresenterTestHelper::extractPayload($response);
426420
Assert::count(1, $updatedAssignment["localizedTexts"]);
427-
Assert::equal("locA", $updatedAssignment["localizedTexts"][0]["locale"]);
421+
Assert::equal("en", $updatedAssignment["localizedTexts"][0]["locale"]);
428422
Assert::equal("Try hard", $updatedAssignment["localizedTexts"][0]["studentHint"]);
429423
Assert::true($updatedAssignment["maxPointsDeadlineInterpolation"]);
430424
}
@@ -446,9 +440,7 @@ class TestAssignmentsPresenter extends Tester\TestCase
446440
[
447441
'isPublic' => true,
448442
'version' => 1,
449-
'localizedTexts' => [
450-
["locale" => "locA", "text" => "descA", "name" => "nameA"]
451-
],
443+
'localizedStudentHints' => ["en" => "Use the force!"],
452444
'firstDeadline' => (new DateTime())->getTimestamp(),
453445
'maxPointsBeforeFirstDeadline' => 123,
454446
'submissionsCountLimit' => 32,
@@ -491,9 +483,7 @@ class TestAssignmentsPresenter extends Tester\TestCase
491483
[
492484
'isPublic' => true,
493485
'version' => 1,
494-
'localizedTexts' => [
495-
["locale" => "locA", "text" => "descA", "name" => "nameA"]
496-
],
486+
'localizedStudentHints' => ["en" => "Use the force!"],
497487
'firstDeadline' => (new DateTime())->getTimestamp(),
498488
'maxPointsBeforeFirstDeadline' => 123,
499489
'submissionsCountLimit' => 32,
@@ -540,9 +530,7 @@ class TestAssignmentsPresenter extends Tester\TestCase
540530
[
541531
'isPublic' => true,
542532
'version' => 1,
543-
'localizedTexts' => [
544-
["locale" => "locA", "text" => "descA", "name" => "nameA"]
545-
],
533+
'localizedStudentHints' => ["en" => "Use the force!"],
546534
'firstDeadline' => (new DateTime())->getTimestamp(),
547535
'maxPointsBeforeFirstDeadline' => 123,
548536
'submissionsCountLimit' => 32,
@@ -587,9 +575,7 @@ class TestAssignmentsPresenter extends Tester\TestCase
587575
[
588576
'isPublic' => true,
589577
'version' => 1,
590-
'localizedTexts' => [
591-
["locale" => "locA", "text" => "descA", "name" => "nameA"]
592-
],
578+
'localizedStudentHints' => ["en" => "Use the force!"],
593579
'firstDeadline' => (new DateTime())->getTimestamp(),
594580
'maxPointsBeforeFirstDeadline' => 123,
595581
'submissionsCountLimit' => 32,
@@ -614,6 +600,42 @@ class TestAssignmentsPresenter extends Tester\TestCase
614600
Assert::false($assignment->isExam());
615601
}
616602

603+
public function testUpdateLocalizedTexts()
604+
{
605+
PresenterTestHelper::loginDefaultAdmin($this->container);
606+
607+
$assignments = $this->assignments->findAll();
608+
/** @var Assignment $assignment */
609+
$assignment = array_pop($assignments);
610+
611+
$request = new Nette\Application\Request(
612+
'V1:Assignments',
613+
'POST',
614+
['action' => 'updateLocalizedTexts', 'id' => $assignment->getId()],
615+
[
616+
'version' => 1,
617+
'localizedTexts' => [
618+
["locale" => "en", "name" => "Overwritten name", "text" => "Overwritten text"],
619+
["locale" => "nw", "name" => "New name", "text" => "New text"],
620+
],
621+
]
622+
);
623+
624+
$response = $this->presenter->run($request);
625+
$updatedAssignment = PresenterTestHelper::extractPayload($response);
626+
Assert::count(2, $updatedAssignment["localizedTexts"]);
627+
$texts = $updatedAssignment["localizedTexts"];
628+
usort($texts, function ($a, $b) {
629+
return strcmp($a["locale"], $b["locale"]);
630+
});
631+
Assert::equal("en", $texts[0]["locale"]);
632+
Assert::equal("Overwritten name", $texts[0]["name"]);
633+
Assert::equal("Overwritten text", $texts[0]["text"]);
634+
Assert::equal("nw", $texts[1]["locale"]);
635+
Assert::equal("New name", $texts[1]["name"]);
636+
Assert::equal("New text", $texts[1]["text"]);
637+
}
638+
617639
public function testCreateAssignment()
618640
{
619641
PresenterTestHelper::loginDefaultAdmin($this->container);

0 commit comments

Comments
 (0)