Skip to content
Snippets Groups Projects
Commit 374e1180 authored by Marcus Eibrink-Lunzenauer's avatar Marcus Eibrink-Lunzenauer
Browse files

Apply Jan's code review suggestions

parent 035288dc
No related branches found
No related tags found
No related merge requests found
Pipeline #28349 failed
...@@ -37,23 +37,24 @@ class Admin_CoursewareController extends AuthenticatedController ...@@ -37,23 +37,24 @@ class Admin_CoursewareController extends AuthenticatedController
$this->blockTypes = BlockType::getBlockTypes(); $this->blockTypes = BlockType::getBlockTypes();
usort($this->blockTypes, function ($blockTypeA, $blockTypeB) { usort(
return $blockTypeA::getTitle() <=> $blockTypeB::getTitle(); $this->blockTypes,
}); fn ($blockTypeA, $blockTypeB) => $blockTypeA::getTitle() <=> $blockTypeB::getTitle()
);
} }
public function bulk_block_types_action() public function bulk_block_types_action()
{ {
CSRFProtection::verifyUnsafeRequest(); CSRFProtection::verifyUnsafeRequest();
switch (Request::quoted('bulk_action')) { switch (Request::option('bulk_action')) {
case 'activate': case 'activate':
return $this->activate_block_types_action(); return $this->activate_block_types_action();
case 'deactivate': case 'deactivate':
return $this->deactivate_block_types_action(); 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')); $this->redirect($this->action_url('block_types'));
} }
...@@ -65,9 +66,10 @@ class Admin_CoursewareController extends AuthenticatedController ...@@ -65,9 +66,10 @@ class Admin_CoursewareController extends AuthenticatedController
$requestedBlockTypes = $this->validateBlockTypes(); $requestedBlockTypes = $this->validateBlockTypes();
$changed = array_sum( $changed = array_sum(
array_map(function ($blockType) { array_map(
return $blockType::activate() ? 1 : 0; fn ($blockType) => $blockType::activate() ? 1 : 0,
}, $requestedBlockTypes) $requestedBlockTypes)
)
); );
PageLayout::postSuccess( PageLayout::postSuccess(
...@@ -87,9 +89,10 @@ class Admin_CoursewareController extends AuthenticatedController ...@@ -87,9 +89,10 @@ class Admin_CoursewareController extends AuthenticatedController
$requestedBlockTypes = $this->validateBlockTypes(); $requestedBlockTypes = $this->validateBlockTypes();
$changed = array_sum( $changed = array_sum(
array_map(function ($blockType) { array_map(
return $blockType::deactivate() ? 1 : 0; fn ($blockType) => $blockType::deactivate() ? 1 : 0,
}, $requestedBlockTypes) $requestedBlockTypes
)
); );
PageLayout::postSuccess( PageLayout::postSuccess(
...@@ -105,8 +108,8 @@ class Admin_CoursewareController extends AuthenticatedController ...@@ -105,8 +108,8 @@ class Admin_CoursewareController extends AuthenticatedController
{ {
$requestedBlockTypes = Request::getArray('block_types'); $requestedBlockTypes = Request::getArray('block_types');
$diff = array_diff($requestedBlockTypes, BlockType::getBlockTypes()); $diff = array_diff($requestedBlockTypes, BlockType::getBlockTypes());
if (count($diff)) { if (count($diff) > 0) {
throw new Trails_Exception(400); throw new Trails\Exception(400);
} }
return $requestedBlockTypes; return $requestedBlockTypes;
...@@ -123,9 +126,10 @@ class Admin_CoursewareController extends AuthenticatedController ...@@ -123,9 +126,10 @@ class Admin_CoursewareController extends AuthenticatedController
$this->containerTypes = ContainerType::getContainerTypes(); $this->containerTypes = ContainerType::getContainerTypes();
usort($this->containerTypes, function ($containerTypeA, $containerTypeB) { usort(
return $containerTypeA::getTitle() <=> $containerTypeB::getTitle(); $this->containerTypes,
}); fn ($containerTypeA, $containerTypeB) => $containerTypeA::getTitle() <=> $containerTypeB::getTitle(),
);
} }
public function bulk_container_types_action() public function bulk_container_types_action()
...@@ -139,7 +143,7 @@ class Admin_CoursewareController extends AuthenticatedController ...@@ -139,7 +143,7 @@ class Admin_CoursewareController extends AuthenticatedController
return $this->deactivate_container_types_action(); 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')); $this->redirect($this->action_url('container_types'));
} }
...@@ -151,9 +155,10 @@ class Admin_CoursewareController extends AuthenticatedController ...@@ -151,9 +155,10 @@ class Admin_CoursewareController extends AuthenticatedController
$requestedContainerTypes = $this->validateContainerTypes(); $requestedContainerTypes = $this->validateContainerTypes();
$changed = array_sum( $changed = array_sum(
array_map(function ($containerType) { array_map(
return $containerType::activate() ? 1 : 0; fn ($containerType) => $containerType::activate() ? 1 : 0,
}, $requestedContainerTypes) $requestedContainerTypes
)
); );
PageLayout::postSuccess( PageLayout::postSuccess(
...@@ -173,9 +178,10 @@ class Admin_CoursewareController extends AuthenticatedController ...@@ -173,9 +178,10 @@ class Admin_CoursewareController extends AuthenticatedController
$requestedContainerTypes = $this->validateContainerTypes(); $requestedContainerTypes = $this->validateContainerTypes();
$changed = array_sum( $changed = array_sum(
array_map(function ($containerType) { array_map(
return $containerType::deactivate() ? 1 : 0; fn ($containerType) => $containerType::deactivate() ? 1 : 0,
}, $requestedContainerTypes) $requestedContainerTypes
)
); );
PageLayout::postSuccess( PageLayout::postSuccess(
...@@ -192,7 +198,7 @@ class Admin_CoursewareController extends AuthenticatedController ...@@ -192,7 +198,7 @@ class Admin_CoursewareController extends AuthenticatedController
$requestedContainerTypes = Request::getArray('container_types'); $requestedContainerTypes = Request::getArray('container_types');
$diff = array_diff($requestedContainerTypes, ContainerType::getContainerTypes()); $diff = array_diff($requestedContainerTypes, ContainerType::getContainerTypes());
if (count($diff)) { if (count($diff)) {
throw new Trails_Exception(400); throw new Trails\Exception(400);
} }
return $requestedContainerTypes; return $requestedContainerTypes;
......
...@@ -29,12 +29,12 @@ ...@@ -29,12 +29,12 @@
</tr> </tr>
</thead> </thead>
<tbody> <tbody>
<? foreach ($blockTypes as $blockType) { ?> <? foreach ($blockTypes as $blockType): ?>
<? $isActivated = $blockType::isActivated(); ?> <? $isActivated = $blockType::isActivated(); ?>
<tr class="<?= $isActivated ? 'activated' : '' ?>"> <tr class="<?= $isActivated ? 'activated' : '' ?>">
<td> <td>
<label> <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) ?>" /> <input type="checkbox" name="block_types[]" value="<?= htmlReady($blockType) ?>" />
</label> </label>
</td> </td>
...@@ -57,21 +57,32 @@ ...@@ -57,21 +57,32 @@
->setContext(sprintf(_('Courseware-Blocktyp "%s"'), $blockType::getTitle())) ->setContext(sprintf(_('Courseware-Blocktyp "%s"'), $blockType::getTitle()))
->setRenderingMode(ActionMenu::RENDERING_MODE_MENU) ->setRenderingMode(ActionMenu::RENDERING_MODE_MENU)
->condition($isActivated) ->condition($isActivated)
->addButton('deactivate_block_type', _('Deaktivieren'), Icon::create('remove'), [ ->addButton(
'deactivate_block_type',
_('Deaktivieren'),
Icon::create('remove'),
[
'formaction' => URLHelper::getURL( 'formaction' => URLHelper::getURL(
'dispatch.php/admin/courseware/deactivate_block_types', 'dispatch.php/admin/courseware/deactivate_block_types',
['block_types' => [$blockType]] ['block_types' => [$blockType]]
), ),
]) ]
)
->condition(!$isActivated) ->condition(!$isActivated)
->addButton('activate_block_type', _('Aktivieren'), Icon::create('add'), [ ->addButton(
'formaction' => URLHelper::getURL('dispatch.php/admin/courseware/activate_block_types', [ 'activate_block_type',
'block_types' => [$blockType], _('Aktivieren'),
]), Icon::create('add'),
]) ?> [
'formaction' => URLHelper::getURL(
'dispatch.php/admin/courseware/activate_block_types',
['block_types' => [$blockType]]
),
]
) ?>
</td> </td>
</tr> </tr>
<? } ?> <? endforeach; ?>
</tbody> </tbody>
</table> </table>
</form> </form>
...@@ -29,12 +29,12 @@ ...@@ -29,12 +29,12 @@
</tr> </tr>
</thead> </thead>
<tbody> <tbody>
<? foreach ($containerTypes as $containerType) { ?> <? foreach ($containerTypes as $containerType): ?>
<? $isActivated = $containerType::isActivated(); ?> <? $isActivated = $containerType::isActivated(); ?>
<tr class="<?= $isActivated ? 'activated' : '' ?>"> <tr class="<?= $isActivated ? 'activated' : '' ?>">
<td> <td>
<label> <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) ?>" /> <input type="checkbox" name="container_types[]" value="<?= htmlReady($containerType) ?>" />
</label> </label>
</td> </td>
...@@ -57,21 +57,32 @@ ...@@ -57,21 +57,32 @@
->setContext(sprintf(_('Courseware-Containertyp "%s"'), $containerType::getTitle())) ->setContext(sprintf(_('Courseware-Containertyp "%s"'), $containerType::getTitle()))
->setRenderingMode(ActionMenu::RENDERING_MODE_MENU) ->setRenderingMode(ActionMenu::RENDERING_MODE_MENU)
->condition($isActivated) ->condition($isActivated)
->addButton('deactivate_container_type', _('Deaktivieren'), Icon::create('remove'), [ ->addButton(
'deactivate_container_type',
_('Deaktivieren'),
Icon::create('remove'),
[
'formaction' => URLHelper::getURL( 'formaction' => URLHelper::getURL(
'dispatch.php/admin/courseware/deactivate_container_types', 'dispatch.php/admin/courseware/deactivate_container_types',
['container_types' => [$containerType]] ['container_types' => [$containerType]]
), ),
]) ]
)
->condition(!$isActivated) ->condition(!$isActivated)
->addButton('activate_container_type', _('Aktivieren'), Icon::create('add'), [ ->addButton(
'formaction' => URLHelper::getURL('dispatch.php/admin/courseware/activate_container_types', [ 'activate_container_type',
'container_types' => [$containerType], _('Aktivieren'),
]), Icon::create('add'),
]) ?> [
'formaction' => URLHelper::getURL(
'dispatch.php/admin/courseware/activate_container_types',
['container_types' => [$containerType]]
),
]
) ?>
</td> </td>
</tr> </tr>
<? } ?> <? endforeach; ?>
</tbody> </tbody>
</table> </table>
</form> </form>
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment