From 89c0d79879ca8247ac7c1af8a4a06f8a52602eb1 Mon Sep 17 00:00:00 2001
From: Peter Rotich <peter@osticket.com>
Date: Tue, 30 Dec 2014 21:07:26 +0000
Subject: [PATCH] fixes: Review

Key permissions definition array
Move canned and faq permissions to their respective classes.
---
 include/ajax.kbase.php                        |  2 +-
 include/class.canned.php                      | 19 +++++++++++++++++
 include/class.email.php                       |  2 ++
 include/class.faq.php                         | 18 +++++++++++++++-
 include/class.knowledgebase.php               | 21 -------------------
 include/class.nav.php                         |  4 ++--
 include/class.organization.php                |  1 -
 include/class.report.php                      |  2 ++
 include/class.role.php                        |  6 ------
 include/class.task.php                        | 12 +++++++++++
 include/class.ticket.php                      | 15 +++++++++++++
 include/staff/category.inc.php                |  2 +-
 include/staff/faq-category.inc.php            |  2 +-
 include/staff/faq-view.inc.php                |  4 ++--
 include/staff/faq.inc.php                     |  2 +-
 include/staff/role.inc.php                    |  4 ++--
 .../streams/core/1ee831c8-36f6b328.task.php   |  4 ++--
 scp/canned.php                                |  2 +-
 scp/categories.php                            |  2 +-
 scp/emailsettings.php                         |  2 +-
 scp/faq.php                                   |  4 ++--
 scp/roles.php                                 |  6 +++++-
 22 files changed, 89 insertions(+), 47 deletions(-)

diff --git a/include/ajax.kbase.php b/include/ajax.kbase.php
index a6b9a388a..3909fd831 100644
--- a/include/ajax.kbase.php
+++ b/include/ajax.kbase.php
@@ -55,7 +55,7 @@ class KbaseAjaxAPI extends AjaxController {
                 $faq->getId(),
                 $faq->getNumAttachments());
         if($thisstaff
-                && $thisstaff->getRole()->hasPerm(KnowledgebaseModel::PERM_FAQ)) {
+                && $thisstaff->getRole()->hasPerm(FAQ::PERM_MANAGE)) {
             $resp.=sprintf(' | <a href="faq.php?id=%d&a=edit">'.__('Edit').'</a>',$faq->getId());
 
         }
diff --git a/include/class.canned.php b/include/class.canned.php
index a7154b974..ca1354332 100644
--- a/include/class.canned.php
+++ b/include/class.canned.php
@@ -15,6 +15,25 @@
 **********************************************************************/
 include_once(INCLUDE_DIR.'class.file.php');
 
+class CannedModel {
+
+    const PERM_MANAGE = 'canned.manage';
+
+    static protected $perms = array(
+            self::PERM_MANAGE => array(
+                'title' =>
+                /* @trans */ 'Premade',
+                'desc'  =>
+                /* @trans */ 'Ability to add/update/disable/delete canned responses')
+    );
+
+    static function getPermissions() {
+        return self::$perms;
+    }
+}
+
+RolePermission::register( /* @trans */ 'Knowledgebase', CannedModel::getPermissions());
+
 class Canned {
     var $id;
     var $ht;
diff --git a/include/class.email.php b/include/class.email.php
index 438f033fa..ab0727f7c 100644
--- a/include/class.email.php
+++ b/include/class.email.php
@@ -39,7 +39,9 @@ class EmailModel extends VerySimpleModel {
 
     static protected $perms = array(
             self::PERM_BANLIST => array(
+                'title' =>
                 /* @trans */ 'Banlist',
+                'desc'  =>
                 /* @trans */ 'Ability to add/remove emails from banlist via ticket interface'),
             );
 
diff --git a/include/class.faq.php b/include/class.faq.php
index 4e0fe8f45..055da0c27 100644
--- a/include/class.faq.php
+++ b/include/class.faq.php
@@ -46,6 +46,16 @@ class FAQ extends VerySimpleModel {
         ),
     );
 
+    const PERM_MANAGE  = 'faq.manage';
+    static protected $perms = array(
+            self::PERM_MANAGE => array(
+                'title' =>
+                /* @trans */ 'FAQ',
+                'desc'  =>
+                /* @trans */ 'Ability to add/update/disable/delete knowledgebase categories and FAQs'),
+            );
+
+
     var $attachments;
     var $topics;
     var $_local;
@@ -481,8 +491,14 @@ class FAQ extends VerySimpleModel {
             $this->updated = SqlFunction::NOW();
         return parent::save($refetch || $this->dirty);
     }
+
+    static function getPermissions() {
+        return self::$perms;
+    }
 }
