From 14847800d43d82a34492e9642fd2b1f4d04ea3ef Mon Sep 17 00:00:00 2001 From: andrefatchip Date: Thu, 12 Mar 2020 11:00:54 +0100 Subject: [PATCH 1/3] OX6-47: Added config validation --- .../controllers/admin/fcpayone_main.php | 84 +++++++++++++++++++ application/views/admin/de/fcPayOne_lang.php | 1 + application/views/admin/en/fcPayOne_lang.php | 2 + application/views/admin/tpl/fcpayone_main.tpl | 11 ++- extend/application/models/fcPayOneOrder.php | 5 +- 5 files changed, 98 insertions(+), 5 deletions(-) diff --git a/application/controllers/admin/fcpayone_main.php b/application/controllers/admin/fcpayone_main.php index cf59b083..e2293239 100755 --- a/application/controllers/admin/fcpayone_main.php +++ b/application/controllers/admin/fcpayone_main.php @@ -64,6 +64,12 @@ class fcpayone_main extends fcpayone_admindetails */ protected $_aCountryList = array(); + /** + * List of config errors encountered + * @var array + */ + protected $_aConfErrors = null; + /** * Set of default config strings * @@ -309,6 +315,12 @@ public function fcpoGetCountryList() */ public function save() { + $blValid = $this->_fcpoValidateData(); + + if (!$blValid) { + return; + } + $oConfig = $this->_oFcpoHelper->fcpoGetConfig(); $aConfBools = $this->_oFcpoHelper->fcpoGetRequestParameter("confbools"); $aConfStrs = $this->_oFcpoHelper->fcpoGetRequestParameter("confstrs"); @@ -359,6 +371,78 @@ public function save() $this->_fcpoLoadConfigs($sOxid); } + /** + * Returns collected errors + * + * @param void + * @return mixed array|false + */ + public function fcpoGetConfigErrors() + { + if (!is_array($this->_aConfErrors)) { + return false; + } + + return $this->_aConfErrors; + } + + /** + * Validation of entered configuration values + * + * @param void + * @return bool + */ + protected function _fcpoValidateData() + { + $blValidAccountData = $this->_fcpoValidateAccountData(); + + $blValid = ( + $blValidAccountData + ); + + return $blValid; + } + + /** + * Checks accountdata section on errors + * + * @param void + * @return bool + */ + protected function _fcpoValidateAccountData() + { + $aConfStrs = $this->_oFcpoHelper->fcpoGetRequestParameter("confstrs"); + + $blValidPrefix = is_numeric($aConfStrs['sFCPORefPrefix']); + + if (!$blValidPrefix) { + $this->_fcpoAddConfigError('FCPO_CONFERROR_PREFIX_NUMERIC'); + } + + $blValid = ( + $blValidPrefix + ); + + return $blValid; + } + + /** + * Adding a detected configuration error + * + * @param $sTranslationString + * @return void + */ + protected function _fcpoAddConfigError($sTranslationString) + { + $oLang = $this->_oFcpoHelper->fcpoGetLang(); + $sMessage = $oLang->translateString($sTranslationString); + + if (!is_array($this->_aConfErrors)) { + $this->_aConfErrors = array(); + } + $this->_aConfErrors[] = $sMessage; + } + /** * Loads list of countries * diff --git a/application/views/admin/de/fcPayOne_lang.php b/application/views/admin/de/fcPayOne_lang.php index 7687268e..37f30452 100755 --- a/application/views/admin/de/fcPayOne_lang.php +++ b/application/views/admin/de/fcPayOne_lang.php @@ -483,6 +483,7 @@ 'FCPO_EMAIL_IBAN' => 'IBAN:', 'FCPO_EMAIL_CLEARING_BODY_THANKYOU' => 'Vielen Dank, dein %SHOPNAME%-Team', 'FCPO_EMAIL_USAGE' => 'Verwendungszweck', + 'FCPO_CONFERROR_PREFIX_NUMERIC' => 'Der optionale Referenznummernpräfix muss numerisch sein.', ); /* diff --git a/application/views/admin/en/fcPayOne_lang.php b/application/views/admin/en/fcPayOne_lang.php index f68656cd..f60a0fdd 100644 --- a/application/views/admin/en/fcPayOne_lang.php +++ b/application/views/admin/en/fcPayOne_lang.php @@ -492,6 +492,8 @@ 'FCPO_EMAIL_IBAN' => 'IBAN:', 'FCPO_EMAIL_CLEARING_BODY_THANKYOU' => 'Thank you, your %SHOPNAME%-Team', 'FCPO_EMAIL_USAGE' => 'Usage', +'FCPO_CONFERROR_PREFIX_NUMERIC' => 'The optional reference prefix must be numeric.', + ); /* diff --git a/application/views/admin/tpl/fcpayone_main.tpl b/application/views/admin/tpl/fcpayone_main.tpl index 86f25d73..d930f468 100755 --- a/application/views/admin/tpl/fcpayone_main.tpl +++ b/application/views/admin/tpl/fcpayone_main.tpl @@ -18,6 +18,8 @@ + +
[{$oViewConf->getHiddenSid()}] @@ -27,7 +29,14 @@ [{oxmultilang ident="FCPO_MAIN_CONFIG_INFOTEXT"}]

[{oxmultilang ident="FCPO_MODULE_VERSION"}] [{$oView->fcpoGetModuleVersion()}]

- + + [{if $oView->fcpoGetConfigErrors()}] + [{foreach from=$oView->fcpoGetConfigErrors() item='sErrorMessage'}] +
+ [{$sErrorMessage}] +
+ [{/foreach}] + [{/if}]
[{oxmultilang ident="FCPO_CONFIG_GROUP_CONN"}] diff --git a/extend/application/models/fcPayOneOrder.php b/extend/application/models/fcPayOneOrder.php index a9ed04e0..b318a9ee 100755 --- a/extend/application/models/fcPayOneOrder.php +++ b/extend/application/models/fcPayOneOrder.php @@ -1161,10 +1161,7 @@ protected function _fcpoGetResponseQuery() {$sAnd} ) "; -$sPath = getShopBasePath()."/log/andre.log"; -$oFile = fopen($sPath, 'a'); -fwrite($oFile, $sQuery."\n"); -fclose($oFile); + return $sQuery; } From 3ce1cfe24b70eb5b3a8c423c7ebec6b2e94af3dc Mon Sep 17 00:00:00 2001 From: andrefatchip Date: Thu, 12 Mar 2020 11:09:38 +0100 Subject: [PATCH 2/3] OX6-47: Fixed unit test --- .../controllers/admin/fcpayone_mainTest.php | 22 ++++++++++--------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/tests/unit/fcPayOne/application/controllers/admin/fcpayone_mainTest.php b/tests/unit/fcPayOne/application/controllers/admin/fcpayone_mainTest.php index 589c9138..53fd3a24 100755 --- a/tests/unit/fcPayOne/application/controllers/admin/fcpayone_mainTest.php +++ b/tests/unit/fcPayOne/application/controllers/admin/fcpayone_mainTest.php @@ -239,16 +239,17 @@ public function test_Save_Coverage() { $oTestObject = $this->getMock( 'fcpayone_main', array( - '_fcpoCheckAndAddStoreId', - '_fcpoCheckAndAddCampaign', - '_fcpoCheckAndAddLogos', - '_fcpoInsertStoreIds', - '_fcpoInsertCampaigns', - '_fcpoCheckRequestAmazonPayConfiguration', - '_handlePayPalExpressLogos', - '_fcpoInsertProfiles', - '_fcpoCheckAndAddRatePayProfile', - '_fcpoLoadConfigs', + '_fcpoCheckAndAddStoreId', + '_fcpoCheckAndAddCampaign', + '_fcpoCheckAndAddLogos', + '_fcpoInsertStoreIds', + '_fcpoInsertCampaigns', + '_fcpoCheckRequestAmazonPayConfiguration', + '_handlePayPalExpressLogos', + '_fcpoInsertProfiles', + '_fcpoCheckAndAddRatePayProfile', + '_fcpoLoadConfigs', + '_fcpoValidateData', ) ); $oTestObject->method('_fcpoCheckAndAddStoreId')->will($this->returnValue(null)); @@ -261,6 +262,7 @@ public function test_Save_Coverage() $oTestObject->method('_fcpoInsertProfiles')->will($this->returnValue(null)); $oTestObject->method('_fcpoCheckAndAddRatePayProfile')->will($this->returnValue(null)); $oTestObject->method('_fcpoLoadConfigs')->will($this->returnValue(null)); + $oTestObject->method('_fcpoValidateData')->will($this->returnValue(true)); $oMockConfig = $this->getMockBuilder('oxConfig')->disableOriginalConstructor()->getMock(); $oMockConfig->expects($this->any())->method('saveShopConfVar')->will($this->returnValue(true)); From 05ba6f1d648f442263a8c5f49d37eeec61ffe17a Mon Sep 17 00:00:00 2001 From: andrefatchip Date: Thu, 12 Mar 2020 15:41:43 +0100 Subject: [PATCH 3/3] OX6-47: Added reference type change to be compatible with alphanumeric prefix --- application/controllers/admin/fcpayone_main.php | 14 +------------- application/views/admin/de/fcPayOne_lang.php | 1 - application/views/admin/en/fcPayOne_lang.php | 2 -- core/fcpayone_events.php | 11 ++++++++--- 4 files changed, 9 insertions(+), 19 deletions(-) diff --git a/application/controllers/admin/fcpayone_main.php b/application/controllers/admin/fcpayone_main.php index e2293239..ee1d0306 100755 --- a/application/controllers/admin/fcpayone_main.php +++ b/application/controllers/admin/fcpayone_main.php @@ -411,19 +411,7 @@ protected function _fcpoValidateData() */ protected function _fcpoValidateAccountData() { - $aConfStrs = $this->_oFcpoHelper->fcpoGetRequestParameter("confstrs"); - - $blValidPrefix = is_numeric($aConfStrs['sFCPORefPrefix']); - - if (!$blValidPrefix) { - $this->_fcpoAddConfigError('FCPO_CONFERROR_PREFIX_NUMERIC'); - } - - $blValid = ( - $blValidPrefix - ); - - return $blValid; + return true; } /** diff --git a/application/views/admin/de/fcPayOne_lang.php b/application/views/admin/de/fcPayOne_lang.php index 37f30452..7687268e 100755 --- a/application/views/admin/de/fcPayOne_lang.php +++ b/application/views/admin/de/fcPayOne_lang.php @@ -483,7 +483,6 @@ 'FCPO_EMAIL_IBAN' => 'IBAN:', 'FCPO_EMAIL_CLEARING_BODY_THANKYOU' => 'Vielen Dank, dein %SHOPNAME%-Team', 'FCPO_EMAIL_USAGE' => 'Verwendungszweck', - 'FCPO_CONFERROR_PREFIX_NUMERIC' => 'Der optionale Referenznummernpräfix muss numerisch sein.', ); /* diff --git a/application/views/admin/en/fcPayOne_lang.php b/application/views/admin/en/fcPayOne_lang.php index f60a0fdd..f68656cd 100644 --- a/application/views/admin/en/fcPayOne_lang.php +++ b/application/views/admin/en/fcPayOne_lang.php @@ -492,8 +492,6 @@ 'FCPO_EMAIL_IBAN' => 'IBAN:', 'FCPO_EMAIL_CLEARING_BODY_THANKYOU' => 'Thank you, your %SHOPNAME%-Team', 'FCPO_EMAIL_USAGE' => 'Usage', -'FCPO_CONFERROR_PREFIX_NUMERIC' => 'The optional reference prefix must be numeric.', - ); /* diff --git a/core/fcpayone_events.php b/core/fcpayone_events.php index 8772621f..d96677b9 100755 --- a/core/fcpayone_events.php +++ b/core/fcpayone_events.php @@ -550,7 +550,7 @@ public static function addDatabaseStructure() self::changeColumnTypeIfWrong('fcpotransactionstatus', 'FCPO_USERID', 'varchar(32)', self::$sQueryChangeToVarchar1); self::changeColumnTypeIfWrong('fcpotransactionstatus', 'FCPO_TXID', 'varchar(32)', self::$sQueryChangeToVarchar2); - self::changeColumnTypeIfWrong('fcporequestlog', 'FCPO_REFNR', 'varchar(32)', self::$sQueryChangeFcporequestlog); + self::changeColumnTypeIfWrong('fcporequestlog', 'FCPO_REFNR', 'int(11)', self::$sQueryChangeFcporequestlog); self::changeColumnTypeIfWrong('oxorder', 'FCPOREFNR', 'varchar(32)', self::$sQueryChangeRefNrToVarchar); self::dropIndexIfExists('fcporefnr', 'FCPO_REFNR'); @@ -642,9 +642,14 @@ public static function insertRowIfNotExists($sTableName, $aKeyValue, $sQuery) */ public static function changeColumnTypeIfWrong($sTableName, $sColumnName, $sExpectedType, $sQuery) { - if (oxDb::getDb()->getOne("SHOW COLUMNS FROM {$sTableName} WHERE FIELD = '{$sColumnName}' AND TYPE = '{$sExpectedType}'")) { + $sCheckQuery = " + SHOW COLUMNS + FROM {$sTableName} + WHERE FIELD = '{$sColumnName}' + AND TYPE = '{$sExpectedType}' + "; + if (oxDb::getDb()->getOne($sCheckQuery)) { oxDb::getDb()->Execute($sQuery); - // echo 'In Tabelle '.$sTableName.' Spalte '.$sColumnName.' auf Typ '.$sExpectedType.' umgestellt.
'; return true; } return false;