OPUSVIER-3541 Fixed notification form for publication.

OPUS4 · Nov 5, 2018 · bd5ebca · bd5ebca
1 parent e71a9b5
commit bd5ebca
Show file tree

Hide file tree

Showing 8 changed files with 137 additions and 70 deletions.
diff --git a/library/Application/Util/Notification.php b/library/Application/Util/Notification.php
@@ -48,6 +48,8 @@ public function __construct($logger = null, $config = null)
      * @param boolean $notifySubmitter Wenn false, wird der Submitter nicht notifiziert
      * @param array $notifyAuthors Bitmaske, die für jeden Autor (über den Index referenziert) angibt, ob ihm/ihr eine
      *                             E-Mail gesendet werden kann (wenn false, dann wird keine Notifizierung versendet)
+     *
+     * TODO this class should not collect recipients on its own -> recipients should be provided
      */
     public function prepareMail($document, $url, $notifySubmitter = true, $notifyAuthors = [])
     {
@@ -56,31 +58,76 @@ public function prepareMail($document, $url, $notifySubmitter = true, $notifyAut
         $logger->info("prepare notification email for document id " . $document->getId());
 
         $authorAddresses = [];
-        $authors = [];
+        $authors = $this->getAuthors($document);
+        $title = $document->getMainTitle();
 
-        $personAuthors = $document->getPersonAuthor();
-        if (!empty($personAuthors)) {
-            $index = 0;
-            foreach ($personAuthors as $author) {
-                // TODO Komma nur wenn FirstName present
-                $name = trim($author->getLastName() . ", " . $author->getFirstName());
-                array_push($authors, $name);
+        $this->scheduleNotification(
+            $this->getMailSubject($document->getId(), $authors, $title),
+            $this->getMailBody($document->getId(), $authors, $title, $url),
+            $this->getRecipients($authorAddresses, $document, $notifySubmitter)
+        );
 
-                $index++;
-            }
-        }
+        $logger->info("notification mail creation was completed successfully");
+    }
+
+    /**
+     * @param $document
+     * @param $url
+     * @param $recipients
+     *
+     * TODO this function is only used for PublicatioNotification at the moment - cleanup!
+     */
+    public function prepareMailFor($document, $url, $recipients)
+    {
+        $logger = $this->getLogger();
+
+        $logger->info("prepare notification email for document id " . $document->getId());
+
+        $authors = $this->getAuthors($document);
 
         $title = $document->getMainTitle();
 
+        // TODO currently we need to convert between the old and new array structure
+        // TODO the components and interfaces involved need to be defined clearly
+
+        $converted = [];
+
+        foreach ($recipients as $address => $recipient) {
+            $entry = [];
+            $entry['address'] = $address;
+
+            if (is_array($recipient['name'])) {
+                $entry['name'] = $recipient['name'][0]; // TODO only use name of first address occurence
+            } else {
+                $entry['name'] = $recipient['name'];
+            }
+        }
+
         $this->scheduleNotification(
             $this->getMailSubject($document->getId(), $authors, $title),
             $this->getMailBody($document->getId(), $authors, $title, $url),
-            $this->getRecipients($authorAddresses, $document, $notifySubmitter)
+            $converted
         );
 
         $logger->info("notification mail creation was completed successfully");
     }
 
+    public function getAuthors($document)
+    {
+        $authors = [];
+
+        $personAuthors = $document->getPersonAuthor();
+        if (!empty($personAuthors)) {
+            foreach ($personAuthors as $author) {
+                // TODO Komma nur wenn FirstName present
+                $name = trim($author->getLastName() . ", " . $author->getFirstName());
+                array_push($authors, $name);
+            }
+        }
+
+        return $authors;
+    }
+
     private function getMailSubject($docId, $authors, $title)
     {
         $logger = $this->getLogger();

diff --git a/library/Application/View/Partial/multicheckboxtable.phtml b/library/Application/View/Partial/multicheckboxtable.phtml
@@ -3,7 +3,10 @@
     <?PHP foreach ($this->element->getRows() as $row) : ?>
     <tr>
         <td><?= $row['type'] ?></td>
-        <td><input type="checkbox" value="<?= $row['value'] ?>" <?= ($row['disabled']) ? 'disabled="disabled"' : '' ?> /></td>
+        <td><input type="checkbox" value="1" <?= ($row['disabled']) ? 'disabled="disabled"' : '' ?>
+                   id="<?= $row['value'] ?>" name="<?= $row['value'] ?>"
+                   <?= ($row['checked'] == 1) ? 'checked="checked"' : '' ?> /></td>
+
         <td><?= $row['name'] ?></td>
         <td><?= $row['email'] ?></td>
     </tr>

diff --git a/modules/admin/controllers/WorkflowController.php b/modules/admin/controllers/WorkflowController.php
@@ -187,6 +187,7 @@ private function _changeState($document, $targetState, $form = null)
     private function _sendNotification($document, $form = null)
     {
         $notification = new Application_Util_PublicationNotification();
+
         $url = $this->view->url([
                 "module" => "frontdoor",
                 "controller" => "index",
@@ -197,24 +198,12 @@ private function _sendNotification($document, $form = null)
             true
         );
 
-        $authorsBitmask = [];
-        $notifySubmitter = true;
-
-        if (!is_null($form)) {
-            foreach ($form->getValues() as $key => $val) {
-                $pos = strpos($key, 'author_');
-                if ($pos !== false && $pos === 0) {
-                    array_push($authorsBitmask, $val == '1');
-                }
-            }
-            $notifySubmitter = $form->getValue('submitter') == '1';
-        }
+        $recipients = $form->getSelectedRecipients($document, $this->getRequest()->getPost());
 
         $notification->prepareMail(
             $document,
             $this->view->serverUrl() . $url,
-            $notifySubmitter,
-            $authorsBitmask
+            $recipients
         );
     }
 

diff --git a/modules/admin/forms/Notification.php b/modules/admin/forms/Notification.php
@@ -89,7 +89,9 @@ public function getRows()
             $option = $this->personToArray($submitters[0]);
             $option['value'] = 'submitter';
             $option['type'] = $translator->translate('admin_workflow_notification_submitter');
+            $option['checked'] = 1;
             $options['submitter'] = $option;
+
         }
 
         $authors = $document->getPersonAuthor();
@@ -110,6 +112,7 @@ public function getRows()
                     $option['value'] = 'author_' . $index;
                     $pos = $index + 1;
                     $option['type'] = "$pos. {$translator->translate('admin_workflow_notification_author')}";
+                    $option['checked'] = ($pos = 1) ? '1' : '0';
                     $options[] = $option;
                 }
             }

