From 709982ad8ca8b9d1a5173d2dc936263a92369ea9 Mon Sep 17 00:00:00 2001
From: Patrick Barroca <pbarroca@afi-sa.fr>
Date: Thu, 1 Jul 2021 09:04:40 +0200
Subject: [PATCH] hotline #132817 : fix dynamic facet dedup without id field

---
 VERSIONS_HOTLINE/132817                       |  1 +
 library/Class/CodifThesaurus.php              | 35 ++++++--
 library/Class/CodifThesaurus/Rules.php        | 50 ++++++------
 .../Record/BibliographicDynamicFacets.php     |  9 +--
 .../scenarios/Thesauri/ThesauriImportTest.php | 79 +++++++++++++++++++
 5 files changed, 136 insertions(+), 38 deletions(-)
 create mode 100644 VERSIONS_HOTLINE/132817
 create mode 100644 tests/scenarios/Thesauri/ThesauriImportTest.php

diff --git a/VERSIONS_HOTLINE/132817 b/VERSIONS_HOTLINE/132817
new file mode 100644
index 00000000000..a62bfda56d7
--- /dev/null
+++ b/VERSIONS_HOTLINE/132817
@@ -0,0 +1 @@
+ - ticket #132817 : Facettes dynamiques : Correction d'un mauvais dédoublonnage des facettes dynamiques définies sans champ identifiant
\ No newline at end of file
diff --git a/library/Class/CodifThesaurus.php b/library/Class/CodifThesaurus.php
index 89317a5615a..763c2dfdec1 100644
--- a/library/Class/CodifThesaurus.php
+++ b/library/Class/CodifThesaurus.php
@@ -728,20 +728,41 @@ class Class_CodifThesaurus extends Storm_Model_Abstract {
 
   public function newChildEntry() {
     $code = $this->getCode();
-    return Class_CodifThesaurus::getLoader()->newInstance(['code' => $code,
-                                                           'id_thesaurus' => Class_CodifThesaurus::getLoader()->findNextThesaurusChildId($code, $this->getIdThesaurus())]);
+    return Class_CodifThesaurus::getLoader()
+      ->newInstance(['code' => $code,
+                     'id_thesaurus' => (Class_CodifThesaurus::getLoader()
+                                        ->findNextThesaurusChildId($code, $this->getIdThesaurus()))]);
   }
 
 
   public function getOrCreateChild($id_origine, $label) {
+    if (!$id_origine)
+      return $this->_getOrCreateChildByLabel($label);
+
     if (!$entry = $this->getChild($id_origine))
       $entry = $this->newChildEntry();
 
     if ($label == $entry->getLibelle())
       return $entry;
 
-    $entry->updateAttributes(['id_origine' => $id_origine,
-                               'libelle' => $label]);
+    $entry->setIdOrigine($id_origine)
+          ->setLibelle($label);
+
+    return $entry->save() ? $entry : null;
+  }
+
+
+  protected function _getOrCreateChildByLabel($label) {
+    if (!$label)
+      return;
+
+    if ($entry = Class_CodifThesaurus::getLoader()->findFirstBy(['code' => $this->getCode(),
+                                                                 'libelle' => $label]))
+      return $entry;
+
+    $entry = $this->newChildEntry()
+                  ->setIdOrigine(strtoupper($label))
+                  ->setLibelle($label);
 
     return $entry->save() ? $entry : null;
   }
@@ -760,9 +781,9 @@ class Class_CodifThesaurus extends Storm_Model_Abstract {
     return Class_CodifThesaurus::getLoader()
       ->findFirstBy(['code' => $this->getCode(),
                      'id_origine' => mb_substr($id_origine,
-                            0,
-                            self::COLUMN_ORIGIN_SIZE,
-                            'UTF-8')]);
+                                               0,
+                                               self::COLUMN_ORIGIN_SIZE,
+                                               'UTF-8')]);
   }
 
 
