From ab6a9d26bab9f021a46968b3aa2a9649abc69b37 Mon Sep 17 00:00:00 2001
From: pbarroca <pbarroca@afi-sa.fr>
Date: Mon, 17 Mar 2014 17:05:40 +0100
Subject: [PATCH] fix #12552 : cleaning empty links in record view

---
 library/Class/Album.php                       |  3 +-
 .../PseudoNotice/UnimarcVisitor.php           |  6 --
 library/Class/Notice.php                      | 73 +++++++++----------
 library/Class/NoticeUnimarc/Writer.php        |  4 +
 ...echercheControllerAlbumAudioRecordTest.php | 12 ++-
 .../Class/Indexation/PseudoNoticeTest.php     | 33 +++++++++
 6 files changed, 85 insertions(+), 46 deletions(-)

diff --git a/library/Class/Album.php b/library/Class/Album.php
index 7e75d40864d..7c6f5cb51aa 100644
--- a/library/Class/Album.php
+++ b/library/Class/Album.php
@@ -1283,7 +1283,8 @@ class Class_Album extends Storm_Model_Abstract {
 					|| !isset($unimarc_value['data']) 
 					|| !isset($unimarc_value['data']['x'])
 					|| !isset($unimarc_value['data']['a'])
-					|| $field !== $unimarc_value['data']['x'])
+					|| $field !== $unimarc_value['data']['x']
+					|| '' == trim($unimarc_value['data']['a']))
 				continue;
 
 			$values[] = $unimarc_value['data']['a'];
diff --git a/library/Class/Indexation/PseudoNotice/UnimarcVisitor.php b/library/Class/Indexation/PseudoNotice/UnimarcVisitor.php
index 69c7238face..6cec0b2ce51 100644
--- a/library/Class/Indexation/PseudoNotice/UnimarcVisitor.php
+++ b/library/Class/Indexation/PseudoNotice/UnimarcVisitor.php
@@ -99,12 +99,6 @@ class Class_Indexation_PseudoNotice_UnimarcVisitor extends Class_Indexation_Pseu
 	}
 
 
