Listeners: Refactor code, add AccessUrlListenerTrait, add session test

Refactor phpunit tests

src/CoreBundle/Entity/Course.php

@@ -386,7 +386,7 @@ class Course extends AbstractResource implements ResourceInterface, ResourceWith
         $this->courseTypeId = null;
         $this->room = null;
         $this->courseLanguage = 'en';
-        $this->visibility = COURSE_VISIBILITY_OPEN_PLATFORM;
+        $this->visibility = self::OPEN_PLATFORM;
         $this->subscribe = true;
         $this->unsubscribe = false;
         //$this->specificFieldValues = new ArrayCollection();
@@ -644,10 +644,7 @@ class Course extends AbstractResource implements ResourceInterface, ResourceWith
         return $this->getTitle();
     }
 
-    /**
-     * @return string
-     */
-    public function getTitleAndCode()
+    public function getTitleAndCode(): string
     {
         return $this->getTitle().' ('.$this->getCode().')';
     }
@@ -659,12 +656,7 @@ class Course extends AbstractResource implements ResourceInterface, ResourceWith
         return $this;
     }
 
-    /**
-     * Get description.
-     *
-     * @return string
-     */
-    public function getDescription()
+    public function getDescription(): ?string
     {
         return $this->description;
     }

src/CoreBundle/Entity/Listener/ResourceListener.php

@@ -20,6 +20,7 @@ use Chamilo\CoreBundle\Entity\Session;
 use Chamilo\CoreBundle\Entity\User;
 use Chamilo\CoreBundle\Security\Authorization\Voter\ResourceNodeVoter;
 use Chamilo\CoreBundle\ToolChain;
+use Chamilo\CoreBundle\Traits\AccessUrlListenerTrait;
 use Chamilo\CourseBundle\Entity\CGroup;
 use Cocur\Slugify\SlugifyInterface;
 use Doctrine\ORM\Event\LifecycleEventArgs;
