From ab47fce80cbb05be9dbe9fc58e461eabb3f92c07 Mon Sep 17 00:00:00 2001 From: DonutsNL Date: Thu, 16 Nov 2023 19:55:15 +0000 Subject: [PATCH] Resolved linting issues --- front/filterpattern.form.php | 4 +- front/filterpattern.php | 4 +- hook.php | 9 +-- setup.php | 11 +-- src/Filter.php | 58 ++++++-------- src/FilterPattern.php | 59 +++++--------- src/TicketHandler.php | 77 ++++++++----------- ...ketFilterTest.php => TicketFilterTest.php} | 24 +++--- tests/bootstrap.php | 2 +- 9 files changed, 99 insertions(+), 149 deletions(-) rename tests/{ticketFilterTest.php => TicketFilterTest.php} (71%) diff --git a/front/filterpattern.form.php b/front/filterpattern.form.php index d302fb1..9021d31 100644 --- a/front/filterpattern.form.php +++ b/front/filterpattern.form.php @@ -38,9 +38,9 @@ use GlpiPlugin\Ticketfilter\FilterPattern; use Plugin; -include ("../../../inc/includes.php"); +include_once '../../../inc/includes.php'; Plugin::load('ticketfilter', true); $dropdown = new FilterPattern(); -include (GLPI_ROOT . "/front/dropdown.common.form.php"); \ No newline at end of file +include_once GLPI_ROOT . "/front/dropdown.common.form.php"; \ No newline at end of file diff --git a/front/filterpattern.php b/front/filterpattern.php index fd37622..d4e95ef 100644 --- a/front/filterpattern.php +++ b/front/filterpattern.php @@ -38,9 +38,9 @@ use GlpiPlugin\Ticketfilter\FilterPattern; use Plugin; -include ("../../../inc/includes.php"); +include_once "../../../inc/includes.php"; Plugin::load('ticketfilter', true); $dropdown = new FilterPattern(); -include(GLPI_ROOT . '/front/dropdown.common.php'); \ No newline at end of file +include_once GLPI_ROOT . '/front/dropdown.common.php'; \ No newline at end of file diff --git a/hook.php b/hook.php index eee986c..e7a1514 100644 --- a/hook.php +++ b/hook.php @@ -44,7 +44,7 @@ * test */ // phpcs:ignore PSR1.Function.CamelCapsMethodName -function plugin_ticketfilter_getDropdown() : array +function plugin_ticketfilter_getDropdown() : array { return [FilterPattern::class => __("Filterpatterns", 'ticketfilter')]; } @@ -52,10 +52,10 @@ function plugin_ticketfilter_getDropdown() : array /** * Summary of plugin_ticketFilter install - * @return boolean + * @return booleansyste * test */ -// phpcs:ignore PSR1.Function.CamelCapsMethodName +//phpcs:ignore PSR1.Function.CamelCapsMethodName function plugin_ticketfilter_install() : bool { @@ -70,11 +70,10 @@ function plugin_ticketfilter_install() : bool /** - * * Summary of plugin_ticketFilter uninstall * @return boolean */ -// phpcs:ignore PSR1.Function.CamelCapsMethodName +//phpcs:ignore PSR1.Function.CamelCapsMethodName function plugin_ticketfilter_uninstall() : bool { diff --git a/setup.php b/setup.php index 37b039f..f028583 100644 --- a/setup.php +++ b/setup.php @@ -65,7 +65,7 @@ function plugin_init_ticketfilter() : void // Add hook (callback) on the PRE_ITEM_ADD event. // All new tickets are to be evaluated no matter the source. $PLUGIN_HOOKS[HOOKS::PRE_ITEM_ADD]['ticketfilter'] = [ - Ticket::class => [Filter::class, 'PreItemAdd'] + Ticket::class => [Filter::class, 'preItemAdd'] ]; } @@ -102,9 +102,6 @@ function plugin_version_ticketfilter() : array */ function plugin_ticketfilter_check_prerequisites() : bool { - if (false) { - return false; - } return true; } @@ -116,12 +113,8 @@ function plugin_ticketfilter_check_prerequisites() : bool */ function plugin_ticketfilter_check_config($verbose = false) : bool { - if (true) { // Your configuration check - return true; - } - if ($verbose) { echo __('Installed / not configured', 'TICKETFILTER'); } - return false; + return (true) ? true : false; } diff --git a/src/Filter.php b/src/Filter.php index e8c4bf1..41ee53b 100644 --- a/src/Filter.php +++ b/src/Filter.php @@ -58,22 +58,19 @@ class Filter { * @since 1.0.0 * @see setup.php hook */ - public static function PreItemAdd(Ticket $item) : void + public static function preItemAdd(Ticket $item) : void { - if(is_object($item)) { - - if(is_array($item->input) // Fields should be an array with values. - && key_exists('name', $item->input) // Name key (that is the Subject of the email) should exist. - && !empty($item->input['name'])) { // Name should not be emtpy, could happen with recurring tickets. - - // Search our pattern in the name field and find corresponding ticket(s) (if any) - self::searchForMatches($item); - } // Not an array with expected keys - } // Not a valid object. + if(is_object($item) && + is_array($item->input) && + key_exists('name', $item->input) && // Name key (that is the Subject of the email) should exist. + !empty($item->input['name'])) { // Name should not be emtpy, could happen with recurring tickets. + // Search our pattern in the name field and find corresponding ticket(s) (if any) + self::searchForMatches($item); + } } /** - * emptyReferencedObject(Ticket item) : void - + * emptyReferencedObject(Ticket item) : void - * Clean the referenced item and delete any mailbox items remaining * * @param Ticket $item The original ticket passed by the pre_item_add hook. @@ -86,7 +83,7 @@ private static function emptyReferencedObject(Ticket $item) : void // We cancelled the ticket creation and by doing so the mailgate will not clean the // email from the mailbox. We need to clean it manually. // Send a meaningfull message if mailgate was triggered by user from UI. - if(self::deleteEmail($item) == true) { + if(self::deleteEmail($item) === true) { Session::addMessageAfterRedirect(__("Ticket removed from mailbox, it is save to ignore related mailgate error"), true, WARNING); } $item->input = false; @@ -94,7 +91,7 @@ private static function emptyReferencedObject(Ticket $item) : void } /** - * searchForMatches(Ticket item) : bool - + * searchForMatches(Ticket item) : bool - * Perform a search in the glpi_tickets table using the searchString if one is found applying the * provided ticket match patterns from dropdown on the ticket name (email subject). * @@ -111,7 +108,7 @@ private static function searchForMatches(Ticket $item) : bool // Evaluate patterns if(is_array($patterns) - && count($patterns) >= 1 + && !empty($patterns) && array_key_exists(FilterPattern::TICKETMATCHSTR, $patterns['0'])) { // Loop through the patterns @@ -129,7 +126,6 @@ private static function searchForMatches(Ticket $item) : bool // Protect against risky patterns or SQL injections by validating the length // of the matchstring against what we expect it to be. - // todo: move to separate method for readability. if(strlen($searchString) <= $Filterpattern[FilterPattern::TICKETMATCHSTRLEN]) { $handler = new TicketHandler(); $r = $handler->searchTicketPool($searchString); @@ -153,28 +149,28 @@ private static function searchForMatches(Ticket $item) : bool trigger_error("TicketFilter: PregMatch failed! please review the pattern $p and correct it", E_USER_WARNING); } } // Pattern is configured inactive. - } // Loop. + } // Loop. } else { trigger_error("TicketFilter: No ticketfilter patterns found, please configure them or disable the plugin", E_USER_WARNING); } // Do we have at least 1 succesfull followup, prevent GLPI from creating a new ticket - if(is_array($itemIsMatched) && + if(is_array($itemIsMatched) && in_array("matched", $itemIsMatched)) { // Clear $item->input and fields to stop object from being created // https://glpi-developer-documentation.readthedocs.io/en/master/plugins/hooks.html self::emptyReferencedObject($item); - } + } return true; } /** * openMailGate(int Id) : MailCollector|null - * Returns a connected mailCollector object or [] - * - * @param int $Id Mail Collector ID - * @return MailCollector|null Union return types might be problematic with older php versions. - * @since 1.0.0 + * + * @param int $Id Mail Collector ID + * @return MailCollector|null Union return types might be problematic with older php versions. + * @since 1.0.0 */ private static function openMailGate(int $Id) : MailCollector|null { @@ -194,24 +190,20 @@ private static function openMailGate(int $Id) : MailCollector|null /** * deleteEmail(Ticket item) : bool - * Delete original email from the mailbox to the accepted folder. This is not performed by the mailgate - * if the Ticket passed by reference is nullified as described in the hook documentation. + * if the Ticket passed by reference is nullified as described in the hook documentation. * This actually causes an error where mailgate leaves the email untouched. - * + * * @param Ticket $item Ticket object containing the mailgate uid - * @return bool Returns true on success and false on failure - * @since 1.0.0 + * @return bool Returns true on success and false on failure + * @since 1.0.0 */ private static function deleteEmail(Ticket $item) : bool { // Check if ticket is fetched from the mailCollector if so open it; $mailCollector = (isset($item->input['_mailgate'])) ? self::openMailGate($item->input['_mailgate']) : false; if(is_object($mailCollector)){ - if($mailCollector->deleteMails($item->input['_uid'], MailCollector::ACCEPTED_FOLDER)) { - return true; - } else { - return false; - } + return ($mailCollector->deleteMails($item->input['_uid'], MailCollector::ACCEPTED_FOLDER)) ? true : false; } // instantiation of mailCollector failed return false; } -} \ No newline at end of file +} diff --git a/src/FilterPattern.php b/src/FilterPattern.php index 7a5450b..c500378 100644 --- a/src/FilterPattern.php +++ b/src/FilterPattern.php @@ -1,5 +1,4 @@ 0) { return __('Filterpatterns', 'ticketfilter'); @@ -87,8 +86,8 @@ static function getTypeName($nb = 0) : string * getMenuContent() : array | bool - * Method called by pre_item_add hook validates the object and passes * it to the RegEx Matching then decides what to do. - * - * @return mixed boolean|array + * + * @return mixed boolean|array */ public static function getMenuContent() { @@ -107,20 +106,20 @@ public static function getMenuContent() /** * getIcon() : string - * Sets icon for object. - * - * @return string $icon + * + * @return string $icon */ public static function getIcon() : string - { - return 'fas fa-filter'; + { + return 'fas fa-filter'; } /** * getAdditionalFields() : array - * Fetch fields for Dropdown 'add' form. Array order is equal with * field order in the form - * - * @return string $icon + * + * @return string $icon */ public function getAdditionalFields() { @@ -211,8 +210,8 @@ public function getAdditionalFields() /** * rawSearchOptions() : array - * Add fields to search and potential table columns - * - * @return array $rawSearchOptions + * + * @return array $rawSearchOptions */ public function rawSearchOptions() : array { @@ -242,23 +241,7 @@ public function rawSearchOptions() : array 'name' => __('Solved Match String', 'ticketfilter'), 'datatype' => 'text', ]; -/* - $tab[] = [ - 'id' => '8', - 'table' => $this->getTable(), - 'field' => 'AutomaticallyMerge', - 'name' => __('Automatically Merge', 'ticketfilter'), - 'datatype' => 'text', - ]; - - $tab[] = [ - 'id' => '9', - 'table' => $this->getTable(), - 'field' => 'LinkClosedTickets', - 'name' => __('Link To Closed Tickets', 'ticketfilter'), - 'datatype' => 'text', - ]; -*/ + $tab[] = [ 'id' => '9', 'table' => $this->getTable(), @@ -306,9 +289,9 @@ public static function getFilterPatterns() : array /** * install(Migration migration) : void - * Install table needed for Ticket Filter configuration dropdowns - * + * * @return void - * @see hook.php:plugin_ticketfilter_install() + * @see hook.php:plugin_ticketfilter_install() */ public static function install(Migration $migration) : void { @@ -340,7 +323,7 @@ public static function install(Migration $migration) : void `AutomaticallyMerge` tinyint NOT NULL DEFAULT '0', `ReopenClosedTickets` tinyint NOT NULL DEFAULT '0', `SearchTicketBody` tinyint NOT NULL DEFAULT '0', - `MatchSpecificSource` text NULL, + `MatchSpecificSource` text NULL, `SuppressNotification` tinyint NOT NULL DEFAULT '1', PRIMARY KEY (`id`), KEY `name` (`name`), @@ -353,7 +336,7 @@ public static function install(Migration $migration) : void // insert example rule; $query = <<\(JIRA-[0-9]{1,4}\)).*/', '11', '/.*?(?Closed).*/', '6'); SQL; $DB->query($query) or die($DB->error()); @@ -369,7 +352,7 @@ public static function install(Migration $migration) : void /** * uninstall(Migration migration) : void - * Uninstall tables uncomment the line to make plugin clean table. - * + * * @return void * @see hook.php:plugin_ticketfilter_uninstall() */ @@ -379,4 +362,4 @@ public static function uninstall(Migration $migration) : void $migration->displayMessage("Uninstalling $table"); $migration->dropTable($table); } -} \ No newline at end of file +} diff --git a/src/TicketHandler.php b/src/TicketHandler.php index d2a275d..4f5be43 100644 --- a/src/TicketHandler.php +++ b/src/TicketHandler.php @@ -47,9 +47,10 @@ class TicketHandler{ private $ticket; // The referenced ticket object - private $pattern = false; // The pattern configuration + private $pattern = false; // The pattern configuration private $status = '-1'; // Status of this object, 1 if reference ticket is loaded. + // Not used public function __construct() {} /** @@ -58,7 +59,7 @@ public function __construct() {} * and populates used pattern config for various evals. * * @param int ticketId identity of the ticket that needs to be loaded - * @param array pattern array holding the pattern used for the match. + * @param array pattern array holding the pattern used for the match. * @return void Object is passed by reference, no return values required * @since 1.1.0 */ @@ -87,8 +88,8 @@ public function initHandler(int $ticketId, array $filterPattern) : bool /** * getId() : int - * Returns ticket ID of ticket being handled or (int) 0 if no ticket was loaded by initHandler(); - * - * @param void + * + * @param void * @return int Status identifier or 0 if no ticket was loaded; * @since 1.1.0 */ @@ -104,8 +105,8 @@ public function getId() : int /** * getStatus() : int - * Returns ticket status of ticket being handled or (int) 0 if no ticket was loaded by initHandler(); - * - * @param void + * + * @param void * @return int Status identifier or 0 if no ticket was loaded; * @since 1.1.0 */ @@ -122,11 +123,11 @@ public function getStatus() : int * setStatusToNew() : bool - * Updates the status of the loaded ticket to NEW int(1) * - * @param void + * @param void * @return bool Allways returns true * @since 1.2.0 */ - public function setStatusToNew() : bool + public function setStatusToNew() : bool { global $DB; // Update status @@ -144,11 +145,11 @@ public function setStatusToNew() : bool * setStatusToSolved() : bool - * Updates the status of the loaded ticket to SOLVED int(5) * - * @param void + * @param void * @return bool Allways returns true * @since 1.2.0 */ - public function setStatusToSolved() : bool + public function setStatusToSolved() : bool { global $DB; // Update status @@ -167,13 +168,13 @@ public function setStatusToSolved() : bool * addReopendMessage(string patternName = '') : bool - * Adds a followup to indicate that ticketfilter reopend the closed ticket . * - * @param void - * @return bool + * @param void + * @return bool * @since 1.2.0 */ public function addReopenedMessage($patternName = '') : bool { - if($ItilFollowup = new ITILFollowup()) { + if($itilFollowup = new ITILFollowup()) { // Check notification config if($this->pattern[FilterPattern::SUPPRESNOTIF]) { $input['_disablenotif'] = true; @@ -184,11 +185,7 @@ public function addReopenedMessage($patternName = '') : bool $input['users_id'] = false; $input['add_reopen'] = 1; $input['itemtype'] = Ticket::class; - if($ItilFollowup->add($input) === false) { - return false; - } else { - return true; - } + return ($itilFollowup->add($input) === false) ? false : true; } } @@ -196,13 +193,13 @@ public function addReopenedMessage($patternName = '') : bool * addSolvedMessage(string patternName = '') : bool - * Adds followup to indicate that ticketfilter updated the status to solved. * - * @param void - * @return bool + * @param void + * @return bool * @since 1.2.0 */ public function addSolvedMessage($patternName = '') : bool { - if($ItilFollowup = new ITILFollowup()) { + if($itilFollowup = new ITILFollowup()) { // Check notification config if($this->pattern[FilterPattern::SUPPRESNOTIF]) { $input['_disablenotif'] = true; @@ -214,21 +211,17 @@ public function addSolvedMessage($patternName = '') : bool $input['add_reopen'] = 1; $input['itemtype'] = Ticket::class; - if($ItilFollowup->add($input) === false) { - return false; - } else { - return true; - } + return ($itilFollowup->add($input) === false) ? false : true; } } /** * processTicket(Ticket ticket) : bool - * Adds a followup based on the passed ticket to the loaded ticket residing in ticketHandler->ticket, - * also performs validations and evaluates if the loaded ticket needs to be reopend, resolved or assets + * also performs validations and evaluates if the loaded ticket needs to be reopend, resolved or assets * should be linked based on the provided pattern configuration in ticketHandler->pattern. * - * @param Ticket $item // Ticket object received from the pre_item_add hook. - * @return bool + * @param Ticket $item // Ticket object received from the pre_item_add hook. + * @return bool * @since 1.1.0 */ public function processTicket(Ticket $item) : bool @@ -236,11 +229,11 @@ public function processTicket(Ticket $item) : bool // Match mailsender with ticket requester. if($this->pattern[FilterPattern::MATCHSOURCE]) { // https://github.com/DonutsNL/ticketfilter/issues/4 - $succesfullUserMatch = false; + $succesfullUserMatch = false; // Get all the users from the ticket Object received from the pre_item_add hook. foreach($item->input['_actors']["requester"] as $null => $mailUser){ - $usersToBeMatched[] = [ + $usersToBeMatched[] = [ 'userId' => $mailUser['items_id'], 'userEmail' => ($mailUser['default_email']) ? $mailUser['default_email'] : $mailUser['alternative_email'] ]; @@ -299,7 +292,7 @@ public function processTicket(Ticket $item) : bool } // Add the followup - if($ItilFollowup = new ITILFollowup()) { + if($itilFollowup = new ITILFollowup()) { // Populate Followup fields $input = $item->input; $input['items_id'] = $this->getId(); @@ -318,20 +311,17 @@ public function processTicket(Ticket $item) : bool unset($input['entities_id']); unset($input['_ruleid']); - if($ItilFollowup->add($input) === false) { - return false; + if($itilFollowup->add($input) === false) { trigger_error('TicketFilter: Unable to add a new followup, database available?', E_USER_WARNING); + return false; } Session::addMessageAfterRedirect(__("New ticket was matched to open ticket: ".$this->getId()." and was added as a followup"), true, INFO); } - // Do we need to link assets to this ticket? - // Future feature. - // Assess the title and solve the ticket if matched with the solved match string. - if($this->pattern[FilterPattern::SOLVEDMATCHSTR]) { - if(!empty($this->pattern[FilterPattern::SOLVEDMATCHSTR])) { + if($this->pattern[FilterPattern::SOLVEDMATCHSTR] && + !empty($this->pattern[FilterPattern::SOLVEDMATCHSTR])) { $p = html_entity_decode($this->pattern[FilterPattern::SOLVEDMATCHSTR]); // Perform the search if(preg_match_all("$p", $item->input['name'], $matchArray)) { @@ -349,7 +339,6 @@ public function processTicket(Ticket $item) : bool } else { trigger_error("TicketFilter: PregMatch failed! please review the Solved pattern $p and correct it", E_USER_WARNING); } - } // No solved pattern configured } return true; } else { @@ -359,10 +348,10 @@ public function processTicket(Ticket $item) : bool /** * getTicketUrl(void) : string - - * Returns the url to the ticket loaded by the handler or returns (string) '0' + * Returns the url to the ticket loaded by the handler or returns (string) '0' * no ticket was loaded by initHandler(). * - * @param void + * @param void * @return string Returns human readable ticket status or (string) '0' * @since 1.1.0 */ @@ -401,6 +390,6 @@ public function searchTicketPool(string $searchString) : array ) as $id => $row) { $r[]=$id; } - return $r; + return $r; } -} \ No newline at end of file +} diff --git a/tests/ticketFilterTest.php b/tests/TicketFilterTest.php similarity index 71% rename from tests/ticketFilterTest.php rename to tests/TicketFilterTest.php index e9d6e51..df2de41 100644 --- a/tests/ticketFilterTest.php +++ b/tests/TicketFilterTest.php @@ -1,6 +1,5 @@ assertTrue( method_exists(TicketHandler::class, 'initHandler'), 'Class TicketHandler::class does not have method initHandler' - )){ print("Ok!\n"); } + )){ print "Ok!\n"; } - print("Evaluate getId exists in TicketHandler::class : "); + print "Evaluate getId exists in TicketHandler::class : "; if(!$this->assertTrue( method_exists(TicketHandler::class, 'getId'), 'Class TicketHandler::class does not have method getId' - )){ print("Ok!\n"); } + )){ print "Ok!\n"; } - print("Evaluate TicketHandler->getId returns interger : "); + print "Evaluate TicketHandler->getId returns interger : "; if(!$this->assertTrue(is_int($tf->getId(1)), 'Class TicketHandler::getId(1) did not return integer')){ - print("Ok!\n"); + print "Ok!\n"; }; - print("Evaluate addSolvedMessage exists in TicketHandler::class : "); + print "Evaluate addSolvedMessage exists in TicketHandler::class : "; if(!$this->assertTrue( method_exists(TicketHandler::class, 'addSolvedMessage'), 'Class TicketHandler::class does not have method addSolvedMessage' - )){ print("Ok!\n"); } + )){ print "Ok!\n"; } $this->assertTrue( method_exists(TicketHandler::class, 'addSolvedMessage'), 'Class TicketHandler::class does not have method addSolvedMessage' ); } - - public function testWithoutDelay() - { - $this->assertNull(null); - } } // This plugin uses extensive database functions // therefor we need to modify the classes to allow for more diff --git a/tests/bootstrap.php b/tests/bootstrap.php index 12fdb21..d238606 100644 --- a/tests/bootstrap.php +++ b/tests/bootstrap.php @@ -1,5 +1,5 @@