From 6cf9c17e8b76909f8960f77c66698aef8b908e90 Mon Sep 17 00:00:00 2001
From: Henri-Damien LAURENT <hdlaurent@afi-sa.fr>
Date: Tue, 20 Sep 2022 12:07:10 +0200
Subject: [PATCH] hotline#163236 : Suggestions : Check Errors and notify

---
 VERSIONS_HOTLINE/163236                       |   1 +
 .../opac/controllers/AbonneController.php     |  15 +-
 .../Class/WebService/SIGB/Nanook/Service.php  |  15 +-
 library/Class/WebService/SIGB/Suggestion.php  |   3 +-
 ...nneControllerSuggestionAchatNanookTest.php | 152 +++++++++++++++++-
 tests/fixtures/NanookFixtures.php             |  13 ++
 6 files changed, 188 insertions(+), 11 deletions(-)
 create mode 100644 VERSIONS_HOTLINE/163236

diff --git a/VERSIONS_HOTLINE/163236 b/VERSIONS_HOTLINE/163236
new file mode 100644
index 00000000000..8faef160e80
--- /dev/null
+++ b/VERSIONS_HOTLINE/163236
@@ -0,0 +1 @@
+ - correctif #163236 : Améliorer la gestion des erreurs webservice pour la création d'une suggestion d'achat
\ No newline at end of file
diff --git a/application/modules/opac/controllers/AbonneController.php b/application/modules/opac/controllers/AbonneController.php
index c0b145a1aac..9078a203ef0 100644
--- a/application/modules/opac/controllers/AbonneController.php
+++ b/application/modules/opac/controllers/AbonneController.php
@@ -998,18 +998,27 @@ class AbonneController extends ZendAfi_Controller_Action {
     if (!$this->_getConfirmation($suggestion))
       return;
 
-    // $suggestion->save(); always return true without considering webservice answers. UX choice ?
-    $suggestion->save();
+    $status = $suggestion->save();
+
+    if ( is_array($status) && ($error_message = $status['erreur'])){
+      $this->_helper->notify($error_message,['statut' => 'error']);
+      return $this->_redirectSuggestion();
+    }
+
 
     try {
       $suggestion->sendMail('noreply@'.$this->_request->getHttpHost());
     } catch (Zend_Mail_Exception $e) {
       $this->_helper->notify($this->_('Aucun courriel envoyé, erreur: %s',
-                                      $this->_($e->getMessage())));
+                                      $e->getMessage()));
     }
 
     $this->_helper->notify($this->_('Suggestion d\'achat enregistrée'));
+    return $this->_redirectSuggestion();
+  }
+
 
