Query perfomance improvement if club is already set (#47)

froozeify · Feb 19, 2025 · 2f7a399 · 2f7a399
2 parents f63d35b + 4c87888
commit 2f7a399
Show file tree

Hide file tree

Showing 11 changed files with 27 additions and 70 deletions.
diff --git a/src/Doctrine/UserClubExtension.php b/src/Doctrine/UserClubExtension.php
@@ -10,6 +10,7 @@
 use App\Entity\Interface\ClubLinkedEntityInterface;
 use App\Entity\User;
 use App\Enum\UserRole;
+use Doctrine\ORM\Query\Expr\Join;
 use Doctrine\ORM\QueryBuilder;
 use Symfony\Bundle\SecurityBundle\Security;
 use Symfony\Component\HttpFoundation\Response;
@@ -55,37 +56,9 @@ private function getUserClubs(): array {
     foreach ($user->getLinkedProfiles() as $club) {
       $userClubs[] = $club->getClub();
     }
-
     return $userClubs;
   }
 
-  private function getUserCurrentClubRole(Club $club) {
-    // TODO: Return the futur UserClub object
-    // It should contain a col `role`, that is the current role
-  }
-
-  private function checkUserIsClubSupervisor(Club $club): bool {
-    $user = $this->getUser();
-    foreach ($user->getMemberships() as $membership) {
-      if ($membership->getMember()?->getClub() === $club) {
-        return $membership->getRole()->isSupervisor();
-      }
-    }
-
-    return false;
-  }
-
-  private function checkUserIsClubAdmin(Club $club): bool {
-    $user = $this->getUser();
-    foreach ($user->getMemberships() as $membership) {
-      if ($membership->getMember()?->getClub() === $club) {
-        return $membership->getRole()->isAdmin();
-      }
-    }
-
-    return false;
-  }
-
   /**
    * Generate all the joins from the query string (exploding dot into join)
    *
@@ -129,19 +102,25 @@ private function addWhere(string $resourceClass): void {
     if (is_subclass_of($resourceClass, ClubLinkedEntityInterface::class)) {
       $userClubs = $this->getUserClubs();
 
-      $clubJoin = $this->addJoins($resourceClass::getClubSqlPath());
-      $this->queryBuilder
-        ->andWhere(
-          $this->queryBuilder->expr()->in($clubJoin, ":clubs"),
-        )
-        ->setParameter("clubs", $userClubs);
-    }
-
-
-    // TODO: Do we restrict also based on the ROLE ? Or Voter will do the final perm check
-    // $resourceClassName = explode("\\", $resourceClass);
-    // $resourceClassName = $resourceClassName[count($resourceClassName) - 1];
-    // $methodName = "restrict" . $resourceClassName . "As";
+      $hasClubLinked = false;
+      foreach ($this->queryBuilder->getDQLPart("join") as $dqlJoin) {
+        /** @var Join $join */
+        foreach ($dqlJoin as $join) {
+          if (str_ends_with($join->getJoin(), "club")) {
+            $hasClubLinked = true;
+            break 2;
+          }
+        }
+      }
 
+      if (!$hasClubLinked) {
+        $clubJoin = $this->addJoins($resourceClass::getClubSqlPath());
+        $this->queryBuilder
+          ->andWhere(
+            $this->queryBuilder->expr()->in($clubJoin, ":clubs"),
+          )
+          ->setParameter("clubs", $userClubs);
+      }
+    }
   }
 }