-FAQ::_inspect();
+
+RolePermission::register( /* @trans */ 'Knowledgebase',
+        FAQ::getPermissions());
 
 class FaqTopic extends VerySimpleModel {
 
diff --git a/include/class.knowledgebase.php b/include/class.knowledgebase.php
index 8dbbbbd4a..33b214d6a 100644
--- a/include/class.knowledgebase.php
+++ b/include/class.knowledgebase.php
@@ -15,27 +15,6 @@
 **********************************************************************/
 require_once("class.file.php");
 
-class KnowledgebaseModel {
-
-    const PERM_PREMADE  = 'kb.premade';
-    const PERM_FAQ      = 'kb.faq';
-
-    static protected $perms = array(
-            self::PERM_PREMADE => array(
-                /* @trans */ 'Premade',
-                /* @trans */ 'Ability to add/update/disable/delete canned responses'),
-            self::PERM_FAQ => array(
-                /* @trans */ 'FAQ',
-                /* @trans */ 'Ability to add/update/disable/delete knowledgebase categories and FAQs'),
-            );
-
-    static function getPermissions() {
-        return self::$perms;
-    }
-}
-
-RolePermission::register( /* @trans */ 'Knowledgebase', KnowledgebaseModel::getPermissions());
-
 class Knowledgebase {
 
     function Knowledgebase($id) {
diff --git a/include/class.nav.php b/include/class.nav.php
index 53df0df3d..e8359e00d 100644
--- a/include/class.nav.php
+++ b/include/class.nav.php
@@ -161,9 +161,9 @@ class StaffNav {
                 case 'kbase':
                     $subnav[]=array('desc'=>__('FAQs'),'href'=>'kb.php', 'urls'=>array('faq.php'), 'iconclass'=>'kb');
                     if($staff) {
-                        if ($staff->getRole()->hasPerm(KnowledgebaseModel::PERM_FAQ))
+                        if ($staff->getRole()->hasPerm(FAQ::PERM_MANAGE))
                             $subnav[]=array('desc'=>__('Categories'),'href'=>'categories.php','iconclass'=>'faq-categories');
-                        if ($staff->getRole()->hasPerm(KnowledgebaseModel::PERM_PREMADE))
+                        if ($staff->getRole()->hasPerm(CannedModel::PERM_MANAGE))
                             $subnav[]=array('desc'=>__('Canned Responses'),'href'=>'canned.php','iconclass'=>'canned');
                     }
                    break;
diff --git a/include/class.organization.php b/include/class.organization.php
index 0c6bb5212..fa0ffc9cf 100644
--- a/include/class.organization.php
+++ b/include/class.organization.php
@@ -466,5 +466,4 @@ Filter::addSupportedMatches(/*@trans*/ 'Organization Data', function() {
     }
     return $matches;
 },40);
-Organization::_inspect();
 ?>
diff --git a/include/class.report.php b/include/class.report.php
index 257b1236c..0e8bae980 100644
--- a/include/class.report.php
+++ b/include/class.report.php
@@ -6,7 +6,9 @@ class ReportModel {
 
     static protected $perms = array(
             self::PERM_AGENTS => array(
+                'title' =>
                 /* @trans */ 'Stats',
+                'desc'  =>
                 /* @trans */ 'Ability to view stats of other agents in allowed departments'),
             );
 
diff --git a/include/class.role.php b/include/class.role.php
index 48e99a784..476bd9a72 100644
--- a/include/class.role.php
+++ b/include/class.role.php
@@ -313,10 +313,4 @@ class RolePermission {
                             static::$_permissions[$group] ?: array(), $perms);
     }
 }
-
-// Classes that might need to register roles permissions
-include_once INCLUDE_DIR.'class.knowledgebase.php';
-include_once INCLUDE_DIR.'class.email.php';
-include_once INCLUDE_DIR.'class.report.php';
-
 ?>
diff --git a/include/class.task.php b/include/class.task.php
index 8a381adf4..54da8170f 100644
--- a/include/class.task.php
+++ b/include/class.task.php
@@ -41,22 +41,34 @@ class TaskModel extends VerySimpleModel {
 
     static protected $perms = array(
             self::PERM_CREATE    => array(
+                'title' =>
                 /* @trans */ 'Create',
+                'desc'  =>
                 /* @trans */ 'Ability to create tasks'),
             self::PERM_EDIT      => array(
+                'title' =>
                 /* @trans */ 'Edit',
+                'desc'  =>
                 /* @trans */ 'Ability to edit tasks'),
             self::PERM_ASSIGN    => array(
+                'title' =>
                 /* @trans */ 'Assign',
+                'desc'  =>
                 /* @trans */ 'Ability to assign tasks to agents or teams'),
             self::PERM_TRANSFER  => array(
+                'title' =>
                 /* @trans */ 'Transfer',
+                'desc'  =>
                 /* @trans */ 'Ability to transfer tasks between departments'),
             self::PERM_CLOSE     => array(
+                'title' =>
                 /* @trans */ 'Close',
+                'desc'  =>
                 /* @trans */ 'Ability to close tasks'),
             self::PERM_DELETE    => array(
+                'title' =>
                 /* @trans */ 'Delete',
+                'desc'  =>
                 /* @trans */ 'Ability to delete tasks'),
             );
 
diff --git a/include/class.ticket.php b/include/class.ticket.php
index 5931c7b30..b1b3ea30c 100644
--- a/include/class.ticket.php
+++ b/include/class.ticket.php
@@ -97,25 +97,40 @@ class TicketModel extends VerySimpleModel {
 
     static protected $perms = array(
             self::PERM_CREATE => array(
+                'title' =>
                 /* @trans */ 'Create',
+                'desc'  =>
                 /* @trans */ 'Ability to open tickets on behalf of users'),
             self::PERM_EDIT => array(
+                'title' =>
                 /* @trans */ 'Edit',
+                'desc'  =>
                 /* @trans */ 'Ability to edit tickets'),
             self::PERM_ASSIGN => array(
+                'title' =>
                 /* @trans */ 'Assign',
+                'desc'  =>
                 /* @trans */ 'Ability to assign tickets to agents or teams'),
             self::PERM_TRANSFER => array(
+
+                'title' =>
                 /* @trans */ 'Transfer',
+                'desc'  =>
                 /* @trans */ 'Ability to transfer tickets between departments'),
             self::PERM_REPLY => array(
+                'title' =>
                 /* @trans */ 'Post Reply',
+                'desc'  =>
                 /* @trans */ 'Ability to post a ticket reply'),
             self::PERM_CLOSE => array(
+                'title' =>
                 /* @trans */ 'Close',
+                'desc'  =>
                 /* @trans */ 'Ability to close tickets'),
             self::PERM_DELETE => array(
+                'title' =>
                 /* @trans */ 'Delete',
+                'desc'  =>
                 /* @trans */ 'Ability to delete tickets'),
             );
 
diff --git a/include/staff/category.inc.php b/include/staff/category.inc.php
index b8942cc6b..55926a0aa 100644
--- a/include/staff/category.inc.php
+++ b/include/staff/category.inc.php
@@ -1,6 +1,6 @@
 <?php
 if (!defined('OSTSCPINC') || !$thisstaff
-        || !$thisstaff->getRole()->hasPerm(KnowledgebaseModel::PERM_FAQ))
+        || !$thisstaff->getRole()->hasPerm(FAQ::PERM_MANAGE))
     die('Access Denied');
 
 $info=array();
diff --git a/include/staff/faq-category.inc.php b/include/staff/faq-category.inc.php
index 721cd9de2..afc5194bb 100644
--- a/include/staff/faq-category.inc.php
+++ b/include/staff/faq-category.inc.php
@@ -17,7 +17,7 @@ if(!defined('OSTSTAFFINC') || !$category || !$thisstaff) die('Access Denied');
 <?php echo Format::display($category->getDescription()); ?>
 </div>
 <?php
-if ($thisstaff->getRole()->hasPerm(KnowledgebaseModel::PERM_FAQ)) {
+if ($thisstaff->getRole()->hasPerm(FAQ::PERM_MANAGE)) {
     echo sprintf('<div class="cat-manage-bar"><a href="categories.php?id=%d" class="Icon editCategory">'.__('Edit Category').'</a>
              <a href="categories.php" class="Icon deleteCategory">'.__('Delete Category').'</a>
              <a href="faq.php?cid=%d&a=add" class="Icon newFAQ">'.__('Add New FAQ').'</a></div>',
diff --git a/include/staff/faq-view.inc.php b/include/staff/faq-view.inc.php
index a18e340c7..8afda9a67 100644
--- a/include/staff/faq-view.inc.php
+++ b/include/staff/faq-view.inc.php
@@ -79,7 +79,7 @@ $query = http_build_query($query); ?>
         echo __('Print'); ?>
     </a></button>
 <?php
-if ($thisstaff->getRole()->hasPerm(KnowledgebaseModel::PERM_FAQ)) { ?>
+if ($thisstaff->getRole()->hasPerm(FAQ::PERM_MANAGE)) { ?>
     <button>
     <i class="icon-edit"></i>
     <a href="faq.php?id=<?php echo $faq->getId(); ?>&a=edit"><?php
@@ -104,7 +104,7 @@ if ($thisstaff->getRole()->hasPerm(KnowledgebaseModel::PERM_FAQ)) { ?>
 <hr>
 
 <?php
-if ($thisstaff->getRole()->hasPerm(KnowledgebaseModel::PERM_FAQ)) { ?>
+if ($thisstaff->getRole()->hasPerm(FAQ::PERM_MANAGE)) { ?>
 <form action="faq.php?id=<?php echo  $faq->getId(); ?>" method="post">
     <?php csrf_token(); ?>
     <input type="hidden" name="do" value="manage-faq">
diff --git a/include/staff/faq.inc.php b/include/staff/faq.inc.php
index 29f3dbe7d..a1bdfcc7a 100644
--- a/include/staff/faq.inc.php
+++ b/include/staff/faq.inc.php
@@ -1,6 +1,6 @@
 <?php
 if (!defined('OSTSCPINC') || !$thisstaff
-        || !$thisstaff->getRole()->hasPerm(KnowledgebaseModel::PERM_FAQ))
+        || !$thisstaff->getRole()->hasPerm(FAQ::PERM_MANAGE))
     die('Access Denied');
 
 $info=array();
diff --git a/include/staff/role.inc.php b/include/staff/role.inc.php
index e36b87073..25384cd39 100644
--- a/include/staff/role.inc.php
+++ b/include/staff/role.inc.php
@@ -98,8 +98,8 @@ $info = Format::htmlchars(($errors && $_POST) ? array_merge($info, $_POST) : $in
               &nbsp;&nbsp;
               <?php
                 echo sprintf('%s - <em>%s</em>',
-                      Format::htmlchars(__($v[0])),
-                    Format::htmlchars(__($v[1])));
+                      Format::htmlchars(__($v['title'])),
+                    Format::htmlchars(__($v['desc'])));
               ?>
              </label>
             </td>
diff --git a/include/upgrader/streams/core/1ee831c8-36f6b328.task.php b/include/upgrader/streams/core/1ee831c8-36f6b328.task.php
index 3b8ff65b4..ef8cd3690 100644
--- a/include/upgrader/streams/core/1ee831c8-36f6b328.task.php
+++ b/include/upgrader/streams/core/1ee831c8-36f6b328.task.php
@@ -11,8 +11,8 @@ class GroupRoles extends MigrationTask {
             'can_assign_tickets'    => 'ticket.assign',
             'can_transfer_tickets'  => 'ticket.transfer',
             'can_ban_emails'        => 'emails.banlist',
-            'can_manage_premade'    => 'kb.premade',
-            'can_manage_faq'        => 'kb.faq',
+            'can_manage_premade'    => 'canned.manage',
+            'can_manage_faq'        => 'faq.manage',
             'can_view_staff_stats'  => 'stats.agents',
     );
 
diff --git a/scp/canned.php b/scp/canned.php
index d6e66a779..c8290c39f 100644
--- a/scp/canned.php
+++ b/scp/canned.php
@@ -19,7 +19,7 @@ include_once(INCLUDE_DIR.'class.canned.php');
 /* check permission */
 if(!$thisstaff
         ||
-        !$thisstaff->getRole()->hasPerm(KnowledgebaseModel::PERM_PREMADE)) {
+        !$thisstaff->getRole()->hasPerm(CannedModel::PERM_MANAGE)) {
     header('Location: kb.php');
     exit;
 }
diff --git a/scp/categories.php b/scp/categories.php
index 00e93620a..d4e6125b4 100644
--- a/scp/categories.php
+++ b/scp/categories.php
@@ -18,7 +18,7 @@ include_once(INCLUDE_DIR.'class.category.php');
 
 /* check permission */
 if(!$thisstaff ||
-        !$thisstaff->getRole()->hasPerm(KnowledgebaseModel::PERM_FAQ)) {
+        !$thisstaff->getRole()->hasPerm(FAQ::PERM_MANAGE)) {
     header('Location: kb.php');
     exit;
 }
diff --git a/scp/emailsettings.php b/scp/emailsettings.php
index 175fa4575..6f871b4bf 100644
--- a/scp/emailsettings.php
+++ b/scp/emailsettings.php
@@ -35,5 +35,5 @@ $nav->setTabActive('emails', 'emailsettings.php');
 require_once(STAFFINC_DIR.'header.inc.php');
 include_once(STAFFINC_DIR.$inc);
 include_once(STAFFINC_DIR.'footer.inc.php');
-?>
 
+?>
diff --git a/scp/faq.php b/scp/faq.php
index 7ea28c407..03c5f1601 100644
--- a/scp/faq.php
+++ b/scp/faq.php
@@ -155,12 +155,12 @@ $inc='faq-categories.inc.php'; //FAQs landing page.
 if($faq) {
     $inc='faq-view.inc.php';
     if ($_REQUEST['a']=='edit'
-            && $role->hasPerm(KnowledgebaseModel::PERM_FAQ))
+            && $role->hasPerm(FAQ::PERM_MANAGE))
         $inc='faq.inc.php';
     elseif ($_REQUEST['a'] == 'print')
         return $faq->printPdf();
 }elseif($_REQUEST['a']=='add'
-        && $role->hasPerm(KnowledgebaseModel::PERM_FAQ)) {
+        && $role->hasPerm(FAQ::PERM_MANAGE)) {
     $inc='faq.inc.php';
 } elseif($category && $_REQUEST['a']!='search') {
     $inc='faq-category.inc.php';
diff --git a/scp/roles.php b/scp/roles.php
index 02bb0fec0..2c9476e29 100644
--- a/scp/roles.php
+++ b/scp/roles.php
@@ -14,7 +14,11 @@
     vim: expandtab sw=4 ts=4 sts=4:
 **********************************************************************/
 
-require('admin.inc.php');
+require 'admin.inc.php';
+include_once INCLUDE_DIR . 'class.canned.php';
+include_once INCLUDE_DIR . 'class.faq.php';
+include_once INCLUDE_DIR . 'class.email.php';
+include_once INCLUDE_DIR . 'class.report.php';
 
 $errors = array();
 $role=null;
-- 
GitLab