Merge branch 'issue115_social_action_versioning' into 'master'

Add versioning and optimistic locking on accompanying period work

Closes #115

See merge request Chill-Projet/chill-bundles!627

.changes/unreleased/Feature-20240208-123318.yaml

@@ -0,0 +1,6 @@
+kind: Feature
+body: Prevent social work to be saved when another user edited conccurently the social
+  work
+time: 2024-02-08T12:33:18.319442398+01:00
+custom:
+  Issue: "115"

src/Bundle/ChillMainBundle/CRUD/Controller/AbstractCRUDController.php

@@ -15,10 +15,14 @@ use Chill\MainBundle\CRUD\Resolver\Resolver;
 use Chill\MainBundle\Pagination\PaginatorFactory;
 use Chill\MainBundle\Pagination\PaginatorInterface;
 use Chill\MainBundle\Security\Authorization\AuthorizationHelper;
+use Doctrine\DBAL\LockMode;
+use Doctrine\ORM\EntityManagerInterface;
+use Doctrine\ORM\OptimisticLockException;
 use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
 use Symfony\Component\EventDispatcher\EventDispatcherInterface;
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpFoundation\Response;
+use Symfony\Component\HttpKernel\Exception\ConflictHttpException;
 use Symfony\Component\Serializer\SerializerInterface;
 use Symfony\Component\Validator\Validator\ValidatorInterface;
 use Symfony\Contracts\Translation\TranslatorInterface;
@@ -173,6 +177,21 @@ abstract class AbstractCRUDController extends AbstractController
         if (null === $e) {
             throw $this->createNotFoundException(sprintf('The object %s for id %s is not found', $this->getEntityClass(), $id));
         }
+        if ($request->query->has('entity_version')) {
+            $expectedVersion = $request->query->getInt('entity_version');
+
+            try {
+                $manager = $this->getDoctrine()->getManagerForClass($this->getEntityClass());
+
+                if ($manager instanceof EntityManagerInterface) {
+                    $manager->lock($e, LockMode::OPTIMISTIC, $expectedVersion);
+                } else {
+                    throw new \LogicException('This manager does not allow locking.');
+                }
+            } catch (OptimisticLockException $e) {
+                throw new ConflictHttpException('Sorry, but someone else has already changed this entity. Please refresh the page and apply the changes again', $e);
+            }
+        }
 
         return $e;
     }

src/Bundle/ChillMainBundle/CRUD/Controller/ApiController.php

@@ -135,7 +135,7 @@ class ApiController extends AbstractCRUDController
         try {
             $entity = $this->deserialize($action, $request, $_format, $entity);
         } catch (NotEncodableValueException $e) {
-            throw new BadRequestHttpException('invalid json', 400, $e);
+            throw new BadRequestHttpException('invalid json', $e, 400);
         }
 
         $errors = $this->validate($action, $request, $_format, $entity);
@@ -153,7 +153,7 @@ class ApiController extends AbstractCRUDController
             return $response;
         }
 
-        $this->getDoctrine()->getManager()->flush();
+        $this->getDoctrine()->getManagerForClass($this->getEntityClass())->flush();
 
         $response = $this->onAfterFlush($action, $request, $_format, $entity, $errors);
 

src/Bundle/ChillMainBundle/Resources/public/lib/api/apiMethods.ts

@@ -55,11 +55,20 @@ export interface ServerExceptionInterface extends TransportExceptionInterface {
   body: string;
 }
 
+export interface ConflictHttpExceptionInterface extends TransportExceptionInterface {
+    name: 'ConflictHttpException';
+    violations: string[];
+}
 
 /**
  * Generic api method that can be adapted to any fetch request
  */