diff --git a/tests/AbstractTestCase.php b/tests/AbstractTestCase.php
@@ -158,10 +158,10 @@ public function makeAllLoggedRequests(
     ResponseCodeEnum $memberClub1Code = ResponseCodeEnum::forbidden,
     ResponseCodeEnum $supervisorClub1Code = ResponseCodeEnum::ok,
     ResponseCodeEnum $adminClub1Code  = ResponseCodeEnum::ok,
-    ?ResponseCodeEnum $adminClub2Code = ResponseCodeEnum::not_found,
+    ?ResponseCodeEnum $adminClub2Code = ResponseCodeEnum::forbidden,
     ResponseCodeEnum $superAdminCode = ResponseCodeEnum::ok,
     ResponseCodeEnum $badgerClub1Code = ResponseCodeEnum::forbidden,
-    ?ResponseCodeEnum $badgerClub2Code = ResponseCodeEnum::not_found,
+    ?ResponseCodeEnum $badgerClub2Code = ResponseCodeEnum::forbidden,
     ?\Closure $requestFunction = null,
   ): void {
     // Super admin

diff --git a/tests/Entity/ClubDependent/MemberSeasonTest.php b/tests/Entity/ClubDependent/MemberSeasonTest.php
@@ -56,9 +56,7 @@ public function testCreate(): void {
       $payloadCheck,
       supervisorClub1Code: ResponseCodeEnum::created,
       adminClub1Code: ResponseCodeEnum::created,
-      adminClub2Code: ResponseCodeEnum::bad_request,
       superAdminCode: ResponseCodeEnum::created,
-      badgerClub2Code: ResponseCodeEnum::bad_request,
       requestFunction: function (string $level, ?int $id) use ($club1, &$payloadCheck, &$count) {
         // We update for each test since the name must be unique
         $sn = "season_" . $count;

diff --git a/tests/Entity/ClubDependent/MemberTest.php b/tests/Entity/ClubDependent/MemberTest.php
@@ -380,7 +380,7 @@ public function testSelfMember(): void {
 
     // We can't read other member datas
     $this->makeGetRequest($this->getIriFromResource($member2));
-    $this->assertResponseStatusCodeSame(ResponseCodeEnum::not_found->value);
+    $this->assertResponseStatusCodeSame(ResponseCodeEnum::forbidden->value);
   }
 
   public function testMemberLinkWithUser(): void {

diff --git a/tests/Entity/ClubDependent/Plugin/Presence/ExternalPresenceTest.php b/tests/Entity/ClubDependent/Plugin/Presence/ExternalPresenceTest.php
@@ -99,10 +99,8 @@ public function testDelete(): void {
       $payloadCheck,
       supervisorClub1Code: ResponseCodeEnum::no_content,
       adminClub1Code: ResponseCodeEnum::no_content,
-      adminClub2Code: ResponseCodeEnum::not_found,
       superAdminCode: ResponseCodeEnum::no_content,
       badgerClub1Code: ResponseCodeEnum::no_content,
-      badgerClub2Code: ResponseCodeEnum::not_found,
       requestFunction: function (string $level, ?int $id) use (&$payloadCheck) {
         $presence = ExternalPresenceFactory::createOne();
         $this->makeDeleteRequest($this->getIriFromResource($presence));

diff --git a/tests/Entity/ClubDependent/Plugin/Presence/MemberPresenceTest.php b/tests/Entity/ClubDependent/Plugin/Presence/MemberPresenceTest.php
@@ -70,10 +70,8 @@ public function testCreate(): void {
       $payloadCheck,
       supervisorClub1Code: ResponseCodeEnum::created,
       adminClub1Code: ResponseCodeEnum::created,
-      adminClub2Code: ResponseCodeEnum::bad_request,
       superAdminCode: ResponseCodeEnum::created,
       badgerClub1Code: ResponseCodeEnum::created,
-      badgerClub2Code: ResponseCodeEnum::bad_request,
       requestFunction: function (string $level, ?int $id) use (&$payloadCheck) {
         $club1 = _InitStory::club_1();
 
@@ -126,10 +124,8 @@ public function testDelete(): void {
       $payloadCheck,
       supervisorClub1Code: ResponseCodeEnum::no_content,
       adminClub1Code: ResponseCodeEnum::no_content,
-      adminClub2Code: ResponseCodeEnum::not_found,
       superAdminCode: ResponseCodeEnum::no_content,
       badgerClub1Code: ResponseCodeEnum::no_content,
-      badgerClub2Code: ResponseCodeEnum::not_found,
       requestFunction: function (string $level, ?int $id) use (&$payloadCheck) {
         $memberPresence = MemberPresenceFactory::createOne([
           'club' => _InitStory::club_1(),

diff --git a/tests/Entity/ClubDependent/Plugin/Sale/InventoryCategoryTest.php b/tests/Entity/ClubDependent/Plugin/Sale/InventoryCategoryTest.php
@@ -33,10 +33,7 @@ public function testCreate(): void {
       $payloadCheck,
       supervisorClub1Code: ResponseCodeEnum::forbidden,
       adminClub1Code: ResponseCodeEnum::created,
-      adminClub2Code: ResponseCodeEnum::forbidden,
       superAdminCode: ResponseCodeEnum::created,
-      badgerClub1Code: ResponseCodeEnum::forbidden,
-      badgerClub2Code: ResponseCodeEnum::forbidden,
       requestFunction: function (string $level, ?int $id) use (&$payloadCheck) {
         $club1 = _InitStory::club_1();
         $payload = [
@@ -70,10 +67,7 @@ public function testDelete(): void {
     $this->makeAllLoggedRequests(
       supervisorClub1Code: ResponseCodeEnum::forbidden,
       adminClub1Code: ResponseCodeEnum::no_content,
-      adminClub2Code: ResponseCodeEnum::not_found,
       superAdminCode: ResponseCodeEnum::no_content,
-      badgerClub1Code: ResponseCodeEnum::forbidden,
-      badgerClub2Code: ResponseCodeEnum::not_found,
       requestFunction: function (string $level, ?int $id) {
         $item = InventoryCategoryFactory::createOne();
         $this->makeDeleteRequest($this->getIriFromResource($item));

diff --git a/tests/Entity/ClubDependent/Plugin/Sale/InventoryItemTest.php b/tests/Entity/ClubDependent/Plugin/Sale/InventoryItemTest.php
@@ -72,10 +72,7 @@ public function testDelete(): void {
     $this->makeAllLoggedRequests(
       supervisorClub1Code: ResponseCodeEnum::forbidden,
       adminClub1Code: ResponseCodeEnum::no_content,
-      adminClub2Code: ResponseCodeEnum::not_found,
       superAdminCode: ResponseCodeEnum::no_content,
-      badgerClub1Code: ResponseCodeEnum::forbidden,
-      badgerClub2Code: ResponseCodeEnum::not_found,
       requestFunction: function (string $level, ?int $id) {
         $item = InventoryItemFactory::createOne();
         $this->makeDeleteRequest($this->getIriFromResource($item));

diff --git a/tests/Entity/ClubDependent/Plugin/Sale/SalePaymentModeTest.php b/tests/Entity/ClubDependent/Plugin/Sale/SalePaymentModeTest.php
@@ -72,10 +72,7 @@ public function testDelete(): void {
     $this->makeAllLoggedRequests(
       supervisorClub1Code: ResponseCodeEnum::forbidden,
       adminClub1Code: ResponseCodeEnum::no_content,
-      adminClub2Code: ResponseCodeEnum::not_found,
       superAdminCode: ResponseCodeEnum::no_content,
-      badgerClub1Code: ResponseCodeEnum::forbidden,
-      badgerClub2Code: ResponseCodeEnum::not_found,
       requestFunction: function (string $level, ?int $id) {
         $item = SalePaymentModeFactory::createOne();
         $this->makeDeleteRequest($this->getIriFromResource($item));

diff --git a/tests/Entity/ClubDependent/Plugin/Sale/SaleTest.php b/tests/Entity/ClubDependent/Plugin/Sale/SaleTest.php
@@ -103,10 +103,7 @@ public function testDelete(): void {
     $this->makeAllLoggedRequests(
       supervisorClub1Code: ResponseCodeEnum::no_content,
       adminClub1Code: ResponseCodeEnum::no_content,
-      adminClub2Code: ResponseCodeEnum::not_found,
       superAdminCode: ResponseCodeEnum::no_content,
-      badgerClub1Code: ResponseCodeEnum::forbidden,
-      badgerClub2Code: ResponseCodeEnum::not_found,
       requestFunction: function (string $level, ?int $id) {
         $item = SaleFactory::createOne(['createdAt' => new \DateTimeImmutable()]);
         $this->makeDeleteRequest($this->getIriFromResource($item));
@@ -117,10 +114,7 @@ public function testDelete(): void {
     $this->makeAllLoggedRequests(
       supervisorClub1Code: ResponseCodeEnum::forbidden,
       adminClub1Code: ResponseCodeEnum::no_content,
-      adminClub2Code: ResponseCodeEnum::not_found,
       superAdminCode: ResponseCodeEnum::no_content,
-      badgerClub1Code: ResponseCodeEnum::forbidden,
-      badgerClub2Code: ResponseCodeEnum::not_found,
       requestFunction: function (string $level, ?int $id) {
         $item = SaleFactory::createOne([
           'createdAt' => \DateTimeImmutable::createFromMutable(faker()->dateTimeBetween('-10 days', '-2 days'))

diff --git a/tests/Entity/ClubTest.php b/tests/Entity/ClubTest.php
@@ -56,6 +56,8 @@ public function testPatch(): void {
       $payload,
       supervisorClub1Code: ResponseCodeEnum::forbidden,
       adminClub1Code: ResponseCodeEnum::forbidden,
+      adminClub2Code: ResponseCodeEnum::not_found,
+      badgerClub2Code: ResponseCodeEnum::not_found,
       requestFunction: function (string $level, ?int $id) use ($iri, $payload) {
         $this->makePatchRequest($iri, $payload);
       }
@@ -68,8 +70,10 @@ public function testDelete(): void {
       memberClub1Code: ResponseCodeEnum::not_found,
       supervisorClub1Code: ResponseCodeEnum::not_found,
       adminClub1Code: ResponseCodeEnum::not_found,
+      adminClub2Code: ResponseCodeEnum::not_found,
       superAdminCode: ResponseCodeEnum::no_content,
       badgerClub1Code: ResponseCodeEnum::not_found,
+      badgerClub2Code: ResponseCodeEnum::not_found,
       requestFunction: function (string $level, ?int $id) {
         $club = ClubFactory::createOne([
           'name' => 'Club to delete',