From 145f49d247968d4416a5d0ce1044ea826fbb7b9a Mon Sep 17 00:00:00 2001 From: objecttothis Date: Wed, 20 May 2020 13:48:24 +0400 Subject: [PATCH 1/2] Code Review Changes - Refactored code to include constants - Fixed comment formatting issues --- application/controllers/Config.php | 56 +++++++++++++++--------------- application/models/Attribute.php | 4 +-- 2 files changed, 30 insertions(+), 30 deletions(-) diff --git a/application/controllers/Config.php b/application/controllers/Config.php index bdea8ca0f..10814f4b0 100644 --- a/application/controllers/Config.php +++ b/application/controllers/Config.php @@ -62,12 +62,12 @@ class Config extends Secure_Controller } elseif($fileinfo->getBasename() == 'bower.LICENSES') { - // set a flag to indicate that the JS Plugin bower.LICENSES file is available and needs to be attached at the end + // set a flag to indicate that the JS Plugin bower.LICENSES file is available and needs to be attached at the end $bower = TRUE; } elseif($fileinfo->getBasename() == 'composer.LICENSES') { - // set a flag to indicate that the composer.LICENSES file is available and needs to be attached at the end + // set a flag to indicate that the composer.LICENSES file is available and needs to be attached at the end $composer = TRUE; } } @@ -125,7 +125,7 @@ class Config extends Secure_Controller $license[$i]['text'] = $this->xss_clean($license[$i]['text']); } - // attach the licenses from the LICENSES file generated by bower + // attach the licenses from the LICENSES file generated by bower if($bower) { ++$i; @@ -177,7 +177,7 @@ class Config extends Secure_Controller { $themes = array(); - // read all themes in the dist folder + // read all themes in the dist folder $dir = new DirectoryIterator('dist/bootswatch'); foreach($dir as $dirinfo) @@ -213,10 +213,10 @@ class Config extends Secure_Controller $data = $this->xss_clean($data); - // load all the license statements, they are already XSS cleaned in the private function + // load all the license statements, they are already XSS cleaned in the private function $data['licenses'] = $this->_licenses(); - // load all the themes, already XSS cleaned in the private function + // load all the themes, already XSS cleaned in the private function $data['themes'] = $this->_themes(); $data['mailchimp'] = array(); @@ -231,7 +231,7 @@ class Config extends Secure_Controller $data['mailchimp']['list_id'] = ''; } - // load mailchimp lists associated to the given api key, already XSS cleaned in the private function + // load mailchimp lists associated to the given api key, already XSS cleaned in the private function $data['mailchimp']['lists'] = $this->_mailchimp(); $this->load->view("configs/manage", $data); @@ -305,14 +305,14 @@ class Config extends Secure_Controller $definition_data['definition_name'] = 'ospos_category'; $definition_data['definition_flags'] = 0; $definition_data['definition_type'] = 'DROPDOWN'; - $definition_data['definition_id'] = -1; + $definition_data['definition_id'] = CATEGORY_DEFINITION_ID; $definition_data['deleted'] = 0; - $this->Attribute->save_definition($definition_data, -1); + $this->Attribute->save_definition($definition_data, CATEGORY_DEFINITION_ID); } - else if($batch_save_data['category_dropdown'] == 0) + else if($batch_save_data['category_dropdown'] == NO_DEFINITION_ID) { - $this->Attribute->delete_definition(-1); + $this->Attribute->delete_definition(CATEGORY_DEFINITION_ID); } $result = $this->Appconfig->batch_save($batch_save_data); @@ -569,7 +569,7 @@ class Config extends Secure_Controller { $location_id = preg_replace("/.*?_(\d+)$/", "$1", $key); - // save or update + // save or update $location_data = array('location_name' => $value); if($this->Stock_location->save($location_data, $location_id)) { @@ -580,7 +580,7 @@ class Config extends Secure_Controller } } - // all locations not available in post will be deleted now + // all locations not available in post will be deleted now $deleted_locations = $this->Stock_location->get_all()->result_array(); foreach($deleted_locations as $location => $location_data) @@ -619,7 +619,7 @@ class Config extends Secure_Controller $dinner_table_id = preg_replace("/.*?_(\d+)$/", "$1", $key); $not_to_delete[] = $dinner_table_id; - // save or update + // save or update $table_data = array('name' => $value); if($this->Dinner_table->save($table_data, $dinner_table_id)) { @@ -628,7 +628,7 @@ class Config extends Secure_Controller } } - // all tables not available in post will be deleted now + // all tables not available in post will be deleted now $deleted_tables = $this->Dinner_table->get_all()->result_array(); foreach($deleted_tables as $dinner_tables => $table) @@ -855,7 +855,7 @@ class Config extends Secure_Controller { $this->load->helper('directory'); - // load upload library + // load upload library $config = array('upload_path' => './uploads/', 'allowed_types' => 'gif|jpg|png', 'max_size' => '1024', @@ -872,42 +872,42 @@ class Config extends Secure_Controller { $encryption_key = $this->config->item('encryption_key'); - // check if the encryption_key config item is the default one + // check if the encryption_key config item is the default one if($encryption_key == '' || $encryption_key == 'YOUR KEY') { - // Config path + // Config path $config_path = APPPATH . 'config/config.php'; - // Open the file + // Open the file $config = file_get_contents($config_path); - // $key will be assigned a 32-byte (256-bit) hex-encoded random key + // $key will be assigned a 32-byte (256-bit) hex-encoded random key $key = bin2hex($this->encryption->create_key(32)); - // set the encryption key in the config item + // set the encryption key in the config item $this->config->set_item('encryption_key', $key); - // replace the empty placeholder with a real randomly generated encryption key + // replace the empty placeholder with a real randomly generated encryption key $config = preg_replace("/(.*encryption_key.*)('');/", "$1'$key';", $config); $result = FALSE; - // Chmod the file + // Chmod the file @chmod($config_path, 0777); - // Verify file permissions + // Verify file permissions if(is_writable($config_path)) { - // Write the new config.php file + // Write the new config.php file $handle = @fopen($config_path, 'w+'); - // Write the file + // Write the file $result = (fwrite($handle, $config) === FALSE) ? FALSE : TRUE; fclose($handle); } - // Chmod the file + // Chmod the file @chmod($config_path, 0444); return $result; @@ -937,7 +937,7 @@ class Config extends Secure_Controller { ob_end_clean(); } - + force_download($file_name, $backup); } else diff --git a/application/models/Attribute.php b/application/models/Attribute.php index 5f95f5504..d673c81b5 100644 --- a/application/models/Attribute.php +++ b/application/models/Attribute.php @@ -6,8 +6,8 @@ define('DECIMAL', 'DECIMAL'); define('DATE', 'DATE'); define('TEXT', 'TEXT'); define('CHECKBOX', 'CHECKBOX'); -define(NO_DEFINITION_ID,0); -define(CATEGORY_DEFINITION_ID,-1); +define(NO_DEFINITION_ID, 0); +define(CATEGORY_DEFINITION_ID, -1); const DEFINITION_TYPES = [GROUP, DROPDOWN, DECIMAL, TEXT, DATE, CHECKBOX]; From 89a56820d2dec1211bbad55b8d635e3f401ce950 Mon Sep 17 00:00:00 2001 From: objecttothis Date: Wed, 20 May 2020 13:52:54 +0400 Subject: [PATCH 2/2] Formatting Corrections --- application/controllers/Config.php | 3 +-- application/models/Attribute.php | 20 ++++++++++---------- 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/application/controllers/Config.php b/application/controllers/Config.php index 10814f4b0..719145fd8 100644 --- a/application/controllers/Config.php +++ b/application/controllers/Config.php @@ -33,8 +33,7 @@ class Config extends Secure_Controller $license[$i]['text'] = 'LICENSE file must be in OSPOS license directory. You are not allowed to use OSPOS application until the distribution copy of LICENSE file is present.'; } - // read all the files in the dir license - $dir = new DirectoryIterator('license'); + $dir = new DirectoryIterator('license'); // read all the files in the dir license foreach($dir as $fileinfo) { diff --git a/application/models/Attribute.php b/application/models/Attribute.php index d673c81b5..bff19d90f 100644 --- a/application/models/Attribute.php +++ b/application/models/Attribute.php @@ -96,10 +96,10 @@ class Attribute extends CI_Model } else { - //Get empty base parent object, as $item_id is NOT an item + //Get empty base parent object, as $item_id is NOT an item $item_obj = new stdClass(); - //Get all the fields from items table + //Get all the fields from items table foreach($this->db->list_fields('attribute_definitions') as $field) { $item_obj->$field = ''; @@ -360,7 +360,7 @@ class Attribute extends CI_Model //From DROPDOWN else if($from_type === DROPDOWN) { - //To TEXT + //To TEXT if(in_array($to_type, [TEXT, CHECKBOX], TRUE)) { $this->db->trans_start(); @@ -372,7 +372,7 @@ class Attribute extends CI_Model $this->db->trans_complete(); - //To CHECKBOX + //To CHECKBOX if($to_type === CHECKBOX) { $checkbox_attribute_values = $this->checkbox_attribute_values($definition_id); @@ -391,7 +391,7 @@ class Attribute extends CI_Model } } - //From any other type + //From any other type else { $success = TRUE; @@ -423,10 +423,10 @@ class Attribute extends CI_Model */ public function save_definition(&$definition_data, $definition_id = NO_DEFINITION_ID) { - //Run these queries as a transaction, we want to make sure we do all or nothing + //Run these queries as a transaction, we want to make sure we do all or nothing $this->db->trans_start(); - //Definition doesn't exist + //Definition doesn't exist if($definition_id === NO_DEFINITION_ID || !$this->exists($definition_id)) { if($this->exists($definition_id,TRUE)) @@ -440,7 +440,7 @@ class Attribute extends CI_Model } } - //Definition already exists + //Definition already exists else { $this->db->select('definition_type, definition_name'); @@ -599,7 +599,7 @@ class Attribute extends CI_Model { $this->db->trans_start(); - //New Attribute + //New Attribute if(empty($attribute_id) || empty($item_id)) { if(in_array($definition_type, [TEXT, DROPDOWN, CHECKBOX], TRUE)) @@ -628,7 +628,7 @@ class Attribute extends CI_Model 'definition_id' => $definition_id)); } - //Existing Attribute + //Existing Attribute else { $this->db->where('attribute_id', $attribute_id);