diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 99b37d73d..6d5a13003 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -31,9 +31,7 @@ jobs: uses: actions/cache/restore@v4 with: path: .cache - key: ci-${{ github.ref_name }}-${{ github.run_id }} - restore-keys: | - ci-${{ github.ref_name }}- + key: ci-${{ github.ref_name }} - name: "Restore Composer Cache" uses: actions/cache/restore@v4 @@ -62,7 +60,7 @@ jobs: if: always() with: path: .cache - key: ci-${{ github.ref_name }}-${{ github.run_id }} + key: ci-${{ github.ref_name }} - name: "Save vendor directory" if: steps.restore-composer.outputs.cache-hit != 'true' diff --git a/EMS/core-bundle/src/Controller/ContentManagement/CrudController.php b/EMS/core-bundle/src/Controller/ContentManagement/CrudController.php index 19bb4a497..dab7290e0 100644 --- a/EMS/core-bundle/src/Controller/ContentManagement/CrudController.php +++ b/EMS/core-bundle/src/Controller/ContentManagement/CrudController.php @@ -51,16 +51,16 @@ public function create(?string $ouuid, string $name, Request $request): Response $this->dataService->refresh($newRevision->giveContentType()->giveEnvironment()); } } catch (\Exception $e) { - if (($e instanceof NotFoundHttpException) or ($e instanceof BadRequestHttpException)) { + if ($e instanceof NotFoundHttpException || $e instanceof BadRequestHttpException) { throw $e; - } else { - $this->logger->error('log.crud.create_error', [ - EmsFields::LOG_CONTENTTYPE_FIELD => $contentType->getName(), - EmsFields::LOG_ERROR_MESSAGE_FIELD => $e->getMessage(), - EmsFields::LOG_EXCEPTION_FIELD => $e, - ]); } + $this->logger->error('log.crud.create_error', [ + EmsFields::LOG_CONTENTTYPE_FIELD => $contentType->getName(), + EmsFields::LOG_ERROR_MESSAGE_FIELD => $e->getMessage(), + EmsFields::LOG_EXCEPTION_FIELD => $e, + ]); + return $this->flashMessageLogger->buildJsonResponse([ 'success' => false, 'ouuid' => $ouuid, diff --git a/EMS/core-bundle/src/Controller/ContentManagement/ReleaseController.php b/EMS/core-bundle/src/Controller/ContentManagement/ReleaseController.php index 7576670df..cb65d7cb6 100644 --- a/EMS/core-bundle/src/Controller/ContentManagement/ReleaseController.php +++ b/EMS/core-bundle/src/Controller/ContentManagement/ReleaseController.php @@ -4,7 +4,7 @@ namespace EMS\CoreBundle\Controller\ContentManagement; -use EMS\CoreBundle\Controller\AbstractCoreController; +use EMS\CoreBundle\Controller\CoreControllerTrait; use EMS\CoreBundle\Core\DataTable\DataTableFactory; use EMS\CoreBundle\Core\Revision\Release\ReleaseRevisionType; use EMS\CoreBundle\DataTable\Type\Release\ReleaseOverviewDataTableType; @@ -20,11 +20,14 @@ use EMS\CoreBundle\Routes; use EMS\CoreBundle\Service\ReleaseService; use Psr\Log\LoggerInterface; +use Symfony\Bundle\FrameworkBundle\Controller\AbstractController; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; -final class ReleaseController extends AbstractCoreController +final class ReleaseController extends AbstractController { + use CoreControllerTrait; + public function __construct( private readonly LoggerInterface $logger, private readonly ReleaseService $releaseService, diff --git a/EMS/core-bundle/src/Controller/AbstractCoreController.php b/EMS/core-bundle/src/Controller/CoreControllerTrait.php similarity index 77% rename from EMS/core-bundle/src/Controller/AbstractCoreController.php rename to EMS/core-bundle/src/Controller/CoreControllerTrait.php index d9c9a9b86..abc8752cc 100644 --- a/EMS/core-bundle/src/Controller/AbstractCoreController.php +++ b/EMS/core-bundle/src/Controller/CoreControllerTrait.php @@ -4,11 +4,10 @@ namespace EMS\CoreBundle\Controller; -use Symfony\Bundle\FrameworkBundle\Controller\AbstractController; use Symfony\Component\Form\Form; use Symfony\Component\Form\FormInterface; -abstract class AbstractCoreController extends AbstractController +trait CoreControllerTrait { protected function getClickedButtonName(FormInterface $form): ?string { diff --git a/EMS/core-bundle/src/Controller/Revision/TrashController.php b/EMS/core-bundle/src/Controller/Revision/TrashController.php index 50db221dc..53e886aba 100644 --- a/EMS/core-bundle/src/Controller/Revision/TrashController.php +++ b/EMS/core-bundle/src/Controller/Revision/TrashController.php @@ -81,9 +81,13 @@ public function putBack(ContentType $contentType, string $ouuid): RedirectRespon throw $this->createAccessDeniedException(); } - $revisionId = $this->dataService->trashPutBack($contentType, $ouuid); + $restoredRevision = $this->dataService->trashPutBackAsDraft($contentType, $ouuid); - return $this->redirectToRoute(Routes::EDIT_REVISION, ['revisionId' => $revisionId]); + if (!$restoredRevision) { + throw new \RuntimeException(\sprintf('Put back failed for ouuid "%s"', $ouuid)); + } + + return $this->redirectToRoute(Routes::EDIT_REVISION, ['revisionId' => $restoredRevision->getId()]); } private function emptyTrashSelection(ContentType $contentType, string ...$ouuids): Response @@ -103,10 +107,10 @@ private function putBackSelection(ContentType $contentType, string ...$ouuids): throw $this->createAccessDeniedException(); } - $revisionId = $this->dataService->trashPutBack($contentType, ...$ouuids); + $restoredRevision = $this->dataService->trashPutBackAsDraft($contentType, ...$ouuids); - if ($revisionId) { - return $this->redirectToRoute(Routes::EDIT_REVISION, ['revisionId' => $revisionId]); + if ($restoredRevision) { + return $this->redirectToRoute(Routes::EDIT_REVISION, ['revisionId' => $restoredRevision->getId()]); } return $this->redirectToRoute(Routes::DRAFT_IN_PROGRESS, ['contentTypeId' => $contentType->getId()]); diff --git a/EMS/core-bundle/src/Exception/DuplicateOuuidException.php b/EMS/core-bundle/src/Exception/DuplicateOuuidException.php index 5e6eaeddb..319b4f51f 100644 --- a/EMS/core-bundle/src/Exception/DuplicateOuuidException.php +++ b/EMS/core-bundle/src/Exception/DuplicateOuuidException.php @@ -1,7 +1,15 @@ addModelTransformer(new DataFieldModelTransformer($fieldType, $this->formRegistry)); } } - - $builder->add('remove_collection_item', SubmitEmsType::class, [ - 'attr' => [ - 'class' => 'btn btn-danger btn-sm remove-content-button', - ], - 'label' => 'Remove', - 'icon' => 'fa fa-trash', - ]); } public function buildObjectArray(DataField $data, array &$out): void diff --git a/EMS/core-bundle/src/Repository/RevisionRepository.php b/EMS/core-bundle/src/Repository/RevisionRepository.php index 2e413147b..7d103bebb 100644 --- a/EMS/core-bundle/src/Repository/RevisionRepository.php +++ b/EMS/core-bundle/src/Repository/RevisionRepository.php @@ -349,12 +349,15 @@ public function countByContentType(ContentType $contentType): int public function countRevisions(string $ouuid, ContentType $contentType): int { - $qb = $this->createQueryBuilder('r') - ->select('COUNT(r)'); - $qb->where($qb->expr()->eq('r.ouuid', ':ouuid')); - $qb->andWhere($qb->expr()->eq('r.contentType', ':contentType')); - $qb->setParameter('ouuid', $ouuid); - $qb->setParameter('contentType', $contentType); + $qb = $this->createQueryBuilder('r'); + $qb + ->select('COUNT(r.id)') + ->andWhere($qb->expr()->eq('r.ouuid', ':ouuid')) + ->andWhere($qb->expr()->eq('r.contentType', ':contentType')) + ->setParameters([ + 'ouuid' => $ouuid, + 'contentType' => $contentType, + ]); return (int) $qb->getQuery()->getSingleScalarResult(); } @@ -475,20 +478,19 @@ public function finaliseRevision(ContentType $contentType, string $ouuid, \DateT public function getCurrentRevision(ContentType $contentType, string $ouuid): ?Revision { - $qb = $this->createQueryBuilder('r')->select() - ->where('r.contentType = ?2') - ->andWhere('r.ouuid = ?3') - ->andWhere('r.endTime is null') - ->setParameter(2, $contentType) - ->setParameter(3, $ouuid); + $qb = $this->createQueryBuilder('r'); + $qb + ->andWhere($qb->expr()->eq('r.ouuid', ':ouuid')) + ->andWhere($qb->expr()->eq('r.contentType', ':contentType')) + ->andWhere($qb->expr()->isNull('r.endTime')) + ->setParameters([ + 'ouuid' => $ouuid, + 'contentType' => $contentType, + ]); - /** @var Revision[] $currentRevision */ - $currentRevision = $qb->getQuery()->execute(); - if (isset($currentRevision[0])) { - return $currentRevision[0]; - } else { - return null; - } + $revision = $qb->getQuery()->getOneOrNullResult(); + + return $revision instanceof Revision ? $revision : null; } public function publishRevision(Revision $revision, bool $draft = false): int @@ -833,6 +835,7 @@ public function findTrashRevisions(ContentType $contentType, string ...$ouuids): ->andWhere($qb->expr()->eq('c.id', ':content_type_id')) ->andWhere($qb->expr()->in('r.ouuid', ':ouuids')) ->andWhere($qb->expr()->eq('r.deleted', $qb->expr()->literal(true))) + ->orderBy('r.startTime', 'DESC') ->setParameter('content_type_id', $contentType->getId()) ->setParameter('ouuids', $ouuids, ArrayParameterType::STRING) ; diff --git a/EMS/core-bundle/src/Resources/views/form/fields.html.twig b/EMS/core-bundle/src/Resources/views/form/fields.html.twig index 6aae39bff..6df093a00 100644 --- a/EMS/core-bundle/src/Resources/views/form/fields.html.twig +++ b/EMS/core-bundle/src/Resources/views/form/fields.html.twig @@ -616,7 +616,9 @@
{{ form_label(form) }}
- {{- form_widget(form.remove_collection_item) -}} + {% if collapsible %}
@@ -631,7 +633,7 @@
- {% for child in form.iterator|filter(c => c.vars.name not in ['remove_collection_item', '_ems_internal_deleted']) %} + {% for child in form.iterator|filter(c => c.vars.name not in ['_ems_internal_deleted']) %}
{{- form_row(child) -}}
diff --git a/EMS/core-bundle/src/Service/DataService.php b/EMS/core-bundle/src/Service/DataService.php index 2b9edb518..de6e3019f 100644 --- a/EMS/core-bundle/src/Service/DataService.php +++ b/EMS/core-bundle/src/Service/DataService.php @@ -49,7 +49,6 @@ use Symfony\Component\Form\FormInterface; use Symfony\Component\Form\FormRegistryInterface; use Symfony\Component\HttpKernel\Exception\BadRequestHttpException; -use Symfony\Component\HttpKernel\Exception\ConflictHttpException; use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface; use Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface; @@ -405,7 +404,7 @@ public function convertInputValues(DataField $dataField): void public static function isInternalField(string $fieldName): bool { - return \in_array($fieldName, ['_ems_internal_deleted', 'remove_collection_item']); + return '_ems_internal_deleted' === $fieldName; } public function generateInputValues(DataField $dataField): void @@ -443,36 +442,14 @@ public function createData(?string $ouuid, array $rawdata, ContentType $contentT $newRevision->setEndTime(null); $newRevision->setDeleted(false); $newRevision->setDraft(true); - if ($byARealUser) { - $token = $this->tokenStorage->getToken(); - if (null === $token) { - throw new \RuntimeException('Unexpected null token'); - } - $newRevision->setLockBy($token->getUserIdentifier()); - } else { - $newRevision->setLockBy('DATA_SERVICE'); - } - $newRevision->setLockUntil($until); - $newRevision->setRawData($rawdata); - $em = $this->doctrine->getManager(); - if (!empty($ouuid)) { - $revisionRepository = $em->getRepository(Revision::class); - $anotherObject = $revisionRepository->findOneBy([ - 'contentType' => $contentType, - 'ouuid' => $newRevision->getOuuid(), - 'endTime' => null, - ]); + $lockBy = $byARealUser ? $this->userService->getCurrentUser()->getUserIdentifier() : 'DATA_SERVICE'; + $newRevision->setLockBy($lockBy); - if (!empty($anotherObject)) { - throw new ConflictHttpException('Duplicate OUUID '.$ouuid.' for this content type'); - } - } - - $em->persist($newRevision); - $em->flush(); + $newRevision->setLockUntil($until); + $newRevision->setRawData($rawdata); - return $newRevision; + return $this->createNewRevision($newRevision); } /** @@ -933,15 +910,9 @@ public function getNewestRevision(string $type, string $ouuid): Revision public function newDocument(ContentType $contentType, ?string $ouuid = null, ?array $rawData = null, ?string $username = null): Revision { $this->hasCreateRights($contentType); - /** @var RevisionRepository $revisionRepository */ - $revisionRepository = $this->em->getRepository(Revision::class); $revision = new Revision(); - if (null !== $ouuid && $revisionRepository->countRevisions($ouuid, $contentType)) { - throw new DuplicateOuuidException(); - } - if (!empty($contentType->getDefaultValue())) { try { $template = $this->twig->createTemplate($contentType->getDefaultValue()); @@ -1025,10 +996,7 @@ public function newDocument(ContentType $contentType, ?string $ouuid = null, ?ar } $this->setMetaFields($revision); - $this->em->persist($revision); - $this->em->flush(); - - return $revision; + return $this->createNewRevision($revision); } /** @@ -1278,25 +1246,23 @@ public function trashEmpty(ContentType $contentType, string ...$ouuids): void $this->em->flush(); } - public function trashPutBack(ContentType $contentType, string ...$ouuids): ?int + public function trashPutBackAsDraft(ContentType $contentType, string ...$ouuids): ?Revision { - $revisionIds = []; $revisions = $this->revRepository->findTrashRevisions($contentType, ...$ouuids); foreach ($revisions as $revision) { - $this->lockRevision($revision); $revision->setDeleted(false); $revision->setDeletedBy(null); if (null === $revision->getEndTime()) { + $this->lockRevision($revision); $revision->setDraft(true); - $revisionIds[] = $revision->getId(); $this->auditLogger->notice('log.revision.restored', LogRevisionContext::update($revision)); } $this->em->persist($revision); } $this->em->flush(); - return 1 === \count($ouuids) ? \array_shift($revisionIds) : null; + return 1 === \count($ouuids) ? \array_shift($revisions) : null; } /** @@ -1986,4 +1952,36 @@ public function getAllDrafts(): array { return $this->revRepository->findAllDrafts(); } + + private function createNewRevision(Revision $revision): Revision + { + $ouuid = $revision->getOuuid(); + $contentType = $revision->giveContentType(); + + $currentRevision = $ouuid ? $this->revRepository->getCurrentRevision($contentType, $ouuid) : null; + if ($ouuid && $currentRevision && !$currentRevision->getDeleted()) { + throw new DuplicateOuuidException($ouuid, $contentType->getName()); + } + + $this->em->getConnection()->beginTransaction(); + + try { + $restoredDraft = $currentRevision ? $this->trashPutBackAsDraft($contentType, $currentRevision->giveOuuid()) : null; + + if ($restoredDraft) { + $restoredDraft->setDraft(false); + $restoredDraft->setEndTime($revision->getStartTime()); + $this->unlockRevision($restoredDraft); + } + + $this->em->persist($revision); + $this->em->flush(); + $this->em->getConnection()->commit(); + } catch (\Throwable $e) { + $this->em->getConnection()->rollBack(); + throw $e; + } + + return $revision; + } }