diff --git a/modules/admin/forms/WorkflowNotification.php b/modules/admin/forms/WorkflowNotification.php
@@ -88,7 +88,6 @@ public function populateFromModel($document)
             $this->addSubForm($subform, 'notification');
         }
     }
-
     /**
      * Returns list of recipients for document state change notifications.
      *
@@ -113,7 +112,7 @@ public function getRecipients($document) {
 
     protected function addPersons(&$recipients, $persons, $role = 'author')
     {
-        foreach ($persons as $person) {
+        foreach ($persons as $index => $person) {
             $fullname = $person->getDisplayName();
             $email = $person->getEmail();
             if (strlen(trim($email)) > 0) {
@@ -153,4 +152,36 @@ protected function addPersons(&$recipients, $persons, $role = 'author')
             }
         }
     }
+
+    /**
+     * Returns the recipients that have been selected in the form.
+     * @return
+     */
+    public function getSelectedRecipients($document, $post)
+    {
+        $recipients = [];
+
+        $authors = $document->getPersonAuthor();
+
+        $selected = [];
+
+        // TODO $post = $this->getValues();
+
+        foreach ($authors as $index => $author) {
+            $key = "author_$index";
+            if (isset($post[$key]) && $post[$key] == '1') {
+                $selected[] = $authors[$index];
+            }
+        }
+
+        if (count($selected) > 0) {
+            $this->addPersons($recipients, $selected, 'author');
+        }
+
+        if (isset($post['submitter']) && $post['submitter'] == '1') {
+            $this->addPersons($recipients, [$document->getPersonSubmitter(0)], 'submitter');
+        }
+
+        return $recipients;
+    }
 }
diff --git a/modules/publish/controllers/DepositController.php b/modules/publish/controllers/DepositController.php
@@ -149,7 +149,7 @@ public function depositAction() {
             true
         );
         $notification->prepareMail(
-            $this->document, Application_Util_Notification::SUBMISSION, $this->view->serverUrl() . $url
+            $this->document,  $this->view->serverUrl() . $url
         );
 
         return $this->_helper->Redirector->redirectToAndExit($targetAction, null, $targetController, $targetModule);

diff --git a/tests/modules/admin/controllers/WorkflowControllerTest.php b/tests/modules/admin/controllers/WorkflowControllerTest.php
@@ -304,10 +304,7 @@ public function testSubmitterNotificationIsAvailable()
 
         $this->assertContains('[email protected]', $body);
         $this->assertContains('[email protected]', $body);
-        $this->assertContains(
-            '<input type="checkbox" name="submitter" id="submitter" value="1" checked="checked"',
-            $body
-        );
+        $this->assertXpath('//input[@type="checkbox" and @id="submitter" and @name="submitter" and @value="1" and @checked="checked"]');
     }
 
     public function testAuthorNotificationIsAvailable()
@@ -322,10 +319,7 @@ public function testAuthorNotificationIsAvailable()
 
         $this->assertContains('[email protected]', $body);
         $this->assertContains('[email protected]', $body);
