From 4fbe30835a1f17cb47faa38cb5e68c2f8a3e952d Mon Sep 17 00:00:00 2001
From: llaffont <llaffont@afi-sa.fr>
Date: Sat, 6 Feb 2016 13:10:24 +0100
Subject: [PATCH] dev #37100 better performances for
 Class_Users::canModifyPanier

---
 library/Class/PanierNotice.php                |  10 +-
 library/Class/Users.php                       |  21 +--
 .../controllers/CatalogueControllerTest.php   |   3 +
 .../opac/controllers/PanierControllerTest.php | 173 +++++++-----------
 tests/library/Class/PanierNoticeTest.php      |   4 +
 5 files changed, 86 insertions(+), 125 deletions(-)

diff --git a/library/Class/PanierNotice.php b/library/Class/PanierNotice.php
index 5ff30caa41d..267a54fb9c9 100644
--- a/library/Class/PanierNotice.php
+++ b/library/Class/PanierNotice.php
@@ -47,13 +47,17 @@ class PanierNoticeLoader extends Storm_Model_Loader {
   }
 
 
-  public function findAllWithCatalogue() {
-    $ids = array_filter(
+  public function findAllIdsWithCatalogue() {
+    return array_filter(
                         array_unique(
                                      array_map(
                                                function($association) {return $association->getIdPanier();},
                                                Class_PanierNoticeCatalogue::findAll())));
-    return Class_PanierNotice::findAllBy(['id' => $ids]);
+  }
+
+
+  public function findAllWithCatalogue() {
+    return Class_PanierNotice::findAllBy(['id' => $this->findAllIdsWithCatalogue()]);
   }
 
 
diff --git a/library/Class/Users.php b/library/Class/Users.php
index 85121d6200c..2de4251b982 100644
--- a/library/Class/Users.php
+++ b/library/Class/Users.php
@@ -1439,23 +1439,18 @@ class Class_Users extends Storm_Model_Abstract {
   }
 
 
-  public function getEditablePaniers() {
-    $user_paniers  = $this->getPaniers();
-    if ($this->canAccessBackend())
-      $user_paniers = array_filter(array_unique(array_merge($user_paniers,
-                                                            Class_PanierNotice::findAllWithCatalogue())));
-    Class_PanierNotice::sortPaniersByLibelle($user_paniers);
-    return $user_paniers;
-  }
-
-
   public function canModifyPanier($panier) {
     if (!$panier || $panier->isNew())
       return true;
 
-    $ids_paniers = array_map(function($panier) {return $panier->getId();},
-                             $this->getEditablePaniers());
-    return in_array($panier->getId(), $ids_paniers);
+    $user_paniers_ids = (new Storm_Model_Collection($this->getPaniers()))->collect('id');
+    if (in_array($panier->getId(), $user_paniers_ids->getArrayCopy()))
+      return true;
+
+    if (!$this->canAccessBackend())
+      return false;
+
+    return in_array($panier->getId(), Class_PanierNotice::findAllIdsWithCatalogue());
   }
 
 
diff --git a/tests/application/modules/admin/controllers/CatalogueControllerTest.php b/tests/application/modules/admin/controllers/CatalogueControllerTest.php
index 065ace0fd40..48dd4c04870 100644
--- a/tests/application/modules/admin/controllers/CatalogueControllerTest.php
+++ b/tests/application/modules/admin/controllers/CatalogueControllerTest.php
@@ -1467,6 +1467,9 @@ class CatalogueControllerPaniersRemovePanierAndUnindexTest extends AbstractContr
   public function setUp() {
     parent::setUp();
 
+    Storm_Test_ObjectWrapper::onLoaderOfModel('Class_Catalogue')
+      ->whenCalled('saveThesaurus')->answers(true);
+
     $pomme = $this->fixture('Class_Notice',
                             ['id' => 1456,
                              'clef_alpha' => 'POMME',
diff --git a/tests/application/modules/opac/controllers/PanierControllerTest.php b/tests/application/modules/opac/controllers/PanierControllerTest.php
index 63e2c5738d0..80d6e0f7f4e 100644
--- a/tests/application/modules/opac/controllers/PanierControllerTest.php
+++ b/tests/application/modules/opac/controllers/PanierControllerTest.php
@@ -52,14 +52,11 @@ abstract class PanierControllerSimpleLoggedUserTestCase extends AbstractControll
 
 
 abstract class PanierControllerTestCase extends AbstractControllerTestCase {
+  protected
+    $_storm_default_to_volatile = true;
+
   public function setUp() {
     parent::setUp();
-    Class_Users::beVolatile();
-    Class_NoticeDomain::beVolatile();
-    Class_UserGroup::beVolatile();
-    Class_Notice::beVolatile();
-    Class_PanierNotice::beVolatile();
-    Class_PanierNoticeCatalogue::beVolatile();
 
     $this->manon = $this->fixture('Class_Users', [
       'id' => 23,
@@ -120,39 +117,41 @@ abstract class PanierControllerTestCase extends AbstractControllerTestCase {
               'order' => 'FIELD(clef_alpha, "MONTESPAN")'])
       ->answers([$montespan]);
 
-    $this->panier_loader = Storm_Test_ObjectWrapper::onLoaderOfModel('Class_PanierNotice')
-      ->whenCalled('save')->answers(true)
-      ->whenCalled('delete')->answers(true)
-
-      ->whenCalled('findAll')->answers([$this->panier_bd, $this->panier_romans])
 
-      ->whenCalled('findAllBy')
-      ->with(['role' => 'user', 'model' => $this->manon])
-      ->answers([$this->panier_bd, $this->panier_romans]);
-
-    $panier_domaine_histoire = Class_PanierNoticeCatalogue::newInstanceWithId(71);
+    $panier_domaine_histoire = $this->fixture('Class_PanierNoticeCatalogue',
+                                              ['id' => 71]);
     $panier_domaine_histoire
-      ->setPanierNotice(Class_PanierNotice::newInstanceWithId(38,
-                                                              ['libelle' => 'selection jeunesse',
-                                                               'user' => Class_Users::newInstanceWithId(45, ['nom' => 'Dupont']),
-                                                               'date_maj' => '19/01/2013',
-                                                               'panier_notice_catalogues' => [$panier_domaine_histoire]]))
-      ->setCatalogue(Class_Catalogue::newInstanceWithId(97, ['libelle' => 'histoire']))->save();
-
-
-    $panier_domaine_bd = Class_PanierNoticeCatalogue::newInstanceWithId(72);
+      ->setPanierNotice($this->fixture('Class_PanierNotice',
+                                       ['id' => 38,
+                                        'libelle' => 'selection jeunesse',
+                                        'user' => Class_Users::newInstanceWithId(45, ['nom' => 'Dupont']),
+                                        'date_maj' => '19/01/2013',
+                                        'panier_notice_catalogues' => [$panier_domaine_histoire]]))
+      ->setCatalogue($this->fixture('Class_Catalogue',
+                                    ['id' => 97,
+                                     'libelle' => 'histoire']))
+      ->save();
+
+
+    $panier_domaine_bd = $this->fixture('Class_PanierNoticeCatalogue',
+                                        ['id' => 72]);
     $panier_domaine_bd
       ->setPanierNotice($this->panier_bd)
-      ->setCatalogue(Class_Catalogue::newInstanceWithId(199,
-                                                        ['libelle' => 'bd',
-                                                         'panier_notice_catalogues' => [$panier_domaine_bd]]))->save();
-
-    $panier_domaine_orphelin = Class_PanierNoticeCatalogue::newInstanceWithId(456);
+      ->setCatalogue($this->fixture('Class_Catalogue',
+                                    ['id' => 199,
+                                     'libelle' => 'bd',
+                                     'panier_notice_catalogues' => [$panier_domaine_bd]]))
+      ->save();
+
+    $panier_domaine_orphelin = $this->fixture('Class_PanierNoticeCatalogue',
+                                              ['id' => 456]);
     $panier_domaine_orphelin
       ->setPanierNotice($this->panier_orphelin)
-      ->setCatalogue(Class_Catalogue::newInstanceWithId(1789,
-                                                        ['libelle' => 'orphelin',
-                                                         'panier_notice_catalogues' => [$panier_domaine_orphelin]]))->save();
+      ->setCatalogue($this->fixture('Class_Catalogue',
+                                    ['id' => 1789,
+                                     'libelle' => 'orphelin',
+                                     'panier_notice_catalogues' => [$panier_domaine_orphelin]]))
+      ->save();
 
   }
 }
@@ -442,6 +441,7 @@ class PanierControllerSupprimerNoticeBlacksadFromBDTest extends PanierController
   public function setUp() {
     parent::setUp();
     $this->dispatch('/panier/paniersupprimernotice/id_panier/2/id_notice/12', true);
+    Class_PanierNotice::clearCache();
   }
 
 
@@ -451,13 +451,6 @@ class PanierControllerSupprimerNoticeBlacksadFromBDTest extends PanierController
   }
 
 
-  /** @test */
-  public function panierShouldHaveBeenSaved() {
-    $this->assertTrue(Class_PanierNotice::methodHasBeenCalled('save'));
-  }
-
-
-  /** @test */
   public function responseShouldRedirectToIndex() {
     $this->assertRedirectTo('/opac/panier/index/id_panier/2');
   }
@@ -469,12 +462,13 @@ class PanierControllerSupprimerBDTest extends PanierControllerTestCase {
   public function setUp() {
     parent::setUp();
     $this->dispatch('/panier/supprimerpanier/id_panier/2', true);
+    Class_PanierNotice::clearCache();
   }
 
 
   /** @test */
   public function panierShouldHaveBeenDeleted() {
-    $this->assertTrue(Class_PanierNotice::methodHasBeenCalled('delete'));
+    $this->assertEmpty(Class_PanierNotice::find(2));
   }
 
 
@@ -493,14 +487,15 @@ class PanierControllerOtherUserSecurityTest extends PanierControllerTestCase {
     Class_PanierNotice::newInstanceWithId(39,
                                           ['libelle' => 'panier Markus',
                                            'user' => Class_Users::newInstanceWithId(98, ['nom' => 'Markus']),
-                                           'date_maj' => '19/01/2013']);
+                                           'date_maj' => '19/01/2013'])->assertSave();
   }
 
 
   /** @test */
   public function panierDomaineHistoireShouldNotBeDeletable() {
     $this->dispatch('/panier/supprimerpanier/id_panier/39', true);
-    $this->assertFalse(Class_PanierNotice::methodHasBeenCalled('delete'));
+    Class_PanierNotice::clearCache();
+    $this->assertNotEmpty(Class_PanierNotice::find(39));
   }
 
 
@@ -521,25 +516,34 @@ class PanierControllerOtherUserSecurityTest extends PanierControllerTestCase {
   /** @test */
   public function addPanierToMarkusShouldBeForbidden() {
     $this->dispatch('/panier/panierajouternotice/id_panier/39/id_notice/12', true);
-    $this->assertFalse(Class_PanierNotice::methodHasBeenCalled('save'));
+    Class_PanierNotice::clearCache();
+    $this->assertNotContains('BLACKSAD',
+                             Class_PanierNotice::find(39)->getClesNotices());
     $this->assertRedirectTo('/opac/panier');
   }
 
 
   /** @test */
-  public function addPanierAjaxToMarkusShouldBeForbidden() {
+  public function addPanierAjaxToMarkusByAbonneSIGBShouldBeForbidden() {
     $this->manon->setRoleLevel(ZendAfi_Acl_AdminControllerRoles::ABONNE_SIGB);
     ZendAfi_Auth::getInstance()->logUser($this->manon);
+
     $this->postDispatch('/panier/ajout-ajax/id_panier/39/id_notice/12', []);
-    $this->assertFalse(Class_PanierNotice::methodHasBeenCalled('save'));
+    $this->assertNotContains('BLACKSAD',
+                             Class_PanierNotice::find(39)->getClesNotices());
+
     $this->assertRedirectTo('/opac/panier');
   }
 
 
   /** @test */
   public function removeNoticeFromPanierMarkusShouldBeForbidden() {
+    Class_PanierNotice::find(39)->setClesNotices(['BLACKSAD'])->assertSave();
     $this->postDispatch('/panier/paniersupprimernotice/id_panier/39/id_notice/12', []);
-    $this->assertFalse(Class_PanierNotice::methodHasBeenCalled('save'));
+
+    Class_PanierNotice::clearCache();
+    $this->assertContains('BLACKSAD', Class_PanierNotice::find(39)->getClesNotices());
+
     $this->assertRedirectTo('/opac/panier');
   }
 
@@ -547,7 +551,8 @@ class PanierControllerOtherUserSecurityTest extends PanierControllerTestCase {
   /** @test */
   public function majTitrePanierMarkusShouldBeForbidden() {
     $this->postDispatch('/panier/majtitrepanier/id_panier/39/', ['new_libelle' => 'crack']);
-    $this->assertFalse(Class_PanierNotice::methodHasBeenCalled('save'));
+    Class_PanierNotice::clearCache();
+    $this->assertEquals('panier Markus', Class_PanierNotice::find(39)->getLibelle());
     $this->assertRedirectTo('/opac/panier');
   }
 }
@@ -617,17 +622,9 @@ class PanierControllerAjoutNoticeBlackSadToPanierMesRomansTest extends PanierCon
   public function setUp() {
     parent::setUp();
 
-    $this->panier_loader
-      ->whenCalled('maxIdPanierForUser')
-      ->answers(4);
-
     $this->dispatch('/panier/panierajouternotice/id_panier/15/id_notice/12', true);
-  }
-
-
-  /** @test */
-  public function panierShouldHaveBeenSaved() {
-    $this->assertTrue(Class_PanierNotice::methodHasBeenCalled('save'));
+    Class_PanierNotice::clearCache();
+    $this->panier_romans = Class_PanierNotice::find(15);
   }
 
 
@@ -651,10 +648,6 @@ class PanierControllerAjoutUnknownNoticeToPanierMesRomansTest extends PanierCont
   public function setUp() {
     parent::setUp();
 
-    $this->panier_loader
-      ->whenCalled('maxIdPanierForUser')
-      ->answers(4);
-
     $this->dispatch('/panier/panierajouternotice/id_panier/15/id_notice/-1', true);
   }
 
@@ -673,12 +666,7 @@ class PanierControllerAjoutNoticeBlackSadToPanierDomaineHistoireTest extends Pan
     parent::setUp();
     $this->manon->changeRoleTo(ZendAfi_Acl_AdminControllerRoles::MODO_PORTAIL);
     $this->dispatch('/panier/panierajouternotice/id_panier/38/id_notice/12', true);
-  }
-
-
-  /** @test */
-  public function panierShouldHaveBeenSaved() {
-    $this->assertTrue(Class_PanierNotice::methodHasBeenCalled('save'));
+    Class_PanierNotice::clearCache();
   }
 
 
@@ -698,14 +686,10 @@ class PanierControllerModifierTitrePanierMesRomansToMesLivresTest extends Panier
     $this->postDispatch('/panier/majtitrepanier/id_panier/15',
                         ['new_libelle' => 'Mes livres'],
                         true);
+    Class_PanierNotice::clearCache();
   }
 
 
-  /** @test */
-  public function panierShouldHaveBeenSaved() {
-    $this->assertTrue(Class_PanierNotice::methodHasBeenCalled('save'));
-  }
-
 
   /** @test */
   public function responseShouldRedirectToIndex() {
@@ -715,7 +699,7 @@ class PanierControllerModifierTitrePanierMesRomansToMesLivresTest extends Panier
 
   /** @test */
   public function panierLibelleShouldBeMesLivres() {
-    $this->assertEquals('Mes livres', $this->panier_romans->getLibelle());
+    $this->assertEquals('Mes livres', Class_PanierNotice::find(15)->getLibelle());
   }
 }
 
@@ -731,12 +715,8 @@ class PanierControllerModifierCataloguePanierTest extends PanierControllerTestCa
     $this->postDispatch('/panier/majtitrepanier/id_panier/15',
                         ['domaine_ids' => '97-199'],
                         true);
-  }
-
-
-  /** @test */
-  public function panierShouldHaveBeenSaved() {
-    $this->assertTrue(Class_PanierNotice::methodHasBeenCalled('save'));
+    Class_PanierNotice::clearCache();
+    $this->panier_romans = Class_PanierNotice::find(15);
   }
 
 
@@ -1041,34 +1021,15 @@ class PanierControllerCreerPanierPostTest extends PanierControllerTestCase {
     parent::setUp();
     $this->manon->setPaniers([]);
 
-    $lesBonLivres= (new Class_PanierNotice);
-    Storm_Test_ObjectWrapper::onLoaderOfModel('Class_PanierNotice')
-      ->whenCalled('save')
-      ->answers(true)
-      ->whenCalled('maxIdPanierForUser')
-      ->answers(1)
-      ->whenCalled('getId')
-      ->answers(1)
-      ->whenCalled('find')
-      ->answers(((new Class_PanierNotice())
-                 ->setUser($this->manon)
-                 ->setLibelle('Par ici les bons livres')
-                 ->setIdPanier(1)));
-
-
     $this->postDispatch('/panier/creer-panier',
                         ['titre'=>'Par ici les bons livres']);
   }
 
-  /** @test */
-  public function panierParIciLesBonsLivresShouldHaveBeenSaved() {
-    $this->assertTrue(Class_PanierNotice::methodHasBeenCalled('save'));
-  }
-
 
   /** @test */
   public function validFormShouldCreatePanierWithLabelleParIciLesBonsLivres() {
-    $this->assertEquals('Par ici les bons livres', Class_PanierNotice::find(1)->getLibelle());
+    $this->assertEquals('Par ici les bons livres',
+                        Class_PanierNotice::findFirstBy(['order' => 'id desc'])->getLibelle());
   }
 }
 
@@ -1084,7 +1045,6 @@ class PanierControllerCreationPanierSuccessTest extends PanierControllerTestCase
     $lesBonLivres= (new Class_PanierNotice);
     Storm_Test_ObjectWrapper::onLoaderOfModel('Class_PanierNotice')
       ->whenCalled('save')->answers(true)
-      ->whenCalled('maxIdPanierForUser')->answers(1)
       ->whenCalled('getId')->answers(1)
       ->whenCalled('find')->answers((new Class_PanierNotice())
                                     ->setUser($this->manon)
@@ -1194,13 +1154,8 @@ class PanierControllerSupprimerNoticeBlacksadFromBDAndRedirectToRefererTest exte
 
   /** @test */
   public function afterDeletePanierShouldContainsOnlyCombatOrdinaire() {
-    $this->assertEquals('COMBAT ORDINAIRE', $this->panier_bd->getNotices());
-  }
-
-
-  /** @test */
-  public function afterDeletePanierShouldHaveBeenSaved() {
-    $this->assertTrue(Class_PanierNotice::methodHasBeenCalled('save'));
+    Class_PanierNotice::clearCache();
+    $this->assertEquals('COMBAT ORDINAIRE', Class_PanierNotice::find(2)->getNotices());
   }
 
 
diff --git a/tests/library/Class/PanierNoticeTest.php b/tests/library/Class/PanierNoticeTest.php
index 4c12f0b26fb..679ce6ca0e3 100644
--- a/tests/library/Class/PanierNoticeTest.php
+++ b/tests/library/Class/PanierNoticeTest.php
@@ -422,6 +422,10 @@ class PanierNoticeWithWrongUserIdTest extends AbstractControllerTestCase {
 class PanierNoticeIndexAllTest extends ModelTestCase {
   public function setUp() {
     parent::setUp();
+
+    Storm_Test_ObjectWrapper::onLoaderOfModel('Class_Catalogue')
+      ->whenCalled('saveThesaurus')->answers(true);
+
     $this->fixture('Class_Notice', ['id' => 4,
                                     'titre_principal' => 'Le Montespan',
                                     'auteur_principal' => 'Jean Teulテδゥ',
-- 
GitLab