-export const makeFetch = <Input, Output>(method: 'POST'|'GET'|'PUT'|'PATCH'|'DELETE', url: string, body?: body | Input | null, options?: FetchParams): Promise<Output> => {
+export const makeFetch = <Input, Output>(
+    method: 'POST'|'GET'|'PUT'|'PATCH'|'DELETE',
+    url: string, body?: body | Input | null,
+    options?: FetchParams
+): Promise<Output> => {
+
     let opts = {
         method: method,
         headers: {
@@ -67,6 +76,7 @@ export const makeFetch = <Input, Output>(method: 'POST'|'GET'|'PUT'|'PATCH'|'DEL
         },
     };
 
+
   if (body !== null && typeof body !== 'undefined') {
       Object.assign(opts, {body: JSON.stringify(body)})
     }
@@ -90,6 +100,10 @@ export const makeFetch = <Input, Output>(method: 'POST'|'GET'|'PUT'|'PATCH'|'DEL
             throw AccessException(response);
         }
 
+        if (response.status === 409) {
+            throw ConflictHttpException(response);
+        }
+
         throw {
             name: 'Exception',
             sta: response.status,
@@ -220,3 +234,12 @@ const ServerException = (code: number, body: string): ServerExceptionInterface =
 
   return error;
 }
+
+const ConflictHttpException = (response: Response): ConflictHttpExceptionInterface => {
+    const error = {} as ConflictHttpExceptionInterface;
+
+    error.name = 'ConflictHttpException';
+    error.violations = ['Sorry, but someone else has already changed this entity. Please refresh the page and apply the changes again']
+
+    return error;
+}

src/Bundle/ChillPersonBundle/Entity/AccompanyingPeriod/AccompanyingPeriodWork.php

@@ -244,6 +244,15 @@ class AccompanyingPeriodWork implements AccompanyingPeriodLinkedWithSocialIssues
      */
     private ?User $updatedBy = null;
 
+    /**
+     * @ORM\Column(type="integer", nullable=false, options={"default": 1})
+     *
+     * @Serializer\Groups({"read", "accompanying_period_work:edit"})
+     *
+     * @ORM\Version
+     */
+    private int $version = 1;
+
     public function __construct()
     {
         $this->goals = new ArrayCollection();
@@ -452,6 +461,18 @@ class AccompanyingPeriodWork implements AccompanyingPeriodLinkedWithSocialIssues
         return $this->updatedBy;
     }
 
+    public function getVersion(): int
+    {
+        return $this->version;
+    }
+
+    public function setVersion(int $version): self
+    {
+        $this->version = $version;
+
+        return $this;
+    }
+
     public function removeAccompanyingPeriodWorkEvaluation(AccompanyingPeriodWorkEvaluation $evaluation): self
     {
         $this->accompanyingPeriodWorkEvaluations

src/Bundle/ChillPersonBundle/Resources/public/vuejs/AccompanyingCourseWorkEdit/App.vue

@@ -297,7 +297,7 @@
             @go-to-generate-workflow="goToGenerateWorkflow"
          ></list-workflow-modal>
       </li>
-      
+
       <li>
          <button v-if="AmIRefferer"
              class="btn btn-notify"
@@ -311,7 +311,7 @@
             </ul>
          </template>
       </li>
-      
+
       <li v-if="!isPosting">
          <button class="btn btn-save" @click="submit">
             {{ $t('action.save') }}
@@ -348,6 +348,10 @@ import { makeFetch } from 'ChillMainAssets/lib/api/apiMethods';
 const i18n = {
    messages: {
       fr: {
+         action: {
+           save: "Enregistrer"
+         },
+         conflict_on_save: "Désolé, cette action d'accompagnement a été modifiée dans une autre fenêtre ou par un autre utilisateur. Rechargez la page pour voir ses changements.",
          action_title: "Action d'accompagnement",
          comments: "Commentaire",
          startDate: "Date de début",
@@ -378,6 +382,7 @@ const i18n = {
          no_evaluations_available: "Aucune évaluation disponible",
          no_goals_available: "Aucun objectif disponible",
          referrers: "Agents traitants",
+         add_referrers: "Ajouter des agents traitants",
          no_referrers: "Aucun agent traitant",
          choose_referrers: "Choisir des agents traitants",
          remove_referrer: "Enlever l'agent",
@@ -468,6 +473,7 @@ export default {
          'errors',
          'templatesAvailablesForAction',
          'me',
+         'version'
       ]),
       ...mapGetters([
          'hasResultsForAction',
@@ -595,7 +601,9 @@ export default {
       submit() {
          this.$store.dispatch('submit').catch((error) => {
             if (error.name === 'ValidationException' || error.name === 'AccessException') {
-               error.violations.forEach((violation) => this.$toast.open({message: violation}));
+              error.violations.forEach((violation) => this.$toast.open({message: violation}));
+            } else if (error.name === 'ConflictHttpException') {
+              this.$toast.open({message: this.$t('conflict_on_save')});
             } else {
                this.$toast.open({message: 'An error occurred'});
                throw error;
@@ -621,8 +629,10 @@ export default {
                   for (let v of error.violations) {
                         this.$toast.open({message: v });
                   }
+               } else if (error.name === 'ConflictHttpException') {
+                 this.$toast.open({message: this.$t('conflict_on_save')});
                } else {
-                     this.$toast.open({message: 'An error occurred'});
+                 this.$toast.open({message: 'An error occurred'});
                }
             })
       },

src/Bundle/ChillPersonBundle/Resources/public/vuejs/AccompanyingCourseWorkEdit/store.js

@@ -35,7 +35,8 @@ const store = createStore({
     referrers: window.accompanyingCourseWork.referrers,
     isPosting: false,
     errors: [],
-    me: null
+    me: null,
+    version: window.accompanyingCourseWork.version
   },
   getters: {
     socialAction(state) {
@@ -74,6 +75,7 @@ const store = createStore({
       return {
         type: 'accompanying_period_work',
         id: state.work.id,
+        version: state.version,
         startDate: state.startDate === null || state.startDate === '' ? null : {
           datetime: datetimeToISO(ISOToDate(state.startDate))
         },
@@ -501,22 +503,26 @@ const store = createStore({
     submit({ getters, state, commit }, callback) {
       let
         payload = getters.buildPayload,
-        url = `/api/1.0/person/accompanying-course/work/${state.work.id}.json`,
+        params = new URLSearchParams({'entity_version': state.version}),
+        url = `/api/1.0/person/accompanying-course/work/${state.work.id}.json?${params}`,
         errors = []
       ;
+
       commit('setIsPosting', true);
 
-      console.log('the social action', payload);
+      // console.log('the social action', payload);
 
       return makeFetch('PUT', url, payload)
         .then(data => {
           if (typeof(callback) !== 'undefined') {
             return callback(data);
           } else {
-            console.info('nothing to do here, bye bye');
+            // console.log('payload', payload.privateComment)
+            // console.info('nothing to do here, bye bye');
             window.location.assign(`/fr/person/accompanying-period/${state.work.accompanyingPeriod.id}/work`);
           }
         }).catch(error => {
+          console.log('error', error)
           commit('setIsPosting', false);
           throw error;
         });

src/Bundle/ChillPersonBundle/Tests/Controller/AccompanyingCoursWorkApiController/ConflictTest.php

@@ -0,0 +1,157 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * Chill is a software for social workers
+ *
+ * For the full copyright and license information, please view
+ * the LICENSE file that was distributed with this source code.
+ */
+
+namespace Tests\Controller\AccompanyingCoursWorkApiController;
+
+use Chill\MainBundle\Repository\UserRepositoryInterface;
+use Chill\MainBundle\Test\PrepareClientTrait;
+use Chill\PersonBundle\Entity\AccompanyingPeriod;
+use Chill\PersonBundle\Entity\Person;
+use Chill\PersonBundle\Entity\SocialWork\SocialAction;
+use Chill\PersonBundle\Entity\SocialWork\SocialIssue;
+use Doctrine\ORM\EntityManagerInterface;
+use Symfony\Bundle\FrameworkBundle\Test\WebTestCase;
+
+/**
+ * @internal
+ *
+ * @coversNothing
+ */
+class ConflictTest extends WebTestCase
+{
+    use PrepareClientTrait;
+
+    protected function setUp(): void
+    {
+        self::ensureKernelShutdown();
+    }
+
+    protected function tearDown(): void
+    {
+        self::ensureKernelShutdown();
+    }
+
+    /**
+     * @dataProvider generateAccompanyingPeriodWork
+     *
+     * @throws \JsonException
+     */
+    public function testWhenEditionAccompanyingPeriodWorkWithCurrentVersionNoConflictOccurs(AccompanyingPeriod\AccompanyingPeriodWork $work, int $personId): void
+    {
+        $client = $this->getClientAuthenticated();
+
+        $currentVersion = $work->getVersion();
+
+        $client->request(
+            'PUT',
+            "/api/1.0/person/accompanying-course/work/{$work->getid()}.json?entity_version={$currentVersion}",
+            content: json_encode([
+                'type' => 'accompanying_period_work',
+                'id' => $work->getId(),
+                'startDate' => [
+                    'datetime' => '2023-12-15T00:00:00+01:00',
+                ],
+                'endDate' => null,
+                'note' => 'This is a note',
+                'accompanyingPeriodWorkEvaluations' => [],
+                'goals' => [],
+                'handlingThirdParty' => null,
+                'persons' => [[
+                    'type' => 'person',
+                    'id' => $personId,
+                ],
+                ],
+                'privateComment' => '',
+                'refferers' => [],
+                'results' => [],
+                'thirdParties' => [],
+            ], JSON_THROW_ON_ERROR)
+        );
+
+        self::assertResponseIsSuccessful();
+        $w = json_decode($client->getResponse()->getContent(), true, 512, JSON_THROW_ON_ERROR);
+
+        self::assertEquals($work->getVersion() + 1, $w['version']);
+    }
+
+    /**
+     * @dataProvider generateAccompanyingPeriodWork
+     */
+    public function testWhenEditingAccompanyingPeriodWorkWithPreviousVersionAnHttpConflictResponseOccurs(AccompanyingPeriod\AccompanyingPeriodWork $work, int $personId): void
+    {
+        $client = $this->getClientAuthenticated();
+
+        $previous = $work->getVersion() - 1;
+
+        $client->request(
+            'PUT',
+            "/api/1.0/person/accompanying-course/work/{$work->getid()}.json?entity_version={$previous}",
+            content: json_encode([
+                'type' => 'accompanying_period_work',
+                'id' => $work->getId(),
+                'startDate' => [
+                    'datetime' => '2023-12-15T00:00:00+01:00',
+                ],
+                'endDate' => null,
+                'note' => 'This is a note',
+                'accompanyingPeriodWorkEvaluations' => [],
+                'goals' => [],
+                'handlingThirdParty' => null,
+                'persons' => [[
+                    'type' => 'person',
+                    'id' => $personId,
+                ],
+                ],
+                'privateComment' => '',
+                'refferers' => [],
+                'results' => [],
+                'thirdParties' => [],
+            ], JSON_THROW_ON_ERROR)
+        );
+
+        self::assertResponseStatusCodeSame(409);
+    }
+
+    public function generateAccompanyingPeriodWork(): iterable
+    {
+        self::bootKernel();
+        $em = self::$container->get(EntityManagerInterface::class);
+        $userRepository = self::$container->get(UserRepositoryInterface::class);
+        $user = $userRepository->findOneByUsernameOrEmail('center a_social');
+
+        $period = new AccompanyingPeriod();
+        $em->persist($period);
+        $period->addPerson(($p = new Person())->setFirstName('test')->setLastName('test')
+            ->setBirthdate(new \DateTime('1980-01-01'))->setGender(Person::BOTH_GENDER));
+        $em->persist($p);
+        $issue = (new SocialIssue())->setTitle(['fr' => 'test']);
+        $em->persist($issue);
+        $action = (new SocialAction())->setIssue($issue);
+        $em->persist($action);
+
+        $work = new AccompanyingPeriod\AccompanyingPeriodWork();
+        $work
+            ->setAccompanyingPeriod($period)
+            ->setStartDate(new \DateTimeImmutable())
+            ->addPerson($p)
+            ->setSocialAction($action)
+            ->setCreatedBy($user)
+            ->setUpdatedBy($user)
+        ;
+        $em->persist($work);
+
+        $em->flush();
+
+        self::ensureKernelShutdown();
+
+        yield [$work, $p->getId()];
+    }
+}

src/Bundle/ChillPersonBundle/migrations/Version20231129113816.php

@@ -0,0 +1,33 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * Chill is a software for social workers
+ *
+ * For the full copyright and license information, please view
+ * the LICENSE file that was distributed with this source code.
+ */
+
+namespace Chill\Migrations\Person;
+
+use Doctrine\DBAL\Schema\Schema;
+use Doctrine\Migrations\AbstractMigration;
+
+final class Version20231129113816 extends AbstractMigration
+{
+    public function getDescription(): string
+    {
+        return 'Add versioning to accompanying period work';
+    }
+
+    public function up(Schema $schema): void
+    {
+        $this->addSql('ALTER TABLE chill_person_accompanying_period_work ADD version INT DEFAULT 1 NOT NULL');
+    }
+
+    public function down(Schema $schema): void
+    {
+        $this->addSql('ALTER TABLE chill_person_accompanying_period_work DROP version');
+    }
+}

src/Bundle/ChillPersonBundle/translations/validators.fr.yml

@@ -21,6 +21,7 @@ The firstname cannot be empty: Le prénom ne peut pas être vide
 The lastname cannot be empty: Le nom de famille ne peut pas être vide
 The gender must be set: Le genre doit être renseigné
 You are not allowed to perform this action: Vous n'avez pas le droit de changer cette valeur.
+Sorry, but someone else has already changed this entity. Please refresh the page and apply the changes again: Désolé, mais quelqu'un d'autre a déjà modifié cette entité. Veuillez actualiser la page et appliquer à nouveau les modifications
 
 A center is required: Un centre est requis