Remove ResourceLink::VISIBILITY_DELETED in favor of soft delete

AngelFQC · Mar 11, 2024 · d7d9dba · d7d9dba
1 parent 4aa246c
commit d7d9dba
Show file tree

Hide file tree

Showing 15 changed files with 31 additions and 76 deletions.
diff --git a/public/main/exercise/exercise.class.php b/public/main/exercise/exercise.class.php
@@ -9,6 +9,7 @@
 use Chamilo\CoreBundle\Entity\TrackEExerciseConfirmation;
 use Chamilo\CoreBundle\Entity\TrackEHotspot;
 use Chamilo\CoreBundle\Framework\Container;
+use Chamilo\CoreBundle\Repository\ResourceLinkRepository;
 use Chamilo\CourseBundle\Entity\CExerciseCategory;
 use Chamilo\CourseBundle\Entity\CQuiz;
 use Chamilo\CourseBundle\Entity\CQuizRelQuestionCategory;
@@ -1805,7 +1806,9 @@ public function delete()
         $exerciseId = $this->iId;
 
         $repo = Container::getQuizRepository();
+        /** @var CQuiz $exercise */
         $exercise = $repo->find($exerciseId);
+        $linksRepo = Container::$container->get(ResourceLinkRepository::class);
 
         if (null === $exercise) {
             return false;
@@ -1825,7 +1828,14 @@ public function delete()
                 WHERE iid = $exerciseId";
         Database::query($sql);
 
-        $repo->softDelete($exercise);
+        $course = api_get_course_entity();
+        $session = api_get_session_entity();
+
+        $link = $exercise->resourceNode->getResourceLinkByContext($course, $session);
+
+        if ($link) {
+            $linksRepo->remove($link);
+        }
 
         SkillModel::deleteSkillsFromItem($exerciseId, ITEM_TYPE_EXERCISE);
 

diff --git a/public/main/inc/lib/document.lib.php b/public/main/inc/lib/document.lib.php
@@ -581,7 +581,7 @@ public static function get_all_document_folders(
                         docs.filetype = 'folder' AND
                         $groupCondition AND
                         n.path NOT LIKE '%shared_folder%' AND
-                        l.visibility NOT IN ('".ResourceLink::VISIBILITY_DELETED."')
+                        l.deleted_at IS NULL
                         $condition_session ";
 
             if (0 != $groupIid) {
@@ -681,7 +681,7 @@ public static function get_all_document_folders(
                         WHERE
                             docs.filetype = 'folder' AND
                             $groupCondition AND
-                            l.visibility NOT IN ('".ResourceLink::VISIBILITY_DELETED."')
+                            l.deleted_at IS NULL
                             $condition_session AND
                             l.c_id = $courseId ";
                 $folder_in_invisible_result = Database::query($sql);
@@ -3139,7 +3139,7 @@ public static function build_directory_selector(
                         l.c_id = $course_id AND
                         docs.filetype = 'folder' AND
                         n.path IN ('".$folder_sql."') AND
-                        l.visibility NOT IN ('".ResourceLink::VISIBILITY_DELETED."')
+                        l.deleted_at IS NULL
                          ";
 
             /*$sql = "SELECT path, title

diff --git a/public/main/inc/lib/message.lib.php b/public/main/inc/lib/message.lib.php
@@ -469,17 +469,6 @@ public static function send_message_simple(
         return $result;
     }
 
-    public static function softDeleteAttachments(Message $message): void
-    {
-        $attachments = $message->getAttachments();
-        if (!empty($attachments)) {
-            $repo = Container::getMessageAttachmentRepository();
-            foreach ($attachments as $file) {
-                $repo->softDelete($file);
-            }
-        }
-    }
-
     /**
      * Saves a message attachment files.
      *

diff --git a/src/CoreBundle/DataProvider/Extension/CToolExtension.php b/src/CoreBundle/DataProvider/Extension/CToolExtension.php
@@ -39,8 +39,6 @@ public function applyToCollection(
             ->andWhere(
                 $queryBuilder->expr()->notIn("$alias.title", ['course_tool', 'course_homepage'])
             )
-            ->andWhere('resource_links.visibility != :visibility_deleted')
-            ->setParameter('visibility_deleted', ResourceLink::VISIBILITY_DELETED)
         ;
     }
 }
diff --git a/src/CoreBundle/DataProvider/Extension/CourseLinkExtensionTrait.php b/src/CoreBundle/DataProvider/Extension/CourseLinkExtensionTrait.php
@@ -37,12 +37,6 @@ protected function addCourseLinkWithVisibilityConditions(
 
     protected function addVisibilityCondition(QueryBuilder $queryBuilder): void
     {
-        // Do not show deleted resources.
-        $queryBuilder
-            ->andWhere('links.visibility != :visibilityDeleted')
-            ->setParameter('visibilityDeleted', ResourceLink::VISIBILITY_DELETED)
-        ;
-
         $allowDraft =
             $this->security->isGranted('ROLE_ADMIN')
             || $this->security->isGranted('ROLE_CURRENT_COURSE_TEACHER');

diff --git a/src/CoreBundle/DataProvider/Extension/PersonalFileExtension.php b/src/CoreBundle/DataProvider/Extension/PersonalFileExtension.php
@@ -71,11 +71,6 @@ private function addWhere(QueryBuilder $queryBuilder, string $resourceClass): vo
         if ($isShared) {
             $queryBuilder->leftJoin('node.resourceLinks', 'links');
 
-            /*$queryBuilder
-                ->andWhere('links.visibility != :visibilityDeleted')
-                ->setParameter('visibilityDeleted', ResourceLink::VISIBILITY_DELETED)
-            ;*/
-
             $queryBuilder
                 ->andWhere('links.visibility = :visibility')
                 ->setParameter('visibility', ResourceLink::VISIBILITY_PUBLISHED)

diff --git a/src/CoreBundle/Entity/ResourceLink.php b/src/CoreBundle/Entity/ResourceLink.php
@@ -32,7 +32,6 @@ class ResourceLink implements Stringable
     public const VISIBILITY_DRAFT = 0;
     public const VISIBILITY_PENDING = 1;
     public const VISIBILITY_PUBLISHED = 2;
-    public const VISIBILITY_DELETED = 3;
 
     #[ORM\Id]
     #[ORM\Column(type: 'integer')]
@@ -295,7 +294,6 @@ public static function getVisibilityList(): array
             'Draft' => self::VISIBILITY_DRAFT,
             'Pending' => self::VISIBILITY_PENDING,
             'Published' => self::VISIBILITY_PUBLISHED,
-            'Deleted' => self::VISIBILITY_DELETED,
         ];
     }
 

diff --git a/src/CoreBundle/Migrations/AbstractMigrationChamilo.php b/src/CoreBundle/Migrations/AbstractMigrationChamilo.php
@@ -260,6 +260,7 @@ public function fixItemProperty(
             $userId = (int) $item['insert_user_id'];
             $sessionId = $item['session_id'] ?? 0;
             $groupId = $item['to_group_id'] ?? 0;
+            $lastUpdatedAt = new \DateTime($item['lastedit_date'], new \DateTimeZone('UTC'));
 
             $newVisibility = ResourceLink::VISIBILITY_DRAFT;
 
@@ -273,11 +274,6 @@ public function fixItemProperty(
                 case 1:
                     $newVisibility = ResourceLink::VISIBILITY_PUBLISHED;
 
-                    break;
-
-                case 2:
-                    $newVisibility = ResourceLink::VISIBILITY_DELETED;
-
                     break;
             }
 
@@ -321,6 +317,12 @@ public function fixItemProperty(
                 $em->persist($resourceNode);
             }
             $resource->addCourseLink($course, $session, $group, $newVisibility);
+
+            if (2 === $visibility) {
+                $link = $resource->getResourceNode()->getResourceLinkByContext($course, $session, $group);
+                $link->setDeletedAt($lastUpdatedAt);
+            }
+
             $em->persist($resource);
         }
 

diff --git a/src/CoreBundle/Repository/ResourceLinkRepository.php b/src/CoreBundle/Repository/ResourceLinkRepository.php
@@ -25,6 +25,7 @@ public function remove(ResourceLink $resourceLink): void
         $resourceLink->setDisplayOrder(-1);
 
         $em->flush();
+        // soft delete handled by Gedmo\SoftDeleteable
         $em->remove($resourceLink);
         $em->flush();
     }

diff --git a/src/CoreBundle/Repository/ResourceNodeRepository.php b/src/CoreBundle/Repository/ResourceNodeRepository.php
@@ -145,7 +145,6 @@ public function getSize(ResourceNode $resourceNode, ResourceType $type, ?Course
             ->innerJoin('node.resourceLinks', 'l')
             ->where('node.resourceType = :type')
             ->andWhere('node.parent = :parentNode')
-            ->andWhere('l.visibility <> :visibility')
             ->andWhere('file IS NOT NULL')
         ;
 
@@ -154,7 +153,6 @@ public function getSize(ResourceNode $resourceNode, ResourceType $type, ?Course
             $qb->andWhere('l.course = :course');
             $params['course'] = $course;
         }
-        $params['visibility'] = ResourceLink::VISIBILITY_DELETED;
         $params['parentNode'] = $resourceNode;
         $params['type'] = $type;
 

diff --git a/src/CoreBundle/Repository/ResourceRepository.php b/src/CoreBundle/Repository/ResourceRepository.php
@@ -282,12 +282,6 @@ public function addVisibilityQueryBuilder(?QueryBuilder $qb = null, bool $checkS
             $checker->isGranted('ROLE_ADMIN')
             || $checker->isGranted('ROLE_CURRENT_COURSE_TEACHER');
 
-        // Do not show deleted resources.
-        $qb
-            ->andWhere('links.visibility != :visibilityDeleted')
-            ->setParameter('visibilityDeleted', ResourceLink::VISIBILITY_DELETED, Types::INTEGER)
-        ;
-
         if ($displayOnlyPublished) {
             if (!$isAdminOrTeacher
                 || ($checkStudentView && 'studentview' === $sessionStudentView)
@@ -605,14 +599,6 @@ public function setResourceName(AbstractResource $resource, $title): void
         // }
     }
 
-    /**
-     * Change all links visibility to DELETED.
-     */
-    public function softDelete(AbstractResource $resource): void
-    {
-        $this->setLinkVisibility($resource, ResourceLink::VISIBILITY_DELETED);
-    }
-
     public function toggleVisibilityPublishedDraft(AbstractResource $resource): void
     {
         $firstLink = $resource->getFirstResourceLink();
@@ -638,11 +624,6 @@ public function setVisibilityDraft(AbstractResource $resource): void
         $this->setLinkVisibility($resource, ResourceLink::VISIBILITY_DRAFT);
     }
 
-    public function setVisibilityDeleted(AbstractResource $resource): void
-    {
-        $this->setLinkVisibility($resource, ResourceLink::VISIBILITY_DELETED);
-    }
-
     public function setVisibilityPending(AbstractResource $resource): void
     {
         $this->setLinkVisibility($resource, ResourceLink::VISIBILITY_PENDING);
@@ -729,12 +710,10 @@ public function getTotalSpaceByCourse(Course $course, ?CGroup $group = null, ?Se
             ->innerJoin('node.resourceLinks', 'l')
             ->innerJoin('node.resourceFile', 'file')
             ->where('l.course = :course')
-            ->andWhere('l.visibility <> :visibility')
             ->andWhere('file IS NOT NULL')
             ->setParameters(
                 [
                     'course' => $course,
-                    'visibility' => ResourceLink::VISIBILITY_DELETED,
                 ]
             )
         ;

diff --git a/src/CoreBundle/Security/Authorization/Voter/ResourceNodeVoter.php b/src/CoreBundle/Security/Authorization/Voter/ResourceNodeVoter.php
@@ -155,11 +155,6 @@ protected function voteOnAttribute(string $attribute, $subject, TokenInterface $
 
         // @todo implement view, edit, delete.
         foreach ($links as $link) {
-            // Block access if visibility is deleted. Creator and admin have already access.
-            if (ResourceLink::VISIBILITY_DELETED === $link->getVisibility()) {
-                continue;
-            }
-
             // Check if resource was sent to the current user.
             $linkUser = $link->getUser();
             if ($linkUser instanceof UserInterface

diff --git a/src/CourseBundle/Repository/CDocumentRepository.php b/src/CourseBundle/Repository/CDocumentRepository.php
@@ -54,10 +54,8 @@ public function findDocumentsByAuthor(int $userId)
             ->innerJoin('d.resourceNode', 'node')
             ->innerJoin('node.resourceLinks', 'l')
             ->where('l.user = :user')
-            ->andWhere('l.visibility <> :visibility')
             ->setParameters([
                 'user' => $userId,
-                'visibility' => ResourceLink::VISIBILITY_DELETED,
             ])
             ->getQuery()
         ;

diff --git a/tests/CourseBundle/Repository/CDocumentRepositoryTest.php b/tests/CourseBundle/Repository/CDocumentRepositoryTest.php
@@ -11,12 +11,14 @@
 use Chamilo\CoreBundle\Entity\ResourceNode;
 use Chamilo\CoreBundle\Entity\Session;
 use Chamilo\CoreBundle\Repository\Node\CourseRepository;
+use Chamilo\CoreBundle\Repository\ResourceLinkRepository;
 use Chamilo\CourseBundle\Entity\CDocument;
 use Chamilo\CourseBundle\Entity\CGroup;
 use Chamilo\CourseBundle\Repository\CDocumentRepository;
 use Chamilo\Tests\AbstractApiTest;
 use Chamilo\Tests\ChamiloTestTrait;
 use LogicException;
+use PHPUnit\Framework\Assert;
 use Symfony\Component\HttpFoundation\Request;
 
 class CDocumentRepositoryTest extends AbstractApiTest
@@ -982,6 +984,7 @@ public function testSetVisibility(): void
     {
         $course = $this->createCourse('Test');
         $documentRepo = self::getContainer()->get(CDocumentRepository::class);
+        $linksRepo = self::getContainer()->get(ResourceLinkRepository::class);
         $admin = $this->getUser('admin');
 
         $document = (new CDocument())
@@ -1031,15 +1034,9 @@ public function testSetVisibility(): void
         $this->assertSame(ResourceLink::VISIBILITY_PENDING, $link->getVisibility());
         $this->assertSame('Pending', $link->getVisibilityName());
 
-        $documentRepo->setVisibilityDeleted($document);
         $link = $document->getFirstResourceLink();
-        $this->assertSame(ResourceLink::VISIBILITY_DELETED, $link->getVisibility());
-        $this->assertSame('Deleted', $link->getVisibilityName());
-
-        $documentRepo->softDelete($document);
-        $link = $document->getFirstResourceLink();
-        $this->assertSame(ResourceLink::VISIBILITY_DELETED, $link->getVisibility());
-        $this->assertSame('Deleted', $link->getVisibilityName());
+        $linksRepo->remove($link);
+        $this->assertTrue($link->isDeleted());
     }
 
     public function testGetTotalSpaceByCourse(): void

diff --git a/tests/scripts/migrate_item_property.php b/tests/scripts/migrate_item_property.php
@@ -142,9 +142,6 @@
                 case '1':
                     $newVisibility = ResourceLink::VISIBILITY_PUBLISHED;
                     break;
-                case '2':
-                    $newVisibility = ResourceLink::VISIBILITY_DELETED;
-                    break;
             }
 
             $link = new ResourceLink();
@@ -157,6 +154,10 @@
                 ->setVisibility($newVisibility)
             ;
 
+            if (2 === (int) $row['visibility']) {
+                $link->setDeletedAt($lastUpdatedAt);
+            }
+
             if (!empty($rights)) {
                 foreach ($rights as $right) {
                     $link->addResourceRight($right);