Drop group name uniqueness at a same level; closes #366

Change the query so it looks for unique groups at the same level only
Drop uniqueness constraint in DB
Check uniqueness on group creation/edition
Add tests

diff --git a/galette/install/scripts/mysql.sql b/galette/install/scripts/mysql.sql
index 6c355bec7f39e248049116201e65b9b8cf306f01..735fc2f84b17cca3ed58563ef2d109ac737db89b 100644 (file)
--- a/galette/install/scripts/mysql.sql
+++ b/galette/install/scripts/mysql.sql
@@ -242,7 +242,6 @@ CREATE TABLE galette_groups (
   creation_date datetime NOT NULL,
   parent_group int(10) DEFAULT NULL,
   PRIMARY KEY (id_group),
-  UNIQUE KEY `name` (group_name),
   FOREIGN KEY (parent_group) REFERENCES galette_groups (id_group) ON DELETE RESTRICT ON UPDATE CASCADE
 ) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci;
 

diff --git a/galette/install/scripts/pgsql.sql b/galette/install/scripts/pgsql.sql
index b3683a7372c411a356bd6435a68adc554639cb5d..ae1adc38c1170b6e4d3118778b8b9ce0ff658255 100644 (file)
--- a/galette/install/scripts/pgsql.sql
+++ b/galette/install/scripts/pgsql.sql
@@ -400,7 +400,7 @@ CREATE TABLE galette_mailing_history (
 DROP TABLE IF EXISTS galette_groups CASCADE;
 CREATE TABLE galette_groups (
   id_group integer DEFAULT nextval('galette_groups_id_seq'::text) NOT NULL,
-  group_name character varying(250) NOT NULL CONSTRAINT name UNIQUE,
+  group_name character varying(250) NOT NULL,
   creation_date timestamp NOT NULL,
   parent_group integer DEFAULT NULL REFERENCES galette_groups(id_group) ON DELETE RESTRICT ON UPDATE CASCADE,
   PRIMARY KEY (id_group)

diff --git a/galette/install/scripts/upgrade-to-0.96-mysql.sql b/galette/install/scripts/upgrade-to-0.96-mysql.sql
index 21b134e7c36d4df8521fc181fa9b92268ac4103e..77009d6ac556090613a93c74851544e15c172c06 100644 (file)
--- a/galette/install/scripts/upgrade-to-0.96-mysql.sql
+++ b/galette/install/scripts/upgrade-to-0.96-mysql.sql
@@ -43,4 +43,7 @@ ALTER TABLE galette_adherents ADD COLUMN num_adh varchar(255) DEFAULT NULL;
 ALTER TABLE  galette_searches DROP INDEX form;
 ALTER TABLE galette_searches DROP COLUMN parameters_sum;
 
+-- drop groups unique name constraint
+ALTER TABLE galette_groups DROP INDEX `name`;
+
 UPDATE galette_database SET version = 0.960;
\ No newline at end of file

diff --git a/galette/install/scripts/upgrade-to-0.96-pgsql.sql b/galette/install/scripts/upgrade-to-0.96-pgsql.sql
index d4fc0452994b73cc5857ea72aec672c389517282..6e8127430e67df5abe31c9392d3edc61e953a07d 100644 (file)
--- a/galette/install/scripts/upgrade-to-0.96-pgsql.sql
+++ b/galette/install/scripts/upgrade-to-0.96-pgsql.sql
@@ -51,4 +51,7 @@ ALTER TABLE galette_adherents ADD COLUMN num_adh character varying (255) DEFAULT
 DROP INDEX galette_searches_idx;
 ALTER TABLE galette_searches DROP COLUMN parameters_sum;
 
+-- drop groups unique name constraint
+ALTER TABLE galette_groups DROP CONSTRAINT name;
+
 UPDATE galette_database SET version = 0.960;
\ No newline at end of file

diff --git a/galette/lib/Galette/Entity/Group.php b/galette/lib/Galette/Entity/Group.php
index cfedcc315d0d8a06ddb1c1a2641b2e26cc744e34..5f59d60bcb900ec168e68085efb3542753872599 100644 (file)
--- a/galette/lib/Galette/Entity/Group.php
+++ b/galette/lib/Galette/Entity/Group.php
@@ -36,6 +36,7 @@
 
 namespace Galette\Entity;
 
+use Galette\Repository\Groups;
 use Throwable;
 use Galette\Core\Login;
 use Analog\Analog;
@@ -407,6 +408,20 @@ class Group
     {
         global $zdb, $hist;
 
+        $parent_group = null;
+        if ($this->parent_group) {
+            $parent_group = $this->parent_group->getId();
+        }
+        if (!Groups::isUnique($zdb, $this->getName(), $parent_group, $this->getId())) {
+            Analog::log(
+                'Group name is not unique at requested level',
+                Analog::WARNING
+            );
+            throw new \RuntimeException(
+                _T("The group name you have requested already exists in the database.")
+            );
+        }
+
         try {
             $values = array(
                 self::PK     => $this->id,
@@ -414,7 +429,7 @@ class Group
             );
 
             if ($this->parent_group) {
-                $values['parent_group'] = $this->parent_group->getId();
+                $values['parent_group'] = $parent_group;
             }
 
             if (!isset($this->id) || $this->id == '') {

diff --git a/galette/lib/Galette/Repository/Groups.php b/galette/lib/Galette/Repository/Groups.php
index ca42d46e9ec886001772f3aec4fd70b0f6c85407..3674c7fad19f97cb2aea98dfa6190fb0c49ebeb2 100644 (file)
--- a/galette/lib/Galette/Repository/Groups.php
+++ b/galette/lib/Galette/Repository/Groups.php
@@ -404,18 +404,30 @@ class Groups
     /**
      * Check if groupname is unique
      *
-     * @param Db     $zdb  Database instance
-     * @param string $name Requested name
+     * @param Db       $zdb     Database instance
+     * @param string   $name    Requested name
+     * @param int|null $parent  Parent groupe (defaults to null)
+     * @param int|null $current Current ID to be excluded (defaults to null)
      *
      * @return boolean
      */
-    public static function isUnique(Db $zdb, $name)
+    public static function isUnique(Db $zdb, string $name, int $parent = null, int $current = null)
     {
         try {
             $select = $zdb->select(Group::TABLE);
-            $select->columns(
-                array('group_name')
-            )->where(array('group_name' => $name));
+            $select->columns(['group_name'])
+                ->where(['group_name'    => $name]);
+
+            if ($parent === null) {
+                $select->where('parent_group IS NULL');
+            } else {
+                $select->where(['parent_group' => $parent]);
+            }
+
+            if ($current !== null) {
+                $select->where->notEqualTo(Group::PK, $current);
+            }
+
             $results = $zdb->execute($select);
             return !($results->count() > 0);
         } catch (Throwable $e) {

diff --git a/tests/Galette/Entity/tests/units/Group.php b/tests/Galette/Entity/tests/units/Group.php
index 1fe10346edfcd9f597250a50577389d21b8e07d0..3975c86e9268fcfb59095ae59ba301933f6a9237 100644 (file)
--- a/tests/Galette/Entity/tests/units/Group.php
+++ b/tests/Galette/Entity/tests/units/Group.php
@@ -174,6 +174,10 @@ class Group extends GaletteTestCase
         $this->boolean($group->store())->isTrue();
         $group_id = $group->getId();
 
+        //update without changes should be ok
+        $group = new \Galette\Entity\Group($group_id);
+        $this->boolean($group->store())->isTrue();
+
         //Adding another group with same name throws an exception
         $group = new \Galette\Entity\Group();
         $group->setLogin($this->login);
@@ -184,7 +188,25 @@ class Group extends GaletteTestCase
                     $group->setName('A group');
                     $this->boolean($group->store())->isFalse();
                 }
-            )->hasMessage('Duplicate entry');
+            )->hasMessage('The group name you have requested already exists in the database.');
+
+        //update with changes should be ok
+        $group = new \Galette\Entity\Group($group_id);
+        $group->setName('A group - edited');
+        $this->boolean($group->store())->isTrue();
+
+        //
+        $group = new \Galette\Entity\Group();
+        $group->setName('Unique one');
+        $this->boolean($group->store())->isTrue();
+        //editing using an existing name is not ok
+        $this
+            ->exception(
+                function () use ($group) {
+                    $group->setName('A group - edited');
+                    $this->boolean($group->store())->isFalse();
+                }
+            )->hasMessage('The group name you have requested already exists in the database.');
     }
 
     /**

diff --git a/tests/Galette/Repository/tests/units/Groups.php b/tests/Galette/Repository/tests/units/Groups.php
index a2bb6312c3a0a8dab716b97888eb2e35fd8430dc..84cc4159e242ab2a2e7f6357b6aa43f99b22c0f0 100644 (file)
--- a/tests/Galette/Repository/tests/units/Groups.php
+++ b/tests/Galette/Repository/tests/units/Groups.php
@@ -219,4 +219,40 @@ class Groups extends GaletteTestCase
         $children_list = $groups->getList(true, $europe);
         $this->array($children_list)->hasSize(4);
     }
+
+    /**
+     * Test group name unicity
+     *
+     * @return void
+     */
+    public function testUnicity()
+    {
+        $group = new \Galette\Entity\Group();
+        $group->setLogin($this->login);
+        $unique_name = 'One group to rule them all';
+        $group->setName($unique_name);
+        $this->boolean($group->store())->isTrue();
+        $group_id = $group->getId();
+
+        $select = $this->zdb->select(\Galette\Entity\Group::TABLE);
+        $select->where(['group_name' => 'Europe']);
+        $result = $this->zdb->execute($select)->current();
+        $europe = $result->{\Galette\Entity\Group::PK};
+
+        $select = $this->zdb->select(\Galette\Entity\Group::TABLE);
+        $select->where(['group_name' => 'France']);
+        $result = $this->zdb->execute($select)->current();
+        $france = $result->{\Galette\Entity\Group::PK};
+
+        //name already exists - not unique
+        $this->boolean(\Galette\Repository\Groups::isUnique($this->zdb, $unique_name))->isFalse();
+        //name does not exist on another level - unique
+        $this->boolean(\Galette\Repository\Groups::isUnique($this->zdb, $unique_name, $europe))->isTrue();
+        //name is the current one - unique
+        $this->boolean(\Galette\Repository\Groups::isUnique($this->zdb, $unique_name, null, $group_id))->isTrue();
+
+        //tests on another level
+        $this->boolean(\Galette\Repository\Groups::isUnique($this->zdb, 'Nord', $france))->isFalse();
+        $this->boolean(\Galette\Repository\Groups::isUnique($this->zdb, 'Creuse', $france))->isTrue();
+    }
 }