-	public function visitModelId($model_id) {
-		$this->unimarc->add_field('856', '1 ', 'b' . $model_id);		
-		return $this;
-	}
-
-
 	public function visitUrl($url) {
 		if (trim($url))
 			$this->unimarc->add_field('856', '  ', 'a' . $url);
diff --git a/library/Class/Notice.php b/library/Class/Notice.php
index ebfe85e4ce7..ec70b173e67 100644
--- a/library/Class/Notice.php
+++ b/library/Class/Notice.php
@@ -618,16 +618,16 @@ class Class_Notice extends Storm_Model_Abstract {
 // ----------------------------------------------------------------
 	public function getChampNotice($champ, $facettes = '') {
 		$ret = [];
-		// Si facettte
-		if (strPos("ADPMG", $champ) !== false) {
+
+		if (strpos('ADPMG', $champ) !== false) {
+			$codification = Class_Codification::getInstance();
 			$items = array_filter(explode(' ', trim($facettes)));
 			foreach ($items as $item) {
-				$type = $item[0];
-				if ($type != $champ)
+				if ($item[0] != $champ)
 					continue;
 
 				$ret[] = ['id' => substr($item, 1),
-									'libelle' => Class_Codification::getInstance()->getLibelleFacette($item), 
+									'libelle' => $codification->getLibelleFacette($item), 
 									'url' => ['controller' => 'recherche',
 														'action' => 'simple',
 														'code_rebond' => $item,
@@ -636,24 +636,23 @@ class Class_Notice extends Storm_Model_Abstract {
 														'serie' => null,
 														'page' => null]];
 			}
+			return $ret;
 		}
-		// Champ texte
-		else
-			{
-				switch ($champ) {
-				case "J": $ret = $this->getZonesTitre(); break;
-				case "E": $ret = $this->getEditeur(); break;
-				case "F": $ret = $this->getCentreInteret(); break;
-				case "C": $ret = $this->getCollection(true); break;
-				case "O": $ret = $this->getNotes(); break;
-				case "K": $ret = $this->getCollation(); break;
-				case "R": $ret = $this->getResume(); break;
-				case "8": $ret = $this->get856a(); break;
-				case "N": $ret = $this->getAnnee(); break;
-				case "I": $ret = (new Class_Isbn($this->getIsbnOrEan()))->getAll()['isbn10']; break;
-				case "9" : $ret = $this->isNouveaute() ? $this->_('Oui'): $this->_('Non'); break;
-				}
-			}
+
+		switch ($champ) {
+		case 'J': $ret = $this->getZonesTitre(); break;
+		case 'E': $ret = $this->getEditeur(); break;
+		case 'F': $ret = $this->getCentreInteret(); break;
+		case 'C': $ret = $this->getCollection(true); break;
+		case 'O': $ret = $this->getNotes(); break;
+		case 'K': $ret = $this->getCollation(); break;
+		case 'R': $ret = $this->getResume(); break;
+		case '8': $ret = $this->get856a(); break;
+		case 'N': $ret = $this->getAnnee(); break;
+		case 'I': $ret = (new Class_Isbn($this->getIsbnOrEan()))->getAll()['isbn10']; break;
+		case '9' : $ret = $this->isNouveaute() ? $this->_('Oui'): $this->_('Non'); break;
+		}
+
 		return $ret;
 	}
 
@@ -1563,42 +1562,40 @@ class Class_Notice extends Storm_Model_Abstract {
 		return $this;
 	}
 
-// ----------------------------------------------------------------
-// Champ 856$a et 856$u (liens internet)
-// ----------------------------------------------------------------
-	public function get856a()	{
-		$lien = array();
 
-	
+	/** liens internet */
+	public function get856a()	{
+		$lien = [];
 		$blocs = $this->get_subfield(856);
+
 		$result = [];
 		foreach ($blocs as $bloc) {
-
 			$datas = $this->decoupe_bloc_champ($bloc);
-			$link=null;
+			$link = null;
 			foreach ($datas as $data) {
 				if ($data['code']=='x') {
-					$link=null;
+					$link = null;
 					break;
 				}
 				if ($data['code']=='u')
-					$link=$data['valeur'];
+					$link = $data['valeur'];
 				
 				if ($data['code']=='a')
-					$link=$data['valeur'];
-			}
-			
+					$link = $data['valeur'];
+			}			
 			$result[]=$link;
 		}
 
+		$result = array_filter($result);
+
 		if (isset($result[0]))	{
 			// black list
-			$trav = fetchOne("select valeur from variables where clef='black_list_856'");
+			$trav = Class_CosmoVar::get('black_list_856');
 			if (trim($trav)) $black_list = explode(";", $trav);
 
 			// controle url pour target
-			$target = fetchOne("select valeur from variables where clef='url_site'");
-			$trav = explode("/", $target);
+			$target = Class_CosmoVar::get('url_site');
+			$trav = explode('/', $target);
 			$ctrl_target = array_pop($trav);
 			if (!trim($ctrl_target)) $ctrl_target = array_pop($trav);
 
diff --git a/library/Class/NoticeUnimarc/Writer.php b/library/Class/NoticeUnimarc/Writer.php
index e3bcbb7ecf5..4388d0d8214 100644
--- a/library/Class/NoticeUnimarc/Writer.php
+++ b/library/Class/NoticeUnimarc/Writer.php
@@ -906,6 +906,8 @@ class Class_NoticeUnimarc_Writer {
 
 			$params = [$value['field'], '1 '];
 			foreach ($value['data'] as $k => $v) {
+				if ('' == trim($v))
+					continue;
 				$params[] = $k;
 				$params[] = $v;
 			}
@@ -913,6 +915,8 @@ class Class_NoticeUnimarc_Writer {
 			return $this;
 		}
 
+		if ('' == trim($value))
+			return $this;
 		$field = explode('$', $key);
 		$this->add_field($field[0], '1 ', $field[1] . $value);
 
diff --git a/tests/application/modules/opac/controllers/RechercheControllerAlbumAudioRecordTest.php b/tests/application/modules/opac/controllers/RechercheControllerAlbumAudioRecordTest.php
index 6868fdef14d..c51d87fd9ad 100644
--- a/tests/application/modules/opac/controllers/RechercheControllerAlbumAudioRecordTest.php
+++ b/tests/application/modules/opac/controllers/RechercheControllerAlbumAudioRecordTest.php
@@ -33,6 +33,8 @@ abstract class RechercheControllerAlbumAudioRecordTestCase extends AbstractContr
 		Class_Notice::setTimeSource(new TimeSourceForTest('2014-01-19 09:00:00'));
 		Class_Exemplaire::beVolatile();
 		Class_CodifAuteur::beVolatile();
+		Class_CosmoVar::newInstanceWithId('url_site', ['valeur' => 'http://mabib.net']);
+		Class_CosmoVar::newInstanceWithId('black_list_856', ['valeur' => 'mabib']);
 		Class_CosmoVar::newInstanceWithId('unimarc_zone_titre',
 																			['valeur' => '200$a;200$e;200$d;200$i;327$a;464$a;461$t;464$t']);
 
@@ -62,6 +64,8 @@ abstract class RechercheControllerAlbumAudioRecordTestCase extends AbstractContr
 			->addRessource($this->fixture('Class_AlbumRessource',
 																		['id' => 4,
 																		 'fichier' => '502_05_the_prophecy.mp3']))
+			->addZone('856', ['a' => 'http://mabib.net/bib-numerique/notice/ido/1'])
+			->addZone('856', ['x' => 'external_uri'])
 			->assertSave();
 
 		$album->index();
@@ -229,7 +233,7 @@ class RechercheControllerAlbumAudioRecordViewDetailsTest extends RechercheContro
 
 	/** @test */
 	public function noUrlShouldBePresent() {
-		$this->assertNotXPath('//dt[contains(@class, "internet")]');
+		$this->assertNotXPath('//dt[contains(@class, "internet")]', $this->_response->getBody());
 	}
 
 	
@@ -239,6 +243,12 @@ class RechercheControllerAlbumAudioRecordViewDetailsTest extends RechercheContro
 																			'Geffen Records',
 																			$this->_response->getBody());
 	} 
+
+
+	/** @test */
+	public function shouldNotContainEmptyLink() {
+		$this->assertNotXPath('//a[@href="http://"]');
+	}
 }
 
 
diff --git a/tests/library/Class/Indexation/PseudoNoticeTest.php b/tests/library/Class/Indexation/PseudoNoticeTest.php
index 371d7f7c6ee..b3bdbacd205 100644
--- a/tests/library/Class/Indexation/PseudoNoticeTest.php
+++ b/tests/library/Class/Indexation/PseudoNoticeTest.php
@@ -27,6 +27,39 @@ abstract class Class_Indexation_PseudoNoticeTestCase extends Storm_Test_ModelTes
 }
 
 
+class Class_Indexation_PseudoNoticeAlbumTest extends Class_Indexation_PseudoNoticeTestCase {
+	public function setUp() {
+		parent::setUp();
+		$this->fixture('Class_Album', 
+									 ['id' => 896,
+										'notes' => 'a:3:{i:1;a:2:{s:5:"field";s:3:"856";s:4:"data";a:2:{s:1:"x";s:12:"external_uri";s:1:"a";N;}}s:5:"215$a";s:0:"";i:2;a:2:{s:5:"field";s:3:"701";s:4:"data";a:2:{s:1:"a";s:16:"Gustave Flaubert";i:4;s:0:"";}}}',
+										'id_origine' => '778997987'])
+			->index();
+		$this->_notice = Class_Notice::find(1);
+	}
+
+
+	/** @test */
+	public function noticeShouldNotHave856b() {
+		$this->assertEmpty($this->_notice->get_subfield('856', 'b'));
+	}
+
+
+	/** @test */
+	public function noticeShouldNotHaveEmpty856a() {
+		foreach ($this->_notice->get_subfield('856', 'a') as $value) {
+			$this->assertNotEmpty($value, $value);
+		}
+	}
+
+
+	/** @test */
+	public function noticeShouldHave856aWithPermalink() {
+		$this->assertContains('/bib-numerique/notice/ido/778997987', 
+													$this->_notice->get_subfield('856', 'a')[0]);
+	}
+}
+
 
 
 class Class_Indexation_PseudoNoticeSitothequeTest extends Class_Indexation_PseudoNoticeTestCase {
-- 
GitLab