@@ -34,11 +35,12 @@ use Symfony\Component\Security\Core\Security;
 
 class ResourceListener
 {
+    use AccessUrlListenerTrait;
+
     protected SlugifyInterface $slugify;
     protected Security $security;
     protected ToolChain $toolChain;
     protected RequestStack $request;
-    protected ?AccessUrl $accessUrl;
 
     public function __construct(
         SlugifyInterface $slugify,
@@ -50,34 +52,6 @@ class ResourceListener
         $this->security = $security;
         $this->toolChain = $toolChain;
         $this->request = $request;
-        $this->accessUrl = null;
-    }
-
-    public function getAccessUrl($em): ?AccessUrl
-    {
-        if (null === $this->accessUrl) {
-            $request = $this->request->getCurrentRequest();
-            if (null === $request) {
-                return null;
-            }
-
-            $sessionRequest = $request->getSession();
-            $id = (int) $sessionRequest->get('access_url_id');
-            if (0 !== $id) {
-                /** @var AccessUrl $url */
-                $url = $em->getRepository(AccessUrl::class)->find($id);
-
-                if (null !== $url) {
-                    $this->accessUrl = $url;
-
-                    return $url;
-                }
-            }
-
-            return null;
-        }
-
-        return $this->accessUrl;
     }
 
     /**
@@ -93,7 +67,7 @@ class ResourceListener
             if (0 === $resource->getUrls()->count()) {
                 // The AccessUrl was not added using $resource->addAccessUrl(),
                 // try getting the URL from the session if possible.
-                $accessUrl = $this->getAccessUrl($em);
+                $accessUrl = $this->getAccessUrl($em, $request);
                 if (null === $accessUrl) {
                     throw new Exception('This resource needs an AccessUrl use $resource->addAccessUrl();');
                 }
@@ -269,7 +243,7 @@ class ResourceListener
 
         // All resources should have a parent, except AccessUrl.
         if (!($resource instanceof AccessUrl) && null === $resourceNode->getParent()) {
-            throw new InvalidArgumentException('Resource Node should have a parent');
+            throw new InvalidArgumentException(\sprintf('Resource %s Node should have a parent', $resource->getResourceName()));
         }
     }
 

src/CoreBundle/Entity/Listener/SessionListener.php

@@ -9,6 +9,7 @@ namespace Chamilo\CoreBundle\Entity\Listener;
 use Chamilo\CoreBundle\Entity\AccessUrl;
 use Chamilo\CoreBundle\Entity\Session;
 use Chamilo\CoreBundle\Repository\SessionRepository;
+use Chamilo\CoreBundle\Traits\AccessUrlListenerTrait;
 use Doctrine\ORM\Event\LifecycleEventArgs;
 use Exception;
 use Symfony\Component\HttpFoundation\RequestStack;
@@ -20,6 +21,8 @@ use Symfony\Component\Security\Core\Security;
  */
 class SessionListener
 {
+    use AccessUrlListenerTrait;
+
     protected RequestStack $request;
     protected Security $security;
 
@@ -31,25 +34,24 @@ class SessionListener
 
     /**
      * This code is executed when a new session is created.
-     *
-     * new object : prePersist
-     * edited object: preUpdate
-     *
-     * @throws Exception
      */
     public function prePersist(Session $session, LifecycleEventArgs $args): void
     {
         $em = $args->getEntityManager();
-        $id = $this->request->getCurrentRequest()->getSession()->get('access_url_id');
-        $url = $em->getRepository(AccessUrl::class)->find($id);
-        $session->addUrl($url);
+        if (0 === $session->getUrls()->count()) {
+            // The AccessUrl was not added using $resource->addAccessUrl(),
+            // try getting the URL from the session if possible.
+            $accessUrl = $this->getAccessUrl($em, $this->request);
+            if (null === $accessUrl) {
+                throw new Exception('This resource needs an AccessUrl use $resource->addAccessUrl();');
+            }
+            $session->addAccessUrl($accessUrl);
+        }
         //$this->checkLimit($repo, $url);
     }
 
     /**
      * This code is executed when a session is updated.
-     *
-     * @throws Exception
      */
     public function preUpdate(Session $session, LifecycleEventArgs $args): void
     {

src/CoreBundle/Entity/Session.php

@@ -18,6 +18,7 @@ use Doctrine\Common\Collections\Collection;
 use Doctrine\Common\Collections\Criteria;
 use Doctrine\ORM\Mapping as ORM;
 use Exception;
+use Symfony\Bridge\Doctrine\Validator\Constraints\UniqueEntity;
 use Symfony\Component\Serializer\Annotation\Groups;
 use Symfony\Component\Validator\Constraints as Assert;
 
@@ -56,8 +57,9 @@ use Symfony\Component\Validator\Constraints as Assert;
  * )
  * @ORM\EntityListeners({"Chamilo\CoreBundle\Entity\Listener\SessionListener"})
  * @ORM\Entity(repositoryClass="Chamilo\CoreBundle\Repository\SessionRepository")
+ * @UniqueEntity("name")
  */
-class Session
+class Session implements ResourceWithAccessUrlInterface
 {
     public const VISIBLE = 1;
     public const READ_ONLY = 2;
@@ -140,6 +142,7 @@ class Session
 
     /**
      * @Assert\NotBlank()
+     *
      * @Groups({"session:read", "session:write", "session_rel_course_rel_user:read", "document:read", "session_rel_user:read"})
      * @ORM\Column(name="name", type="string", length=150)
      */
@@ -1011,28 +1014,34 @@ class Session
         return $this->urls;
     }
 
-    public function setUrls(Collection $urls): void
+    public function setUrls(Collection $urls): self
     {
         $this->urls = new ArrayCollection();
 
         foreach ($urls as $url) {
             $this->addUrls($url);
         }
+
+        return $this;
     }
 
-    public function addUrl(AccessUrl $url): void
+    public function addAccessUrl(AccessUrl $url): self
     {
         $accessUrlRelSession = new AccessUrlRelSession();
         $accessUrlRelSession->setUrl($url);
         $accessUrlRelSession->setSession($this);
 
         $this->addUrls($accessUrlRelSession);
+
+        return $this;
     }
 
-    public function addUrls(AccessUrlRelSession $url): void
+    public function addUrls(AccessUrlRelSession $url): self
     {
         $url->setSession($this);
-        $this->urls[] = $url;
+        $this->urls->add($url);
+
+        return $this;
     }
 
     /**

src/CoreBundle/Entity/User.php

@@ -710,7 +710,7 @@ class User implements UserInterface, EquatableInterface, ResourceInterface, Reso
         $this->status = self::STUDENT;
         $this->salt = sha1(uniqid('', true));
         $this->active = true;
-        $this->enabled = false;
+        $this->enabled = true;
         $this->locked = false;
         $this->expired = false;
 

src/CoreBundle/Repository/Node/IllustrationRepository.php

@@ -70,7 +70,7 @@ final class IllustrationRepository extends ResourceRepository implements GridInt
      */
     public function addIllustration(
         ResourceInterface $resource,
-        User $user,
+        User $creator,
         UploadedFile $uploadFile = null,
         string $crop = ''
     ): ?ResourceFile {
@@ -83,7 +83,7 @@ final class IllustrationRepository extends ResourceRepository implements GridInt
 
         if (null === $illustrationNode) {
             $illustration = new Illustration();
-            $this->addResourceNode($illustration, $user, $resource);
+            $this->addResourceNode($illustration, $creator, $resource);
             $em->persist($illustration);
         } else {
             $illustration = $this->findOneBy([

src/CoreBundle/Repository/SessionRepository.php

@@ -29,6 +29,12 @@ class SessionRepository extends ServiceEntityRepository
         parent::__construct($registry, Session::class);
     }
 
+    public function create(Session $session): void
+    {
+        $this->getEntityManager()->persist($session);
+        $this->getEntityManager()->flush();
+    }
+
     /**
      * @return SessionRelUser[]
      */
@@ -44,6 +50,9 @@ class SessionRepository extends ServiceEntityRepository
         return $qb->getQuery()->getResult();
     }
 
+    /**
+     * @return SessionRelUser[]
+     */
     public function getUsersByCourse(Session $session, Course $course, AccessUrl $url)
     {
         if (0 === $session->getUsers()->count()) {

src/CoreBundle/Traits/AccessUrlListenerTrait.php

@@ -0,0 +1,43 @@
+<?php
+
+declare(strict_types=1);
+
+/* For licensing terms, see /license.txt */
+
+namespace Chamilo\CoreBundle\Traits;
+
+use Chamilo\CoreBundle\Entity\AccessUrl;
+use Doctrine\ORM\EntityManagerInterface;
+use Symfony\Component\HttpFoundation\RequestStack;
+
+trait AccessUrlListenerTrait
+{
+    protected ?AccessUrl $accessUrl = null;
+
+    public function getAccessUrl(EntityManagerInterface $em, RequestStack $request): ?AccessUrl
+    {
+        if (null === $this->accessUrl) {
+            $request = $request->getCurrentRequest();
+            if (null === $request) {
+                return null;
+            }
+
+            $sessionRequest = $request->getSession();
+            $id = (int) $sessionRequest->get('access_url_id');
+            if (0 !== $id) {
+                /** @var AccessUrl $url */
+                $url = $em->getRepository(AccessUrl::class)->find($id);
+
+                if (null !== $url) {
+                    $this->accessUrl = $url;
+
+                    return $url;
+                }
+            }
+
+            return null;
+        }
+
+        return $this->accessUrl;
+    }
+}

tests/ChamiloTestTrait.php

@@ -4,22 +4,33 @@ namespace Chamilo\Tests;
 
 use Chamilo\CoreBundle\Entity\AccessUrl;
 use Chamilo\CoreBundle\Entity\Course;
+use Chamilo\CoreBundle\Entity\Session;
 use Chamilo\CoreBundle\Entity\User;
 use Chamilo\CoreBundle\Repository\Node\AccessUrlRepository;
 use Chamilo\CoreBundle\Repository\Node\CourseRepository;
 use Chamilo\CoreBundle\Repository\Node\UserRepository;
+use Chamilo\CoreBundle\Repository\SessionRepository;
+use Symfony\Component\HttpFoundation\File\UploadedFile;
 use Symfony\Component\Validator\ConstraintViolationList;
 use Symfony\Component\Validator\Validator\ValidatorInterface;
 
 trait ChamiloTestTrait
 {
-    public function createUser(string $username, string $password, string $email): ?User
+    public function createUser(string $username, string $password = '', string $email = ''): ?User
     {
         /** @var UserRepository $repo */
         $repo = static::getContainer()->get(UserRepository::class);
 
         $admin = $this->getUser('admin');
 
+        if (empty($email)) {
+            $email = "$username@example.com";
+        }
+
+        if (empty($password)) {
+            $password = $username;
+        }
+
         $user = $repo->createUser()
             ->setLastname($username)
             ->setFirstname($username)
@@ -48,16 +59,32 @@ trait ChamiloTestTrait
 
     public function createCourse($title): ?Course
     {
-        $courseRepo = self::getContainer()->get(CourseRepository::class);
+        $repo = self::getContainer()->get(CourseRepository::class);
         $course = (new Course())
             ->setTitle($title)
             ->addAccessUrl($this->getAccessUrl())
-            ->setCreator($this->getUser('admin'));
-        $courseRepo->create($course);
+            ->setCreator($this->getUser('admin'))
+        ;
+
+        $repo->create($course);
 
         return $course;
     }
 
+    public function createSession($title): ?Session
+    {
+        $repo = self::getContainer()->get(SessionRepository::class);
+
+        $session = (new Session())
+            ->setName($title)
+            ->setGeneralCoach($this->getUser('admin'))
+            ->addAccessUrl($this->getAccessUrl())
+        ;
+        $repo->create($session);
+
+        return $session;
+    }
+
     public function getAccessUrl(string $url = ''): ?AccessUrl
     {
         if (empty($url)) {
@@ -72,10 +99,8 @@ trait ChamiloTestTrait
 
     public function assertHasNoEntityViolations($entity)
     {
-        /** @var ValidatorInterface $validator */
-        $validator = static::$kernel->getContainer()->get('validator');
         /** @var ConstraintViolationList $errors */
-        $errors = $validator->validate($entity);
+        $errors = $this->getViolations($entity);
 
         $message = [];
         foreach ($errors as $error) {
@@ -84,4 +109,27 @@ trait ChamiloTestTrait
 
         $this->assertEquals(0, $errors->count(), implode(', ', $message));
     }
+
+    public function getUploadableFile(): UploadedFile
+    {
+        $path = $this->getContainer()->get('kernel')->getProjectDir();
+        $filePath = $path.'/public/img/logo.png';
+        $fileName = basename($filePath);
+
+        return new UploadedFile(
+            $filePath,
+            $fileName,
+            'image/png',
+        );
+    }
+
+    public function getViolations($entity)
+    {
+        /** @var ValidatorInterface $validator */
+        $validator = static::$kernel->getContainer()->get('validator');
+        /** @var ConstraintViolationList $errors */
+        $errors = $validator->validate($entity);
+
+        return $errors;
+    }
 }

tests/CoreBundle/Repository/Node/CourseRepositoryTest.php

@@ -36,41 +36,95 @@ class CourseRepositoryTest extends WebTestCase
         $courseRepo->create($course);
     }
 
+    public function testCreateCourseSameTitle(): void
+    {
+        self::bootKernel();
+        $course = $this->createCourse('Test course');
+        $this->assertSame('TESTCOURSE', $course->getCode());
+
+        $course = $this->createCourse('Test course');
+        $this->assertSame('TESTCOURSE1', $course->getCode());
+    }
+
     /**
-     * Create a course with a creator.
+     * Create a course with a creator + check course tool creation (ToolChain).
      */
     public function testCreate(): void
     {
         $courseRepo = self::getContainer()->get(CourseRepository::class);
         $course = $this->createCourse('Test course');
 
+        $this->assertHasNoEntityViolations($course);
+
         $count = $courseRepo->count([]);
         $this->assertSame(1, $count);
 
         // Check tools.
         $this->assertSame(25, \count($course->getTools()));
 
-        // Check course code.
-        $this->assertSame('TESTCOURSE', $course->getCode());
-
-        // The course should connected with a Access URL
+        // The course should connected with the current Access URL.
         $this->assertSame(1, $course->getUrls()->count());
     }
 
-    public function testCourseAccess(): void
+    public function testCourseStudentSubscription(): void
     {
-        self::bootKernel();
+        $client = static::createClient();
+
         /** @var CourseRepository $courseRepo */
         $courseRepo = self::getContainer()->get(CourseRepository::class);
-        //$toolChain = self::getContainer()->get(ToolChain::class);
+
+        // Create default course.
         $course = $this->createCourse('Test course');
+        $course->setVisibility(Course::REGISTERED);
 
-        $student = $this->createUser('student', 'student', 'student@student.com');
+        // Create a user.
+        $student = $this->createUser('student', 'student');
 
+        // Add user to the course.
         $course->addUser($student, 0, null, 5);
+        $courseRepo->update($course);
+
+        $this->assertSame(1, $course->getUsers()->count());
+
+        // retrieve the admin
+        $user = $this->getUser('student');
 
+        $client->loginUser($user);
+
+        $client->request('GET', sprintf('/course/%s/home', $course->getId()));
+        $this->assertResponseIsSuccessful();
+    }
+
+    public function testCourseRegisteredVisibility(): void
+    {
+        $client = static::createClient();
+
+        /** @var CourseRepository $courseRepo */
+        $courseRepo = self::getContainer()->get(CourseRepository::class);
+
+        // Create default course.
+        $course = $this->createCourse('Test course');
+        $course->setVisibility(Course::REGISTERED);
+        $courseRepo->update($course);
+
+        // Create a user.
+        $student = $this->createUser('student', 'student');
+
+        // Add user to the course.
+        $course->addUser($student, 0, null, 5);
         $courseRepo->update($course);
 
         $this->assertSame(1, $course->getUsers()->count());
+
+        // retrieve the admin
+        $user = $this->getUser('student');
+
+        $client->loginUser($user);
+
+        $client->request('GET', sprintf('/course/%s/home', $course->getId()));
+        $this->assertResponseIsSuccessful();
+
+        // Create a user.
+        $student2 = $this->createUser('student2');
     }
 }

tests/CoreBundle/Repository/Node/PersonalFileRepositoryTest.php

@@ -6,7 +6,6 @@ declare(strict_types=1);
 
 namespace Chamilo\Tests\CoreBundle\Repository\Node;
 
-use Chamilo\CoreBundle\Repository\Node\PersonalFileRepository;
 use Chamilo\Tests\AbstractApiTest;
 use Chamilo\Tests\ChamiloTestTrait;
 use Symfony\Component\HttpFoundation\File\UploadedFile;
@@ -24,7 +23,7 @@ class PersonalFileRepositoryTest extends AbstractApiTest
         $username = 'test';
         $password = 'test';
 
-        $user = $this->createUser($username, $password, 'test@test.com');
+        $user = $this->createUser($username, $password);
         $token = $this->getUserToken([
             'username' => $username,
             'password' => $password,
@@ -57,7 +56,6 @@ class PersonalFileRepositoryTest extends AbstractApiTest
                     'filetype' => 'file',
                     'size' => filesize($filePath),
                     'parentResourceNodeId' => $resourceNodeId,
-                    //'resourceLinkList' => json_encode($resourceLinkList),
                 ],
             ]
         );
@@ -74,7 +72,7 @@ class PersonalFileRepositoryTest extends AbstractApiTest
 
         $url = $response->toArray()['contentUrl'];
 
-        // Access file as anon.
+        // Access file as anon, redirects to the login.
         $this->createClient()->request(
             'GET',
             $url
@@ -82,7 +80,7 @@ class PersonalFileRepositoryTest extends AbstractApiTest
         $this->assertResponseRedirects('/login');
 
         // Access file as another user should be forbidden.
-        $this->createUser('another', 'another', 'another@test.com');
+        $this->createUser('another', 'another');
         $client = $this->getClientWithGuiCredentials('another', 'another');
         $client->request(
             'GET',
@@ -90,7 +88,7 @@ class PersonalFileRepositoryTest extends AbstractApiTest
         );
         $this->assertResponseStatusCodeSame(403); // forbidden
 
-        // Acces with the same user should be allowed.
+        // Access with the same user should be allowed.
         $client = $this->getClientWithGuiCredentials($username, $password);
         // Access file as user that created the file.
         $client->request(

tests/CoreBundle/Repository/Node/SessionRepositoryTest.php

@@ -0,0 +1,60 @@
+<?php
+
+declare(strict_types=1);
+
+/* For licensing terms, see /license.txt */
+
+namespace Chamilo\Tests\CoreBundle\Repository\Node;
+
+use Chamilo\CoreBundle\Entity\Session;
+use Chamilo\CoreBundle\Repository\SessionRepository;
+use Chamilo\Tests\ChamiloTestTrait;
+use Doctrine\DBAL\Exception\UniqueConstraintViolationException;
+use Symfony\Bundle\FrameworkBundle\Test\WebTestCase;
+
+/**
+ * @covers \SessionRepository
+ */
+class SessionRepositoryTest extends WebTestCase
+{
+    use ChamiloTestTrait;
+
+    /**
+     * Create a session.
+     */
+    public function testCreate(): void
+    {
+        self::bootKernel();
+        /** @var SessionRepository $repo */
+        $repo = self::getContainer()->get(SessionRepository::class);
+
+        $session = $this->createSession('session');
+
+        $this->assertHasNoEntityViolations($session);
+
+        $count = $repo->count([]);
+
+        $this->assertSame(1, $count);
+    }
+
+    public function testCreateSessionSameTitle(): void
+    {
+        self::bootKernel();
+        $name = 'session';
+        $session = $this->createSession($name);
+        $this->assertHasNoEntityViolations($session);
+
+        $repo = self::getContainer()->get(SessionRepository::class);
+
+        $session = (new Session())
+            ->setName($name)
+            ->setGeneralCoach($this->getUser('admin'))
+            ->addAccessUrl($this->getAccessUrl())
+        ;
+        $errors = $this->getViolations($session);
+        $this->assertSame(1, \count($errors));
+
+        $this->expectException(UniqueConstraintViolationException::class);
+        $repo->create($session);
+    }
+}

tests/CoreBundle/Repository/Node/UserRepositoryTest.php

@@ -27,7 +27,7 @@ class UserRepositoryTest extends KernelTestCase
     {
         self::bootKernel();
 
-        $this->createUser('user', 'user', 'user@example.org');
+        $this->createUser('user', 'user');
 
         $count = self::getContainer()->get(UserRepository::class)->count([]);
         $this->assertSame(3, $count);