Skip to content

Commit

Permalink
minor #4024 Replaced some magic strings by constants (javiereguiluz)
Browse files Browse the repository at this point in the history
This PR was merged into the 3.0.x-dev branch.

Discussion
----------

Replaced some magic strings by constants

We use DTOs, objects and constants for all the config stuff ... but the arguments of some functions and the keys of some arrays were still using "magic strings". Let's fix that.

In PHP 8.1 we'll probably have Enums, but for now, let's use very simple classes with public constants.

Please note that I don't want to update the docs about this. Let's keep using "magic strings" in docs for now, specially because using constants in Twig is super-verbose, so I don't want readers to think that they should use these long and boring constants in their templates.

#SymfonyHackday

Commits
-------

0333d87 Replaced some magic strings by constants
  • Loading branch information
javiereguiluz committed Dec 7, 2020
2 parents 06e6ab2 + 0333d87 commit 4bd8e11
Show file tree
Hide file tree
Showing 27 changed files with 179 additions and 79 deletions.
5 changes: 3 additions & 2 deletions src/Config/Crud.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

namespace EasyCorp\Bundle\EasyAdminBundle\Config;

use EasyCorp\Bundle\EasyAdminBundle\Config\Option\SortOrder;
use EasyCorp\Bundle\EasyAdminBundle\Dto\CrudDto;
use EasyCorp\Bundle\EasyAdminBundle\Dto\PaginatorDto;
use EasyCorp\Bundle\EasyAdminBundle\Field\DateTimeField;
Expand Down Expand Up @@ -192,8 +193,8 @@ public function setDefaultSort(array $sortFieldsAndOrder): self
{
$sortFieldsAndOrder = array_map('strtoupper', $sortFieldsAndOrder);
foreach ($sortFieldsAndOrder as $sortField => $sortOrder) {
if (!\in_array($sortOrder, ['ASC', 'DESC'])) {
throw new \InvalidArgumentException(sprintf('The sort order can be only "ASC" or "DESC", "%s" given.', $sortOrder));
if (!\in_array($sortOrder, [SortOrder::ASC, SortOrder::DESC], true)) {
throw new \InvalidArgumentException(sprintf('The sort order can be only "%s" or "%s", "%s" given.', SortOrder::ASC, SortOrder::DESC, $sortOrder));
}

if (!\is_string($sortField)) {
Expand Down
5 changes: 3 additions & 2 deletions src/Config/Dashboard.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

namespace EasyCorp\Bundle\EasyAdminBundle\Config;

use EasyCorp\Bundle\EasyAdminBundle\Config\Option\TextDirection;
use EasyCorp\Bundle\EasyAdminBundle\Dto\DashboardDto;

/**
Expand Down Expand Up @@ -47,8 +48,8 @@ public function setTranslationDomain(string $translationDomain): self

public function setTextDirection(string $direction): self
{
if (!\in_array($direction, ['ltr', 'rtl'], true)) {
throw new \InvalidArgumentException(sprintf('The "%s" value given to the textDirection option is not valid. It can only be "ltr" or "rtl"', $direction));
if (!\in_array($direction, [TextDirection::LTR, TextDirection::RTL], true)) {
throw new \InvalidArgumentException(sprintf('The "%s" value given to the textDirection option is not valid. It can only be "%s" or "%s"', $direction, TextDirection::LTR, TextDirection::RTL));
}

$this->dto->setTextDirection($direction);
Expand Down
20 changes: 11 additions & 9 deletions src/Config/Menu/CrudMenuItem.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

namespace EasyCorp\Bundle\EasyAdminBundle\Config\Menu;

use EasyCorp\Bundle\EasyAdminBundle\Config\Option\EA;
use EasyCorp\Bundle\EasyAdminBundle\Config\Option\SortOrder;
use EasyCorp\Bundle\EasyAdminBundle\Contracts\Menu\MenuItemInterface;
use EasyCorp\Bundle\EasyAdminBundle\Dto\MenuItemDto;

Expand All @@ -22,18 +24,18 @@ public function __construct(string $label, ?string $icon, string $entityFqcn)
$this->dto->setLabel($label);
$this->dto->setIcon($icon);
$this->dto->setRouteParameters([
'crudAction' => 'index',
'crudId' => null,
'entityFqcn' => $entityFqcn,
'entityId' => null,
EA::CRUD_ACTION => 'index',
EA::CRUD_ID => null,
EA::ENTITY_FQCN => $entityFqcn,
EA::ENTITY_ID => null,
]);
}

public function setController(string $controllerFqcn): self
{
$this->dto->setRouteParameters(array_merge(
$this->dto->getRouteParameters(),
['crudControllerFqcn' => $controllerFqcn]
[EA::CRUD_CONTROLLER_FQCN => $controllerFqcn]
));

return $this;
Expand All @@ -43,7 +45,7 @@ public function setAction(string $actionName): self
{
$this->dto->setRouteParameters(array_merge(
$this->dto->getRouteParameters(),
['crudAction' => $actionName]
[EA::CRUD_ACTION => $actionName]
));

return $this;
Expand All @@ -53,7 +55,7 @@ public function setEntityId($entityId): self
{
$this->dto->setRouteParameters(array_merge(
$this->dto->getRouteParameters(),
['entityId' => $entityId]
[EA::ENTITY_ID => $entityId]
));

return $this;
Expand All @@ -66,7 +68,7 @@ public function setDefaultSort(array $sortFieldsAndOrder): self
{
$sortFieldsAndOrder = array_map('strtoupper', $sortFieldsAndOrder);
foreach ($sortFieldsAndOrder as $sortField => $sortOrder) {
if (!\in_array($sortOrder, ['ASC', 'DESC'])) {
if (!\in_array($sortOrder, [SortOrder::ASC, SortOrder::DESC])) {
throw new \InvalidArgumentException(sprintf('The sort order can be only "ASC" or "DESC", "%s" given.', $sortOrder));
}

Expand All @@ -77,7 +79,7 @@ public function setDefaultSort(array $sortFieldsAndOrder): self

$this->dto->setRouteParameters(array_merge(
$this->dto->getRouteParameters(),
['sort' => $sortFieldsAndOrder]
[EA::SORT => $sortFieldsAndOrder]
));

return $this;
Expand Down
23 changes: 23 additions & 0 deletions src/Config/Option/EA.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<?php

namespace EasyCorp\Bundle\EasyAdminBundle\Config\Option;

/**
* @author Javier Eguiluz <javier.eguiluz@gmail.com>
*/
final class EA
{
public const CONTEXT_NAME = 'eaContext';
public const MENU_INDEX = 'menuIndex';
public const SUBMENU_INDEX = 'submenuIndex';
public const QUERY = 'query';
public const FILTERS = 'filters';
public const SORT = 'sort';
public const REFERRER = 'referrer';
public const DASHBOARD_CONTROLLER_FQCN = 'dashboardControllerFqcn';
public const CRUD_CONTROLLER_FQCN = 'crudControllerFqcn';
public const CRUD_ACTION = 'crudAction';
public const CRUD_ID = 'crudId';
public const ENTITY_FQCN = 'entityFqcn';
public const ENTITY_ID = 'entityId';
}
14 changes: 14 additions & 0 deletions src/Config/Option/Size.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
<?php

namespace EasyCorp\Bundle\EasyAdminBundle\Config\Option;

/**
* @author Javier Eguiluz <javier.eguiluz@gmail.com>
*/
final class Size
{
public const SM = 'sm';
public const MD = 'md';
public const LG = 'lg';
public const XL = 'xl';
}
12 changes: 12 additions & 0 deletions src/Config/Option/SortOrder.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<?php

namespace EasyCorp\Bundle\EasyAdminBundle\Config\Option;

/**
* @author Javier Eguiluz <javier.eguiluz@gmail.com>
*/
final class SortOrder
{
public const ASC = 'ASC';

This comment has been minimized.

Copy link
@littlefuntik

littlefuntik Jan 19, 2021

I now about PHP constants SORT_ASC and SORT_DESC. Why not use them?

https://www.php.net/manual/en/array.constants.php

public const DESC = 'DESC';
}
13 changes: 13 additions & 0 deletions src/Config/Option/TextAlign.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<?php

namespace EasyCorp\Bundle\EasyAdminBundle\Config\Option;

/**
* @author Javier Eguiluz <javier.eguiluz@gmail.com>
*/
final class TextAlign
{
public const LEFT = 'left';
public const CENTER = 'center';
public const RIGHT = 'right';
}
12 changes: 12 additions & 0 deletions src/Config/Option/TextDirection.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<?php

namespace EasyCorp\Bundle\EasyAdminBundle\Config\Option;

/**
* @author Javier Eguiluz <javier.eguiluz@gmail.com>
*/
final class TextDirection
{
public const LTR = 'ltr';
public const RTL = 'rtl';
}
7 changes: 4 additions & 3 deletions src/Context/AdminContext.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

namespace EasyCorp\Bundle\EasyAdminBundle\Context;

use EasyCorp\Bundle\EasyAdminBundle\Config\Option\EA;
use EasyCorp\Bundle\EasyAdminBundle\Config\UserMenu;
use EasyCorp\Bundle\EasyAdminBundle\Contracts\Controller\DashboardControllerInterface;
use EasyCorp\Bundle\EasyAdminBundle\Dto\AssetsDto;
Expand Down Expand Up @@ -63,7 +64,7 @@ public function getRequest(): Request

public function getReferrer(): ?string
{
return $this->request->query->get('referrer');
return $this->request->query->get(EA::REFERRER);
}

public function getI18n(): I18nDto
Expand Down Expand Up @@ -129,8 +130,8 @@ public function getMainMenu(): MainMenuDto

$configuredMenuItems = $this->dashboardControllerInstance->configureMenuItems();
$mainMenuItems = \is_array($configuredMenuItems) ? $configuredMenuItems : iterator_to_array($configuredMenuItems, false);
$selectedMenuIndex = $this->request->query->getInt('menuIndex', -1);
$selectedMenuSubIndex = $this->request->query->getInt('submenuIndex', -1);
$selectedMenuIndex = $this->request->query->getInt(EA::MENU_INDEX, -1);
$selectedMenuSubIndex = $this->request->query->getInt(EA::SUBMENU_INDEX, -1);

return $this->mainMenuDto = $this->menuFactory->createMainMenu($mainMenuItems, $selectedMenuIndex, $selectedMenuSubIndex);
}
Expand Down
7 changes: 4 additions & 3 deletions src/Controller/AbstractCrudController.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
use EasyCorp\Bundle\EasyAdminBundle\Config\Crud;
use EasyCorp\Bundle\EasyAdminBundle\Config\Filters;
use EasyCorp\Bundle\EasyAdminBundle\Config\KeyValueStore;
use EasyCorp\Bundle\EasyAdminBundle\Config\Option\EA;
use EasyCorp\Bundle\EasyAdminBundle\Context\AdminContext;
use EasyCorp\Bundle\EasyAdminBundle\Contracts\Controller\CrudControllerInterface;
use EasyCorp\Bundle\EasyAdminBundle\Dto\EntityDto;
Expand Down Expand Up @@ -392,7 +393,7 @@ public function delete(AdminContext $context)
return $this->redirect($referrer);
}

return $this->redirect($this->get(CrudUrlGenerator::class)->build()->setAction('index')->unset('entityId')->generateUrl());
return $this->redirect($this->get(CrudUrlGenerator::class)->build()->setAction(Action::INDEX)->unset(EA::ENTITY_ID)->generateUrl());
}

public function autocomplete(AdminContext $context): JsonResponse
Expand All @@ -402,7 +403,7 @@ public function autocomplete(AdminContext $context): JsonResponse
$autocompleteContext = $context->getRequest()->get(AssociationField::PARAM_AUTOCOMPLETE_CONTEXT);

/** @var CrudControllerInterface $controller */
$controller = $this->get(ControllerFactory::class)->getCrudControllerInstance($autocompleteContext['crudId'], Action::INDEX, $context->getRequest());
$controller = $this->get(ControllerFactory::class)->getCrudControllerInstance($autocompleteContext[EA::CRUD_ID], Action::INDEX, $context->getRequest());
/** @var FieldDto $field */
$field = FieldCollection::new($controller->configureFields($autocompleteContext['originatingPage']))->get($autocompleteContext['propertyName']);
/** @var \Closure|null $queryBuilderCallable */
Expand Down Expand Up @@ -431,7 +432,7 @@ public function renderFilters(AdminContext $context): KeyValueStore
/** @var FiltersFormType $filtersForm */
$filtersForm = $this->get(FormFactory::class)->createFiltersForm($filters, $context->getRequest());
$formActionParts = parse_url($filtersForm->getConfig()->getAction());
$queryString = $formActionParts['query'] ?? [];
$queryString = $formActionParts[EA::QUERY] ?? [];
parse_str($queryString, $queryStringAsArray);

$responseParameters = KeyValueStore::new([
Expand Down
9 changes: 5 additions & 4 deletions src/EventListener/AdminContextListener.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

namespace EasyCorp\Bundle\EasyAdminBundle\EventListener;

use EasyCorp\Bundle\EasyAdminBundle\Config\Option\EA;
use EasyCorp\Bundle\EasyAdminBundle\Context\AdminContext;
use EasyCorp\Bundle\EasyAdminBundle\Contracts\Controller\CrudControllerInterface;
use EasyCorp\Bundle\EasyAdminBundle\Contracts\Controller\DashboardControllerInterface;
Expand Down Expand Up @@ -34,7 +35,7 @@ public function __construct(AdminContextFactory $adminContextFactory, Controller

public function onKernelController(ControllerEvent $event): void
{
$contextId = $event->getRequest()->query->get('eaContext');
$contextId = $event->getRequest()->query->get(EA::CONTEXT_NAME);
$currentControllerInstance = $this->getCurrentControllerInstance($event);
if (!$this->isEasyAdminRequest($contextId, $currentControllerInstance)) {
return;
Expand All @@ -50,8 +51,8 @@ public function onKernelController(ControllerEvent $event): void
return;
}

$crudId = $event->getRequest()->query->get('crudId');
$crudAction = $event->getRequest()->query->get('crudAction');
$crudId = $event->getRequest()->query->get(EA::CRUD_ID);
$crudAction = $event->getRequest()->query->get(EA::CRUD_ACTION);
$crudControllerInstance = $this->controllerFactory->getCrudControllerInstance($crudId, $crudAction, $event->getRequest());

if (null !== $crudId && null === $dashboardControllerInstance) {
Expand Down Expand Up @@ -89,7 +90,7 @@ public function onKernelController(ControllerEvent $event): void
* Request is associated to EasyAdmin if one of these conditions meet:
* * current controller is an instance of DashboardControllerInterface (the single point of
* entry for all requests directly served by EasyAdmin)
* * the contextId passed via the 'eaContext' query string parameter is not null
* * the contextId passed via the EA::CONTEXT_NAME query string parameter is not null
* (that's used in menu items that link to Symfony routes not served by EasyAdmin, so
* those routes can still be associated with some EasyAdmin dashboard to display the menu, etc.).
*/
Expand Down
19 changes: 10 additions & 9 deletions src/Factory/ActionFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
use EasyCorp\Bundle\EasyAdminBundle\Collection\ActionCollection;
use EasyCorp\Bundle\EasyAdminBundle\Config\Action;
use EasyCorp\Bundle\EasyAdminBundle\Config\Crud;
use EasyCorp\Bundle\EasyAdminBundle\Config\Option\EA;
use EasyCorp\Bundle\EasyAdminBundle\Dto\ActionConfigDto;
use EasyCorp\Bundle\EasyAdminBundle\Dto\ActionDto;
use EasyCorp\Bundle\EasyAdminBundle\Dto\EntityDto;
Expand Down Expand Up @@ -145,21 +146,21 @@ private function generateActionUrl(string $adminContextId, string $currentAction
$routeParameters = $routeParameters($entityInstance);
}

$routeParameters = array_merge(['eaContext' => $adminContextId], $routeParameters);
$routeParameters = array_merge([EA::CONTEXT_NAME => $adminContextId], $routeParameters);

return $this->urlGenerator->generate($routeName, $routeParameters);
}

$requestParameters = [
'crudId' => $request->query->get('crudId'),
'crudAction' => $actionDto->getCrudActionName(),
'referrer' => $this->generateReferrerUrl($request, $actionDto, $currentAction),
EA::CRUD_ID => $request->query->get(EA::CRUD_ID),
EA::CRUD_ACTION => $actionDto->getCrudActionName(),
EA::REFERRER => $this->generateReferrerUrl($request, $actionDto, $currentAction),
];

if (\in_array($actionDto->getName(), [Action::INDEX, Action::NEW], true)) {
$requestParameters['entityId'] = null;
$requestParameters[EA::ENTITY_ID] = null;
} elseif (null !== $entityDto) {
$requestParameters['entityId'] = $entityDto->getPrimaryKeyValueAsString();
$requestParameters[EA::ENTITY_ID] = $entityDto->getPrimaryKeyValueAsString();
}

return $this->crudUrlGenerator->build($requestParameters)->generateUrl();
Expand All @@ -183,10 +184,10 @@ private function generateReferrerUrl(Request $request, ActionDto $actionDto, str
return null;
}

$referrer = $request->get('referrer');
$referrer = $request->get(EA::REFERRER);
$referrerParts = parse_url($referrer);
parse_str($referrerParts['query'] ?? '', $referrerQueryStringVariables);
$referrerCrudAction = $referrerQueryStringVariables['crudAction'] ?? null;
parse_str($referrerParts[EA::QUERY] ?? '', $referrerQueryStringVariables);
$referrerCrudAction = $referrerQueryStringVariables[EA::CRUD_ACTION] ?? null;

if (Action::EDIT === $currentAction) {
if (\in_array($referrerCrudAction, [Action::INDEX, Action::DETAIL], true)) {
Expand Down
16 changes: 9 additions & 7 deletions src/Factory/AdminContextFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

use EasyCorp\Bundle\EasyAdminBundle\Cache\CacheWarmer;
use EasyCorp\Bundle\EasyAdminBundle\Config\Crud;
use EasyCorp\Bundle\EasyAdminBundle\Config\Option\EA;
use EasyCorp\Bundle\EasyAdminBundle\Config\Option\TextDirection;
use EasyCorp\Bundle\EasyAdminBundle\Context\AdminContext;
use EasyCorp\Bundle\EasyAdminBundle\Contracts\Controller\CrudControllerInterface;
use EasyCorp\Bundle\EasyAdminBundle\Contracts\Controller\DashboardControllerInterface;
Expand Down Expand Up @@ -47,7 +49,7 @@ public function __construct(string $cacheDir, TranslatorInterface $translator, ?

public function create(Request $request, DashboardControllerInterface $dashboardController, ?CrudControllerInterface $crudController): AdminContext
{
$crudAction = $request->query->get('crudAction');
$crudAction = $request->query->get(EA::CRUD_ACTION);
$validPageNames = [Crud::PAGE_INDEX, Crud::PAGE_DETAIL, Crud::PAGE_EDIT, Crud::PAGE_NEW];
$pageName = \in_array($crudAction, $validPageNames, true) ? $crudAction : null;

Expand Down Expand Up @@ -166,15 +168,15 @@ private function getI18nDto(Request $request, DashboardDto $dashboardDto, ?CrudD

$configuredTextDirection = $dashboardDto->getTextDirection();
$localePrefix = strtolower(substr($locale, 0, 2));
$defaultTextDirection = \in_array($localePrefix, ['ar', 'fa', 'he']) ? 'rtl' : 'ltr';
$defaultTextDirection = \in_array($localePrefix, ['ar', 'fa', 'he']) ? TextDirection::RTL : TextDirection::LTR;
$textDirection = $configuredTextDirection ?? $defaultTextDirection;

$translationDomain = $dashboardDto->getTranslationDomain();

$translationParameters = [];
if (null !== $crudDto) {
$translationParameters['%entity_name%'] = $entityName = basename(str_replace('\\', '/', $crudDto->getEntityFqcn()));
$translationParameters['%entity_id%'] = $entityId = $request->query->get('entityId');
$translationParameters['%entity_id%'] = $entityId = $request->query->get(EA::ENTITY_ID);
$translationParameters['%entity_short_id%'] = null === $entityId ? null : u((string) $entityId)->truncate(7);

$entityInstance = null === $entityDto ? null : $entityDto->getInstance();
Expand All @@ -198,10 +200,10 @@ public function getSearchDto(Request $request, ?CrudDto $crudDto): ?SearchDto

$queryParams = $request->query->all();
$searchableProperties = $crudDto->getSearchFields();
$query = $queryParams['query'] ?? null;
$query = $queryParams[EA::QUERY] ?? null;
$defaultSort = $crudDto->getDefaultSort();
$customSort = $queryParams['sort'] ?? [];
$appliedFilters = $queryParams['filters'] ?? [];
$customSort = $queryParams[EA::SORT] ?? [];
$appliedFilters = $queryParams[EA::FILTERS] ?? [];

return new SearchDto($request, $searchableProperties, $query, $defaultSort, $customSort, $appliedFilters);
}
Expand All @@ -225,6 +227,6 @@ private function getEntityDto(Request $request, ?CrudDto $crudDto): ?EntityDto
return null;
}

return $this->entityFactory->create($crudDto->getEntityFqcn(), $request->query->get('entityId'), $crudDto->getEntityPermission());
return $this->entityFactory->create($crudDto->getEntityFqcn(), $request->query->get(EA::ENTITY_ID), $crudDto->getEntityPermission());
}
}

0 comments on commit 4bd8e11

Please sign in to comment.