From 0766c0922f04e5f19cf09dc1286fcd753d126bdf Mon Sep 17 00:00:00 2001
From: Chris Wheeler <grintor@gmail.com>
Date: Thu, 14 Aug 2014 21:17:40 -0400
Subject: [PATCH] Disable closing tickets with empty required fields

This will disable the ability for a user to close a ticket when they
have not filled out all of the required fields. Explains the problem to
the user trying to close and directs them to the edit page. Added some
functions to the $ticket class to make it easy to determine the missing
required fields.
---
 include/ajax.tickets.php                      | 10 ++++---
 include/class.dynamic_forms.php               |  3 ++
 include/class.ticket.php                      | 28 ++++++++++++++++-
 include/staff/templates/dynamic-form.tmpl.php |  8 ++++-
 .../staff/templates/status-options.tmpl.php   | 30 +++++++++++--------
 include/staff/ticket-view.inc.php             | 12 ++++++--
 scp/css/scp.css                               |  3 +-
 7 files changed, 73 insertions(+), 21 deletions(-)

diff --git a/include/ajax.tickets.php b/include/ajax.tickets.php
index d5d12bea9..d8fccf5dc 100644
--- a/include/ajax.tickets.php
+++ b/include/ajax.tickets.php
@@ -609,7 +609,7 @@ class TicketsAjaxAPI extends AjaxController {
 
         $state = strtolower($status->getState());
 
-        if (!$errors && $ticket->setStatus($status, $_REQUEST['comments'])) {
+        if (!$errors && $ticket->setStatus($status, $_REQUEST['comments'], $errors)) {
 
             if ($state == 'deleted') {
                 $msg = sprintf('%s %s',
@@ -723,13 +723,15 @@ class TicketsAjaxAPI extends AjaxController {
                 if (($ticket=Ticket::lookup($tid))
                         && $ticket->getStatusId() != $status->getId()
                         && $ticket->checkStaffAccess($thisstaff)
-                        && $ticket->setStatus($status, $comments))
+                        && $ticket->setStatus($status, $comments, $errors))
                     $i++;
             }
 
-            if (!$i)
-                $errors['err'] = sprintf(__('Unable to change status for %s'),
+            if (!$i) {
+                $errors['err'] = $errors['err']
+                    ?: sprintf(__('Unable to change status for %s'),
                         _N('the selected ticket', 'any of the selected tickets', $count));
+            }
             else {
                 // Assume success
                 if ($i==$count) {
diff --git a/include/class.dynamic_forms.php b/include/class.dynamic_forms.php
index 3620cc1e8..d2cee5573 100644
--- a/include/class.dynamic_forms.php
+++ b/include/class.dynamic_forms.php
@@ -622,6 +622,9 @@ class DynamicFormField extends VerySimpleModel {
     function isRequiredForUsers() {
         return $this->hasFlag(self::FLAG_CLIENT_REQUIRED);
     }
+    function isRequiredForClose() {
+        return $this->hasFlag(self::FLAG_CLOSE_REQUIRED);
+    }
     function isEditableToStaff() {
         return $this->hasFlag(self::FLAG_ENABLED)
             && $this->hasFlag(self::FLAG_AGENT_EDIT);
diff --git a/include/class.ticket.php b/include/class.ticket.php
index 33400acbd..5bdec7646 100644
--- a/include/class.ticket.php
+++ b/include/class.ticket.php
@@ -801,6 +801,26 @@ class Ticket {
         }
     }
 
+    function getMissingRequiredFields() {
+        $returnArray = array();
+        $forms=DynamicFormEntry::forTicket($this->getId());
+        foreach ($forms as $form) {
+            foreach ($form->getFields() as $field) {
+                if ($field->isRequiredForClose()) {
+                    if (!($field->answer->get('value'))) {
+                        array_push($returnArray, $field->get('label'));
+                    }
+                }
+            }
+        }
+        return $returnArray;
+    }
+
+    function getMissingRequiredField() {
+        $fields = $this->getMissingRequiredFields();
+        return $fields[0];
+    }
+
     function addCollaborator($user, $vars, &$errors) {
 
         if (!$user || $user->getId()==$this->getOwnerId())
@@ -957,7 +977,7 @@ class Ticket {
     }
 
     //Status helper.
-    function setStatus($status, $comments='') {
+    function setStatus($status, $comments='', &$errors=array()) {
         global $thisstaff;
 
         if ($status && is_numeric($status))
@@ -979,6 +999,12 @@ class Ticket {
         $ecb = null;
         switch($status->getState()) {
             case 'closed':
+                if ($this->getMissingRequiredFields()) {
+                    $errors['err'] = sprintf(__(
+                        'This ticket is missing data on %s one or more required fields %s and cannot be closed'),
+                    '', '');
+                    return false;
+                }
                 $sql.=', closed=NOW(), lastupdate=NOW(), duedate=NULL ';
                 if ($thisstaff)
                     $sql.=', staff_id='.db_input($thisstaff->getId());
diff --git a/include/staff/templates/dynamic-form.tmpl.php b/include/staff/templates/dynamic-form.tmpl.php
index 4efb94a21..8d725dece 100644
--- a/include/staff/templates/dynamic-form.tmpl.php
+++ b/include/staff/templates/dynamic-form.tmpl.php
@@ -50,7 +50,7 @@ if (isset($options['entry']) && $options['mode'] == 'edit') { ?>
                 <?php
             }
             else { ?>
-                <td class="multi-line <?php if ($field->isRequiredForStaff()) echo 'required';
+                <td class="multi-line <?php if ($field->isRequiredForStaff() || $field->isRequiredForClose()) echo 'required';
                 ?>" style="min-width:120px;" <?php if ($options['width'])
                     echo "width=\"{$options['width']}\""; ?>>
                 <?php echo Format::htmlchars($field->getLocal('label')); ?>:</td>
@@ -77,6 +77,12 @@ if (isset($options['entry']) && $options['mode'] == 'edit') { ?>
                 ?>" data-entry-id="<?php echo $field->getAnswer()->get('entry_id');
                 ?>"> <i class="icon-trash"></i> </a></div><?php
             }
+            if (!$a->getValue() && $field->isRequiredForClose()) {
+?><i class="icon-warning-sign help-tip warning"
+    data-title="<?php echo __('Required to close ticket'); ?>"
+    data-content="<?php echo __('Data is required in this field in order to close the related ticket'); ?>"
+/></i><?php
+            }
             if ($field->get('hint') && !$field->isBlockLevel()) { ?>
                 <br /><em style="color:gray;display:inline-block"><?php
                     echo Format::htmlchars($field->getLocal('hint')); ?></em>
diff --git a/include/staff/templates/status-options.tmpl.php b/include/staff/templates/status-options.tmpl.php
index cdcaa395b..c82696e6f 100644
--- a/include/staff/templates/status-options.tmpl.php
+++ b/include/staff/templates/status-options.tmpl.php
@@ -12,6 +12,23 @@ $actions= array(
             'action' => 'reopen'
             ),
         );
+
+$states = array('open');
+if ($thisstaff->canCloseTickets() && (!$ticket || !$ticket->getMissingRequiredFields()))
+    $states = array_merge($states, array('closed'));
+
+$statusId = $ticket ? $ticket->getStatusId() : 0;
+$nextStatuses = array();
+foreach (TicketStatusList::getStatuses(
+            array('states' => $states)) as $status) {
+    if (!isset($actions[$status->getState()])
+            || $statusId == $status->getId())
+        continue;
+    $nextStatuses[] = $status;
+}
+
+if (!$nextStatuses)
+    return;
 ?>
 
 <span
@@ -26,18 +43,7 @@ $actions= array(
 <div id="action-dropdown-statuses"
     class="action-dropdown anchor-right">
     <ul>
-    <?php
-    $states = array('open');
-    if ($thisstaff->canCloseTickets())
-        $states = array_merge($states, array('closed'));
-
-    $statusId = $ticket ? $ticket->getStatusId() : 0;
-    foreach (TicketStatusList::getStatuses(
-                array('states' => $states))->all() as $status) {
-        if (!isset($actions[$status->getState()])
-                || $statusId == $status->getId())
-            continue;
-        ?>
+<?php foreach ($nextStatuses as $status) { ?>
         <li>
             <a class="no-pjax <?php
                 echo $ticket? 'ticket-action' : 'tickets-action'; ?>"
diff --git a/include/staff/ticket-view.inc.php b/include/staff/ticket-view.inc.php
index 74f819e0d..207d573c5 100644
--- a/include/staff/ticket-view.inc.php
+++ b/include/staff/ticket-view.inc.php
@@ -605,15 +605,23 @@ print $response_form->getField('attachments')->render();
                 </td>
             </tr>
             <tr>
-                <td width="120">
+                <td width="120" style="vertical-align:top">
                     <label><strong><?php echo __('Ticket Status');?>:</strong></label>
                 </td>
                 <td>
+                    <?php
+                    if ($outstanding = $ticket->getMissingRequiredFields()) { ?>
+                    <div class="warning-banner"><?php echo sprintf(__(
+                        'This ticket is missing data on %s one or more required fields %s and cannot be closed'),
+                        "<a href=\"tickets.php?id={$ticket->getId()}&a=edit\">",
+                        '</a>'
+                    ); ?></div>
+<?php               } ?>
                     <select name="reply_status_id">
                     <?php
                     $statusId = $info['reply_status_id'] ?: $ticket->getStatusId();
                     $states = array('open');
-                    if ($thisstaff->canCloseTickets())
+                    if ($thisstaff->canCloseTickets() && !$outstanding)
                         $states = array_merge($states, array('closed'));
 
                     foreach (TicketStatusList::getStatuses(
diff --git a/scp/css/scp.css b/scp/css/scp.css
index 6bbdcc8ab..b97e1cc2b 100644
--- a/scp/css/scp.css
+++ b/scp/css/scp.css
@@ -2018,7 +2018,8 @@ button a:hover {
     font-style: italic;
 }
 
-.form_table tr:hover i.help-tip {
+.form_table tr:hover i.help-tip,
+.form_table tr i.help-tip.warning {
     opacity: 1;
     color: #ffc20f;
 }
-- 
GitLab