-        $this->assertContains(
-            '<input type="checkbox" name="author_1" id="author_1" value="1" checked="checked"',
-            $body
-        );
+        $this->assertXpath('//input[@type="checkbox" and @id="author_0" and @name="author_0" and @value="1" and @checked="checked"]');
     }
 
     public function testSubmitterNotificationIsNotAvailable()
@@ -339,14 +333,8 @@ public function testSubmitterNotificationIsNotAvailable()
 
         $this->assertNotContains('[email protected]', $body);
         $this->assertContains('[email protected]', $body);
-        $this->assertContains(
-            '<input type="checkbox" name="submitter" id="submitter" value="1" disabled="1"',
-            $body
-        );
-        $this->assertContains(
-            '<input type="checkbox" name="author_1" id="author_1" value="1" checked="checked"',
-            $body
-        );
+        $this->assertXpath('//input[@type="checkbox" and @id="submitter" and @name="submitter" and @value="1" and @disabled="disabled"]');
+        $this->assertXpath('//input[@type="checkbox" and @id="author_0" and @name="author_0" and @value="1" and @checked="checked"]');
     }
 
     public function testAuthorNotificationIsNotAvailable()
@@ -360,14 +348,8 @@ public function testAuthorNotificationIsNotAvailable()
 
         $this->assertContains('[email protected]', $body);
         $this->assertNotContains('[email protected]', $body);
-        $this->assertContains(
-            '<input type="checkbox" name="submitter" id="submitter" value="1" checked="checked"',
-            $body
-        );
-        $this->assertContains(
-            '<input type="checkbox" name="author_1" id="author_1" value="1" disabled="1"',
-            $body
-        );
+        $this->assertXpath('//input[@type="checkbox" and @id="submitter" and @name="submitter" and @value="1" and @checked="checked"]');
+        $this->assertXpath('//input[@type="checkbox" and @id="author_0" and @name="author_0" and @value="1" and @disabled="disabled"]');
     }
 
     public function testAuthorNotificationForMultipleAuthors()
@@ -405,21 +387,11 @@ public function testAuthorNotificationForMultipleAuthors()
         $this->assertContains('[email protected]', $body);
         $this->assertContains('[email protected]', $body);
 
-        $this->assertContains(
-            '<input type="checkbox" name="submitter" id="submitter" value="1" checked="checked"', $body
-        );
-        $this->assertContains(
-            '<input type="checkbox" name="author_1" id="author_1" value="1" checked="checked"', $body
-        );
-        $this->assertContains(
-            '<input type="checkbox" name="author_2" id="author_2" value="1" checked="checked"', $body
-        );
-        $this->assertContains(
-            '<input type="checkbox" name="author_3" id="author_3" value="1" disabled="1"', $body
-        );
-        $this->assertContains(
-            '<input type="checkbox" name="author_4" id="author_4" value="1" checked="checked"', $body
-        );
+        $this->assertXpath('//input[@type="checkbox" and @id="submitter" and @name="submitter" and @value="1" and @checked="checked"]');
+        $this->assertXpath('//input[@type="checkbox" and @id="author_0" and @name="author_0" and @value="1" and @checked="checked"]');
+        $this->assertXpath('//input[@type="checkbox" and @id="author_1" and @name="author_1" and @value="1" and @checked="checked"]');
+        $this->assertXpath('//input[@type="checkbox" and @id="author_2" and @name="author_2" and @value="1" and @disabled="disabled"]');
+        $this->assertXpath('//input[@type="checkbox" and @id="author_3" and @name="author_3" and @value="1" and @checked="checked"]');
     }
 
     public function testShowDocInfoOnConfirmationPage()

diff --git a/tests/modules/admin/forms/WorkflowNotificationTest.php b/tests/modules/admin/forms/WorkflowNotificationTest.php
@@ -86,7 +86,7 @@ public function testGetRecipients()
     {
         $this->setUpTestDocument();
 
-        $form = new Admin_Form_DocumentStateChange('published');
+        $form = new Admin_Form_WorkflowNotification('published');
 
         $recipients = $form->getRecipients($this->doc);
 
@@ -112,6 +112,28 @@ public function testGetRecipients()
         ], $recipients);
     }
 
+    public function testGetSelectedRecipients() {
+        $this->setUpTestDocument();
+
+        $form = new Admin_Form_WorkflowNotification('published');
+
+        $post = [
+            'sureyes' => 'Yes',
+            'id' => 150,
+            'submitter' => '1',
+            'author_0' => '1',
+            'author_1' => '1',
+            'author_2' => '1'
+        ];
+
+        $recipients = $form->getSelectedRecipients($this->doc, $post);
+
+        $this->assertCount(2, $recipients);
+        $this->assertArrayHasKey('[email protected]', $recipients);
+        $this->assertArrayHasKey('[email protected]', $recipients);
+
+        // TODO check more expectations (array structure)
+    }
 
     /* TODO integrate or delete
      *     /**