diff --git a/library/Class/CodifThesaurus/Rules.php b/library/Class/CodifThesaurus/Rules.php
index c8832e21a87..f4dba2a67c9 100644
--- a/library/Class/CodifThesaurus/Rules.php
+++ b/library/Class/CodifThesaurus/Rules.php
@@ -116,44 +116,44 @@ class Class_CodifThesaurus_Rules extends Class_Entity {
    * @param $reader cosmogramme notice_unimarc
    */
   public function extractIdAndFields($reader) {
-
-    $label_field = trim($this->getLabelField());
-    $id_field = trim($this->getIdField());
-
-
-    if (!$label_field)
+    if (!$label_field = trim($this->getLabelField()))
       return [];
 
+    $id_field = trim($this->getIdField());
     $filter_field = $this->getFilterField();
 
-    $filter_subfield_in = function($block) {
+    $fields = $reader->cutBlockBySubfields($this->getZonePadded(),
+                                           [$label_field => 'label',
+                                            $id_field => 'id',
+                                            $filter_field => 'filter'],
+                                           [$this, 'filterSubfieldIn']);
 
-      $label = $this->truncateLabel($block->get('label'));
-      if (!$this->isValidLabel($label))
-        return false;
-      $filter_zone = $this->getZone();
-      $filter_field = $this->getFilterField();
-      $filter_value = $this->getFilterValue();
+    return array_filter(array_map([$this, 'buildAuthorityStructure'],
+                                  $fields));
+  }
 
-      if (!$filter_zone || (null === $filter_field) | !$filter_value)
-        return  true;
-      return  ($block->get('filter') == $filter_value);
 
-    };
+  public function filterSubfieldIn($block) {
+    $label = $this->truncateLabel($block->get('label'));
+    if (!$this->isValidLabel($label))
+      return false;
 
-    $fields = $reader->cutBlockBySubfields($this->getZonePadded(),
-                                           [$label_field =>'label',
-                                            $id_field =>'id',
-                                            $filter_field => 'filter'], $filter_subfield_in);
+    $filter_zone = $this->getZone();
+    $filter_field = $this->getFilterField();
+    $filter_value = $this->getFilterValue();
 
-      return array_filter(array_map([$this,'buildAuthorityStructure'],$fields));
+    if (!$filter_zone || (null === $filter_field) | !$filter_value)
+      return true;
+
+    return $block->get('filter') == $filter_value;
   }
 
+
   public function buildAuthorityStructure($block){
     $label = $this->truncateLabel($block->get('label'));
-    return ['id' => $block->get('id')
-            ? strtoupper($block->get('id'))
-            : strtoupper($label),
+    return ['id' => ($block->get('id')
+                     ? strtoupper($block->get('id'))
+                     : ''),
             'label' => $label];
   }
 
diff --git a/library/Class/Cosmogramme/Integration/Record/BibliographicDynamicFacets.php b/library/Class/Cosmogramme/Integration/Record/BibliographicDynamicFacets.php
index 23d2239266b..343fb854d45 100644
--- a/library/Class/Cosmogramme/Integration/Record/BibliographicDynamicFacets.php
+++ b/library/Class/Cosmogramme/Integration/Record/BibliographicDynamicFacets.php
@@ -67,13 +67,10 @@ class Class_Cosmogramme_Integration_Record_BibliographicDynamicFacets_Simple {
   protected function _getOrCreateChild($id_label) {
     $id = trim($id_label['id']);
     $label = trim($id_label['label']);
-    if (!$label)
-      return;
 
-    if (!$id)
-      $id = strtoupper($label);
-
-    return $this->_thesaurus->getOrCreateChild($id, $label);
+    return $label
+      ? $this->_thesaurus->getOrCreateChild($id, $label)
+      : null;
   }
 }
 
diff --git a/tests/scenarios/Thesauri/ThesauriImportTest.php b/tests/scenarios/Thesauri/ThesauriImportTest.php
new file mode 100644
index 00000000000..0094c6ad315
--- /dev/null
+++ b/tests/scenarios/Thesauri/ThesauriImportTest.php
@@ -0,0 +1,79 @@
+<?php
+/**
+ * Copyright (c) 2012-2021, Agence Française Informatique (AFI). All rights reserved.
+ *
+ * BOKEH is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU AFFERO GENERAL PUBLIC LICENSE as published by
+ * the Free Software Foundation.
+ *
+ * There are special exceptions to the terms and conditions of the AGPL as it
+ * is applied to this software (see README file).
+ *
+ * BOKEH is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU AFFERO GENERAL PUBLIC LICENSE for more details.
+ *
+ * You should have received a copy of the GNU AFFERO GENERAL PUBLIC LICENSE
+ * along with BOKEH; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301  USA
+ */
+
+
+class ThesauriImportWithExistingTest extends ModelTestCase {
+  protected $_thesaurus;
+
+  public function setUp() {
+    parent::setUp();
+    $this->fixture(Class_CodifThesaurus::class,
+                   ['id'                    => 1,
+                    'libelle'               => 'Domaine',
+                    'libelle_facette'       => 'Domaine',
+                    'id_thesaurus'          => 'DOMA',
+                    'id_origine'            => null,
+                    'code'                  => 'DOMA',
+                    'rule_list_zone'        => ['995'],
+                    'rule_list_label_field' => ['h']]);
+
+    $this->fixture(Class_CodifThesaurus::class,
+                   ['id'              => 2,
+                    'libelle'         => '/[CD-Partitions/] Chanson francophone',
+                    'libelle_facette' => '/[CD-Partitions/] Chanson francophone',
+                    'id_thesaurus'    => 'DOMA0018',
+                    'id_origine'      => '/[CD-PARTITIONS/] CH',
+                    'code'            => 'DOMA']);
+
+    $unimarc = (new Class_NoticeUnimarc_Fluent)
+      ->zoneWithChildren('995',
+                         ['h' => '/[CD-Partitions/] Chansons anglophones et autres'])
+      ->render();
+
+    $thesauri = (new Class_Cosmogramme_Integration_Record_BibliographicDynamicFacets)
+      ->thesauriOf((new Class_NoticeUnimarc)->setNotice($unimarc),
+                   $this->fixture(Class_IntBib::class, ['id' => 3]));
+
+    $this->_thesaurus = reset($thesauri);
+  }
+
+
+  /** @test */
+  public function thesaurusIdShouldBe3() {
+    $this->assertEquals(3, $this->_thesaurus->getId());
+  }
+
+
+  /** @test */
+  public function thesaurusLabelShouldContainsAnglophones() {
+    $this->assertContains('anglophones', $this->_thesaurus->getLibelle());
+  }
+
+
+  /** @test */
+  public function thesaurusIdOrigineShouldStillBeCdPratitionsCh() {
+    $this->assertEquals('/[CD-PARTITIONS/] CH',
+                        mb_substr($this->_thesaurus->getIdOrigine(),
+                                  0,
+                                  Class_CodifThesaurus::COLUMN_ORIGIN_SIZE,
+                                  'UTF-8'));
+  }
+}
-- 
GitLab