+  protected function _redirectSuggestion(){
     $action = Class_Template::current()->isLegacy()
       ? 'suggestion-achat'
       : 'suggestions';
diff --git a/library/Class/WebService/SIGB/Nanook/Service.php b/library/Class/WebService/SIGB/Nanook/Service.php
index 2d7aa47e5aa..fb98f2ee627 100644
--- a/library/Class/WebService/SIGB/Nanook/Service.php
+++ b/library/Class/WebService/SIGB/Nanook/Service.php
@@ -220,9 +220,6 @@ class Class_Webservice_SIGB_Nanook_Service extends Class_WebService_SIGB_Abstrac
   // Class_Suggestion->save() always return true without considering webservice answers.
   // Looks like an user experience choice ? So no return statement is needed ...
   public function suggest($suggestion) {
-    $url = $this->buildQueryURL(['service' => 'CreateSuggest',
-                                 'patronId' => $suggestion->getUser()->getIdSigb()]);
-
     $doctype_label = Class_TypeDoc::find($suggestion->getDocType())->getLibelle();
 
     $post_data = ['site' => (($user = Class_Users::getIdentity()) ? $user->getLibraryId() : 0),
@@ -232,7 +229,17 @@ class Class_Webservice_SIGB_Nanook_Service extends Class_WebService_SIGB_Abstrac
                   'desclink' => $suggestion->getUrl(),
                   'comment' => $suggestion->getComment()];
 
-    $xml = $this->getWebClient()->postData($url, $post_data);
+    $xml = $this->httpPost(['service' => 'CreateSuggest',
+                            'patronId' => $suggestion->getUser()->getIdSigb()],
+                           $post_data);
+
+    try {
+      new SimpleXMLElement($xml);
+    } catch (Exception $e) {
+      return $this->_error($this->_('Suggestion non ajoutée dans le système d\'information de la médiathèque.\nImpossible de lire la réponse XML : %s', $e->getMessage()));
+    }
+
+    return $this->ilsdiCheckXml($xml, 'error', $this->_('Le Webservice a retourné une erreur : Suggestion d\'achat non enregistrée'));
   }
 
 
diff --git a/library/Class/WebService/SIGB/Suggestion.php b/library/Class/WebService/SIGB/Suggestion.php
index 019634b98d5..29a8f9b1598 100644
--- a/library/Class/WebService/SIGB/Suggestion.php
+++ b/library/Class/WebService/SIGB/Suggestion.php
@@ -54,8 +54,7 @@ class Class_WebService_SIGB_Suggestion extends Class_Entity {
 
   public function save() {
     $user = $this->getUser();
-    $user->getSIGBComm()->suggest($this);
-    return true;
+    return $user->getSIGBComm()->suggest($this);
   }
 
 
diff --git a/tests/application/modules/opac/controllers/AbonneControllerSuggestionAchatNanookTest.php b/tests/application/modules/opac/controllers/AbonneControllerSuggestionAchatNanookTest.php
index 9a556718526..e247d0c548b 100644
--- a/tests/application/modules/opac/controllers/AbonneControllerSuggestionAchatNanookTest.php
+++ b/tests/application/modules/opac/controllers/AbonneControllerSuggestionAchatNanookTest.php
@@ -242,6 +242,7 @@ class AbonneControllerSuggestionAchatNanookAddTest extends AbstractAbonneControl
 
 
 
+
 class AbonneControllerSuggestionAchatNanookAddPostTest extends AbstractAbonneControllerSuggestionAchatNanookTestCase {
   protected $_storm_default_to_volatile = true;
 
@@ -276,6 +277,152 @@ class AbonneControllerSuggestionAchatNanookAddPostTest extends AbstractAbonneCon
 
 
 
+
+class AbonneControllerSuggestionAchatNanookAddPostWithUrlTest extends AbstractAbonneControllerSuggestionAchatNanookTestCase {
+  public function setUp() {
+    parent::setUp();
+
+    $mock_transport = new MockMailTransport();
+    Zend_Mail::setDefaultTransport($mock_transport);
+    $this->mock_web_client
+    ->whenCalled('postData')
+      ->with('http://nanookService/service/CreateSuggest/patronId/187',
+             ['site' => '12',
+              'title' => '[Livres] fu',
+              'author' => 'jq',
+              'isbnean' => '2-07-0541 27_4',
+              'desclink' => 'http://www.google.fr',
+              'comment' => 'no'])
+    ->answers(NanookFixtures::createSuggestFuSucces());
+
+    $this->postDispatch('/opac/abonne/suggestion-achat-add',
+                        ['DocType' => Class_TypeDoc::LIVRE,
+                         'Site' => '12',
+                         'Title' => 'fu',
+                         'Author' => 'jq',
+                         'Isbn' => '2-07-0541 27_4',
+                         'Url' => 'http://www.google.fr',
+                         'Comment' => 'no']);
+  }
+
+
+  /** @test */
+  public function suggestionShouldHaveBeenSend() {
+    $this->assertFlashMessengerContains(['notification' => ['message' => 'Suggestion d\'achat enregistrée']]);
+  }
+
+
+  /** @test */
+  public function shouldRedirect() {
+    $this->assertRedirect();
+  }
+}
+
+
+
+
+class AbonneControllerSuggestionAchatNanookAddErrorXMLTest extends AbstractAbonneControllerSuggestionAchatNanookTestCase {
+  public function setUp() {
+    parent::setUp();
+
+    $this->mockmailtransport = new MockMailTransport();
+    Zend_Mail::setDefaultTransport($this->mockmailtransport);
+    $this->mock_web_client
+    ->whenCalled('postData')
+      ->with('http://nanookService/service/CreateSuggest/patronId/187',
+             ['site' => '12',
+              'title' => '[Livres] fu',
+              'author' => 'jq',
+              'isbnean' => '2-07-0541 27_4',
+              'desclink' => 'http://www.google.fr',
+              'comment' => 'no'])
+    ->answers(NanookFixtures::createSuggestError());
+
+    $this->postDispatch('/opac/abonne/suggestion-achat-add',
+                        ['DocType' => Class_TypeDoc::LIVRE,
+                         'Site' => '12',
+                         'Title' => 'fu',
+                         'Author' => 'jq',
+                         'Isbn' => '2-07-0541 27_4',
+                         'Url' => 'http://www.google.fr',
+                         'Comment' => 'no']);
+  }
+
+
+  /** @test */
+  public function webserviceErrorShouldBeNotified() {
+    $this->assertFlashMessengerContains(['notification' => ['message' => 'Le Webservice a retourné une erreur : Suggestion d\'achat non enregistrée', 'statut' => 'error'] ]);
+  }
+
+
+  /** @test */
+  public function onXmlErrorActionshouldNotHaveSentEmail() {
+    $this->assertNull( $this->mockmailtransport->sent_mail);
+  }
+
+
+  /** @test */
+  public function shouldRedirect() {
+    $this->assertRedirect();
+  }
+}
+
+
+
+
+class AbonneControllerSuggestionAchatNanookAddErrorHTMLTest extends AbstractAbonneControllerSuggestionAchatNanookTestCase {
+  public function setUp() {
+    parent::setUp();
+
+    $this->mockmailtransport = new MockMailTransport();
+    Zend_Mail::setDefaultTransport($this->mockmailtransport);
+    $this->mock_web_client
+    ->whenCalled('postData')
+      ->with('http://nanookService/service/CreateSuggest/patronId/187',
+             ['site' => '12',
+              'title' => '[Livres] fu',
+              'author' => 'jq',
+              'isbnean' => '2-07-0541 27_4',
+              'desclink' => 'http://www.google.fr',
+              'comment' => 'no'])
+    ->answers(NanookFixtures::createSuggestHTMLError());
+
+    $this->postDispatch('/opac/abonne/suggestion-achat-add',
+                        ['DocType' => Class_TypeDoc::LIVRE,
+                         'Site' => '12',
+                         'Title' => 'fu',
+                         'Author' => 'jq',
+                         'Isbn' => '2-07-0541 27_4',
+                         'Url' => 'http://www.google.fr',
+                         'Comment' => 'no']);
+  }
+
+
+  /** @test */
+  public function htmlErrorShouldBeNotified() {
+    $this->assertFlashMessengerContains(['notification' =>
+                                         ['message' => 'Service indisponible',
+                                          'statut' => 'error'
+                                         ]
+                                         ]);
+  }
+
+
+  /** @test */
+  public function onErrorActionshouldNotHaveSentEmail() {
+    $this->assertNull( $this->mockmailtransport->sent_mail);
+  }
+
+
+  /** @test */
+  public function shouldRedirect() {
+    $this->assertRedirect();
+  }
+}
+
+
+
+
 class AbonneControllerSuggestionAchatNanookAddPostErrorsTest extends AbstractAbonneControllerSuggestionAchatNanookTestCase {
   protected $_storm_default_to_volatile = true;
 
@@ -296,8 +443,9 @@ class AbonneControllerSuggestionAchatNanookAddPostErrorsTest extends AbstractAbo
 
 
   /** @test */
-  public function suggestionShouldHaveBeenSend() {
-    $this->assertFlashMessengerContains(['notification' => ['message' => 'Suggestion d\'achat enregistrée']]);
+  public function suggestionNotificationShouldDisplayErrorMessage() {
+    $this->assertFlashMessengerContains(['notification' => ['message' => 'Le Webservice a retourné une erreur : Suggestion d\'achat non enregistrée',
+                                                            'statut'=> 'error']]);
   }
 
 
diff --git a/tests/fixtures/NanookFixtures.php b/tests/fixtures/NanookFixtures.php
index 722b59f86fd..3cfa3daf5f2 100644
--- a/tests/fixtures/NanookFixtures.php
+++ b/tests/fixtures/NanookFixtures.php
@@ -541,6 +541,19 @@ class NanookFixtures {
   }
 
 
+  public static function createSuggestError(){
+    return '<?xml version="1.0" encoding="UTF-8"?>
+            <CreateSuggest>
+              <error>Cannot Create</error>
+            </CreateSuggest>';
+  }
+
+
+  public static function createSuggestHTMLError(){
+    return '<html><body><title>Ma zo Page</title></body></html>';
+  }
+
+
   public static function createSuggestFuSucces() {
     return '    <getpatroninfo>
         <patronid>
-- 
GitLab