From 374e11800fc8ce6f3c79f9419225c75d327b84b7 Mon Sep 17 00:00:00 2001
From: Marcus Eibrink-Lunzenauer <lunzenauer@elan-ev.de>
Date: Wed, 30 Oct 2024 10:49:20 +0000
Subject: [PATCH] Apply Jan's code review suggestions
---
app/controllers/admin/courseware.php | 54 ++++++++++---------
app/views/admin/courseware/block_types.php | 39 +++++++++-----
.../admin/courseware/container_types.php | 39 +++++++++-----
3 files changed, 80 insertions(+), 52 deletions(-)
diff --git a/app/controllers/admin/courseware.php b/app/controllers/admin/courseware.php
index aa92176e242..7940bff0496 100644
--- a/app/controllers/admin/courseware.php
+++ b/app/controllers/admin/courseware.php
@@ -37,23 +37,24 @@ class Admin_CoursewareController extends AuthenticatedController
$this->blockTypes = BlockType::getBlockTypes();
- usort($this->blockTypes, function ($blockTypeA, $blockTypeB) {
- return $blockTypeA::getTitle() <=> $blockTypeB::getTitle();
- });
+ usort(
+ $this->blockTypes,
+ fn ($blockTypeA, $blockTypeB) => $blockTypeA::getTitle() <=> $blockTypeB::getTitle()
+ );
}
public function bulk_block_types_action()
{
CSRFProtection::verifyUnsafeRequest();
- switch (Request::quoted('bulk_action')) {
+ switch (Request::option('bulk_action')) {
case 'activate':
return $this->activate_block_types_action();
case 'deactivate':
return $this->deactivate_block_types_action();
}
- PageLayout::postInfo(_("Keine Aktion ausgewählt."));
+ PageLayout::postInfo(_('Keine Aktion ausgewählt.'));
$this->redirect($this->action_url('block_types'));
}
@@ -65,9 +66,10 @@ class Admin_CoursewareController extends AuthenticatedController
$requestedBlockTypes = $this->validateBlockTypes();
$changed = array_sum(
- array_map(function ($blockType) {
- return $blockType::activate() ? 1 : 0;
- }, $requestedBlockTypes)
+ array_map(
+ fn ($blockType) => $blockType::activate() ? 1 : 0,
+ $requestedBlockTypes)
+ )
);
PageLayout::postSuccess(
@@ -87,9 +89,10 @@ class Admin_CoursewareController extends AuthenticatedController
$requestedBlockTypes = $this->validateBlockTypes();
$changed = array_sum(
- array_map(function ($blockType) {
- return $blockType::deactivate() ? 1 : 0;
- }, $requestedBlockTypes)
+ array_map(
+ fn ($blockType) => $blockType::deactivate() ? 1 : 0,
+ $requestedBlockTypes
+ )
);
PageLayout::postSuccess(
@@ -105,8 +108,8 @@ class Admin_CoursewareController extends AuthenticatedController
{
$requestedBlockTypes = Request::getArray('block_types');
$diff = array_diff($requestedBlockTypes, BlockType::getBlockTypes());
- if (count($diff)) {
- throw new Trails_Exception(400);
+ if (count($diff) > 0) {
+ throw new Trails\Exception(400);
}
return $requestedBlockTypes;
@@ -123,9 +126,10 @@ class Admin_CoursewareController extends AuthenticatedController
$this->containerTypes = ContainerType::getContainerTypes();
- usort($this->containerTypes, function ($containerTypeA, $containerTypeB) {
- return $containerTypeA::getTitle() <=> $containerTypeB::getTitle();
- });
+ usort(
+ $this->containerTypes,
+ fn ($containerTypeA, $containerTypeB) => $containerTypeA::getTitle() <=> $containerTypeB::getTitle(),
+ );
}
public function bulk_container_types_action()
@@ -139,7 +143,7 @@ class Admin_CoursewareController extends AuthenticatedController
return $this->deactivate_container_types_action();
}
- PageLayout::postInfo(_("Keine Aktion ausgewählt."));
+ PageLayout::postInfo(_('Keine Aktion ausgewählt.'));
$this->redirect($this->action_url('container_types'));
}
@@ -151,9 +155,10 @@ class Admin_CoursewareController extends AuthenticatedController
$requestedContainerTypes = $this->validateContainerTypes();
$changed = array_sum(
- array_map(function ($containerType) {
- return $containerType::activate() ? 1 : 0;
- }, $requestedContainerTypes)
+ array_map(
+ fn ($containerType) => $containerType::activate() ? 1 : 0,
+ $requestedContainerTypes
+ )
);
PageLayout::postSuccess(
@@ -173,9 +178,10 @@ class Admin_CoursewareController extends AuthenticatedController
$requestedContainerTypes = $this->validateContainerTypes();
$changed = array_sum(
- array_map(function ($containerType) {
- return $containerType::deactivate() ? 1 : 0;
- }, $requestedContainerTypes)
+ array_map(
+ fn ($containerType) => $containerType::deactivate() ? 1 : 0,
+ $requestedContainerTypes
+ )
);
PageLayout::postSuccess(
@@ -192,7 +198,7 @@ class Admin_CoursewareController extends AuthenticatedController
$requestedContainerTypes = Request::getArray('container_types');
$diff = array_diff($requestedContainerTypes, ContainerType::getContainerTypes());
if (count($diff)) {
- throw new Trails_Exception(400);
+ throw new Trails\Exception(400);
}
return $requestedContainerTypes;
diff --git a/app/views/admin/courseware/block_types.php b/app/views/admin/courseware/block_types.php
index b4a5d4637f3..bd30b8cbb60 100644
--- a/app/views/admin/courseware/block_types.php
+++ b/app/views/admin/courseware/block_types.php
@@ -29,12 +29,12 @@
</tr>
</thead>
<tbody>
- <? foreach ($blockTypes as $blockType) { ?>
+ <? foreach ($blockTypes as $blockType): ?>
<? $isActivated = $blockType::isActivated(); ?>
<tr class="<?= $isActivated ? 'activated' : '' ?>">
<td>
<label>
- <span class="sr-only"><? printf(_('Blocktyp "%s" auswählen'), $blockType::getTitle()) ?></span>
+ <span class="sr-only"><? printf(_('Blocktyp "%s" auswählen'), htmlReady($blockType::getTitle())) ?></span>
<input type="checkbox" name="block_types[]" value="<?= htmlReady($blockType) ?>" />
</label>
</td>
@@ -57,21 +57,32 @@
->setContext(sprintf(_('Courseware-Blocktyp "%s"'), $blockType::getTitle()))
->setRenderingMode(ActionMenu::RENDERING_MODE_MENU)
->condition($isActivated)
- ->addButton('deactivate_block_type', _('Deaktivieren'), Icon::create('remove'), [
- 'formaction' => URLHelper::getURL(
- 'dispatch.php/admin/courseware/deactivate_block_types',
- ['block_types' => [$blockType]]
- ),
- ])
+ ->addButton(
+ 'deactivate_block_type',
+ _('Deaktivieren'),
+ Icon::create('remove'),
+ [
+ 'formaction' => URLHelper::getURL(
+ 'dispatch.php/admin/courseware/deactivate_block_types',
+ ['block_types' => [$blockType]]
+ ),
+ ]
+ )
->condition(!$isActivated)
- ->addButton('activate_block_type', _('Aktivieren'), Icon::create('add'), [
- 'formaction' => URLHelper::getURL('dispatch.php/admin/courseware/activate_block_types', [
- 'block_types' => [$blockType],
- ]),
- ]) ?>
+ ->addButton(
+ 'activate_block_type',
+ _('Aktivieren'),
+ Icon::create('add'),
+ [
+ 'formaction' => URLHelper::getURL(
+ 'dispatch.php/admin/courseware/activate_block_types',
+ ['block_types' => [$blockType]]
+ ),
+ ]
+ ) ?>
</td>
</tr>
- <? } ?>
+ <? endforeach; ?>
</tbody>
</table>
</form>
diff --git a/app/views/admin/courseware/container_types.php b/app/views/admin/courseware/container_types.php
index eb0d4995000..46b50662a02 100644
--- a/app/views/admin/courseware/container_types.php
+++ b/app/views/admin/courseware/container_types.php
@@ -29,12 +29,12 @@
</tr>
</thead>
<tbody>
- <? foreach ($containerTypes as $containerType) { ?>
+ <? foreach ($containerTypes as $containerType): ?>
<? $isActivated = $containerType::isActivated(); ?>
<tr class="<?= $isActivated ? 'activated' : '' ?>">
<td>
<label>
- <span class="sr-only"><? printf(_('Containertyp "%s" auswählen'), $containerType::getTitle()) ?></span>
+ <span class="sr-only"><? printf(_('Containertyp "%s" auswählen'), htmlReady($containerType::getTitle())) ?></span>
<input type="checkbox" name="container_types[]" value="<?= htmlReady($containerType) ?>" />
</label>
</td>
@@ -57,21 +57,32 @@
->setContext(sprintf(_('Courseware-Containertyp "%s"'), $containerType::getTitle()))
->setRenderingMode(ActionMenu::RENDERING_MODE_MENU)
->condition($isActivated)
- ->addButton('deactivate_container_type', _('Deaktivieren'), Icon::create('remove'), [
- 'formaction' => URLHelper::getURL(
- 'dispatch.php/admin/courseware/deactivate_container_types',
- ['container_types' => [$containerType]]
- ),
- ])
+ ->addButton(
+ 'deactivate_container_type',
+ _('Deaktivieren'),
+ Icon::create('remove'),
+ [
+ 'formaction' => URLHelper::getURL(
+ 'dispatch.php/admin/courseware/deactivate_container_types',
+ ['container_types' => [$containerType]]
+ ),
+ ]
+ )
->condition(!$isActivated)
- ->addButton('activate_container_type', _('Aktivieren'), Icon::create('add'), [
- 'formaction' => URLHelper::getURL('dispatch.php/admin/courseware/activate_container_types', [
- 'container_types' => [$containerType],
- ]),
- ]) ?>
+ ->addButton(
+ 'activate_container_type',
+ _('Aktivieren'),
+ Icon::create('add'),
+ [
+ 'formaction' => URLHelper::getURL(
+ 'dispatch.php/admin/courseware/activate_container_types',
+ ['container_types' => [$containerType]]
+ ),
+ ]
+ ) ?>
</td>
</tr>
- <? } ?>
+ <? endforeach; ?>
</tbody>
</table>
</form>
--
GitLab