From 0c0ae1fbee51acf7b18ce74eaf522a7922a97449 Mon Sep 17 00:00:00 2001 From: proess <pascal.roessler@math.uzh.ch> Date: Mon, 20 Mar 2023 12:48:17 +0100 Subject: [PATCH] S15790: implemented 'lockSameOwner' to make dirty record lock easier to work with refs #15790 --- Documentation-develop/CONFIG.md | 108 ++++++++++++++++-- Documentation/Installation.rst | 3 + extension/Classes/Core/AbstractBuildForm.php | 4 + extension/Classes/Core/Constants.php | 7 +- extension/Classes/Core/Form/Dirty.php | 34 ++++-- extension/Classes/Core/Helper/Support.php | 29 +++++ extension/Classes/Core/QuickFormQuery.php | 9 +- extension/Classes/Core/Store/Config.php | 1 + extension/Classes/Sql/qfqDefaultTables.sql | 1 + extension/Tests/Unit/Core/Store/StoreTest.php | 2 +- extension/ext_conf_template.txt | 3 + 11 files changed, 180 insertions(+), 21 deletions(-) diff --git a/Documentation-develop/CONFIG.md b/Documentation-develop/CONFIG.md index 27f440c5b..ab8b3b971 100644 --- a/Documentation-develop/CONFIG.md +++ b/Documentation-develop/CONFIG.md @@ -19,17 +19,109 @@ IMATHUZH\Qfq\Core\Store\Store::fillStoreSystem() IMATHUZH\Qfq\Core\Store\Config::getConfigArray() IMATHUZH\Qfq\Core\Store\Config::readConfig() -To create a new QFQ Config option: +How to create a new config option +================================== -ext_conf_template.txt: +To create a new config option, you have to make the changes specified below in the following files. -* create new entry -* set a default value in ext_conf_template.txt +ext_conf_template.txt +--------------------- -config::setDefaults() +The following variables must be set: -* Define defaults if nothing is given +**cat** +(category where the new option will be located, in the extension configuration of your typo3 backend) -DatabaseUpdate=>checkT3QfqConfig(): +**type** (datatype of the config option) -* In case existing installations should get a new default during QFQ update. +possible datatypes: +* boolean (checkbox) +* color (colorpicker) +* int (integer value) +* int+ (positive integer value) +* integer (integer value) +* offset (offset) +* options (option select) +```type=options[label1=value1,label2=value2,value3];``` +* small (small text field) +* string (text field) +* user (user function) +```type=user[Vendor\MyExtensionKey\ViewHelpers\MyConfigurationClass->render];``` +* wrap (wrap field) + +**label** (title and description of the config option, split by ":") + +**myVariable** (name the variable of the config option and assign a default value) + +**Example** + +``` +# cat=config/config; type=boolean; label=MyLabel:Description +myVariable = value1 +``` + +Constants.php +------------- + +Best practice would be defining constants with the name of your variable, +since this name should never be changed. + +``` +const SYSTEM_MY_VARIABLE = 'myVariable'; +const F_MY_VARIABLE = 'SYSTEM_MY_VARIABLE'; +const FE_MY_VARIABLE = 'SYSTEM_MY_VARIABLE'; +``` + + +Config.php +--------------------- + +In the function **setDefaults()** a default value should be set. +</br>Important in case of new variables: new variables do not exist in QFQ extension config and do not get the default defined in ext_conf_template.txt + +``` +default = [ + ... + SYSTEM_MY_VARIABLE => 'true', + ... +]; +``` + +Support.php +---------- + +To set the default value of a FormElement you can use the **setFeDefaults()** function. +</br>Wich provides the default value for the FormElement using the **system store**. +</br>The **system store** contains all the variables defined in the typo3 extension configuration. + +``` +self::setIfNotSet($formElement, FE_MY_VARIABLE, $store->getVar(SYSTEM_MY_VARIABLE, STORE_SYSTEM)); +``` + +StoreTest.php +------------- + +The expected default value must be specified in the **testConfigIniDefaultValues()** function so that the unit test can run without errors. + +``` +$expect = [ + ... + SYSTEM_LOCK_SAME_OWNER => 'true', + ... +]; +``` + +How to handle variables +-------------------------------------- + +Here is an example on how you would go about handling variables that are defined on all levels (SYSTEM -> Form -> FormElement) + +``` +$myVar = $store->getVar(SYSTEM_MY_VARIABLE, STORE_SYSTEM); +if(isset($this->formSpec[F_MY_VARIABLE])){ + $myVar = $this->formSpec[F_MY_VARIABLE]; +} +if(isset($this->formElement[FE_MY_VARIABLE])){ + $myVar = $this->formElement[FE_MY_VARIABLE]; +} +``` diff --git a/Documentation/Installation.rst b/Documentation/Installation.rst index a0687275f..0a884a863 100644 --- a/Documentation/Installation.rst +++ b/Documentation/Installation.rst @@ -626,6 +626,9 @@ Extension Manager: QFQ Configuration +-----------------------------------+-------------------------------------------------------+----------------------------------------------------------------------------+ | **Form-Config** | +-----------------------------------+-------------------------------------------------------+----------------------------------------------------------------------------+ +| lockSameOwner | true | true|false: Should the same user that triggered the 'Dirty Record' | +| | | be able to remove it. | ++-----------------------------------+-------------------------------------------------------+----------------------------------------------------------------------------+ | recordLockTimeoutSeconds | 900 | Timeout for record locking. After this time, a record will be replaced. | +-----------------------------------+-------------------------------------------------------+----------------------------------------------------------------------------+ | enterAsSubmit | enterAsSubmit = 1 | 0: off, 1: Pressing *enter* in a form means *save* and *close*. | diff --git a/extension/Classes/Core/AbstractBuildForm.php b/extension/Classes/Core/AbstractBuildForm.php index c936c6096..aaad424d3 100644 --- a/extension/Classes/Core/AbstractBuildForm.php +++ b/extension/Classes/Core/AbstractBuildForm.php @@ -1162,6 +1162,9 @@ abstract class AbstractBuildForm { */ public function buildHiddenSip(array &$json) { + if(isset($this->formSpec[F_LOCK_SAME_OWNER])){ + $this->store::setVar(F_LOCK_SAME_OWNER,$this->formSpec[F_LOCK_SAME_OWNER],STORE_SIP); + } $sipArray = $this->store->getStore(STORE_SIP); // do not include system vars @@ -4205,6 +4208,7 @@ EOT; return $tgMax; } + /** * @param array $formElement * @param $htmlElement diff --git a/extension/Classes/Core/Constants.php b/extension/Classes/Core/Constants.php index 5adc69dda..520bbae11 100644 --- a/extension/Classes/Core/Constants.php +++ b/extension/Classes/Core/Constants.php @@ -569,6 +569,8 @@ const SYSTEM_FORM_SUBMIT_LOG_MODE = 'formSubmitLogMode'; const FORM_SUBMIT_LOG_MODE_ALL = 'all'; const FORM_SUBMIT_LOG_MODE_NONE = 'none'; +const SYSTEM_LOCK_SAME_OWNER = 'lockSameOwner'; + const SYSTEM_DATE_FORMAT = 'dateFormat'; const SYSTEM_DATE_TIME_PICKER_TYPE = 'dateTimePickerType'; const SYSTEM_REDIRECT_ALL_MAIL_TO = 'redirectAllMailTo'; @@ -815,6 +817,7 @@ const SIP_EXCLUDE_TYPE = 'type'; const SIP_EXCLUDE_L = 'L'; const SIP_EXCLUDE_XDEBUG_SESSION_START = 'XDEBUG_SESSION_START'; + // FURTHER: all extracted params from 'urlparam const ACTION_KEYWORD_SLAVE_ID = 'slaveId'; @@ -1197,6 +1200,8 @@ const F_ORDER_INTERVAL = 'orderInterval'; const F_ORDER_COLUMN = 'orderColumn'; const F_ORDER_COLUMN_NAME = 'ord'; +const F_LOCK_SAME_OWNER = SYSTEM_LOCK_SAME_OWNER; + const F_SHOW_ID_IN_FORM_TITLE = SYSTEM_SHOW_ID_IN_FORM_TITLE; const F_INPUT_CLEAR_ME = SYSTEM_INPUT_CLEAR_ME; const F_MULTI_MSG_NO_RECORD = 'multiMsgNoRecord'; @@ -1640,7 +1645,7 @@ const COLUMN_UID = 'uid'; const COLUMN_HEADER = 'header'; const COLUMN_TITLE = 'title'; const COLUMN_SUBHEADER = 'subheader'; - +const COLUMN_EXPIRE = 'expire'; const INDEX_PHP = 'index.php'; // QuickFormQuery.php diff --git a/extension/Classes/Core/Form/Dirty.php b/extension/Classes/Core/Form/Dirty.php index 457190674..a5694784b 100644 --- a/extension/Classes/Core/Form/Dirty.php +++ b/extension/Classes/Core/Form/Dirty.php @@ -10,6 +10,7 @@ namespace IMATHUZH\Qfq\Core\Form; use IMATHUZH\Qfq\Core\Database\Database; use IMATHUZH\Qfq\Core\Helper\OnArray; +use IMATHUZH\Qfq\Core\Helper\Support; use IMATHUZH\Qfq\Core\Store\Client; use IMATHUZH\Qfq\Core\Store\Session; use IMATHUZH\Qfq\Core\Store\Sip; @@ -233,17 +234,30 @@ class Dirty { */ private function conflict(array $recordDirty, $currentFormDirtyMode, $primaryKey) { $status = API_ANSWER_STATUS_CONFLICT; - $at = "at " . $recordDirty[COLUMN_CREATED] . " from " . $recordDirty[DIRTY_REMOTE_ADDRESS]; - + $until = "until " . $recordDirty[COLUMN_EXPIRE] . '.'; // Compare modified timestamp if ($this->isRecordModified($recordDirty[DIRTY_TABLE_NAME], $primaryKey, $recordDirty[DIRTY_RECORD_ID], $recordDirty[DIRTY_RECORD_HASH_MD5], $dummy)) { return [API_STATUS => API_ANSWER_STATUS_CONFLICT, API_MESSAGE => 'The record has been modified in the meantime. Please reload the form, edit and save again. [2]']; } if ($this->client[CLIENT_COOKIE_QFQ] == $recordDirty[DIRTY_QFQ_USER_SESSION_COOKIE]) { - $msg = "The record has already been locked by you (maybe in another browser tab) $at!"; - $status = ($recordDirty[F_DIRTY_MODE] == DIRTY_MODE_EXCLUSIVE) ? API_ANSWER_STATUS_CONFLICT : API_ANSWER_STATUS_CONFLICT_ALLOW_FORCE; + $lockSameOwner = (bool) $this->store::getVar(SYSTEM_LOCK_SAME_OWNER,STORE_SYSTEM); + if(is_string($this->store::getVar(SYSTEM_LOCK_SAME_OWNER,STORE_SIP))) { + $lockSameOwnerSip = filter_var($this->store::getVar(SYSTEM_LOCK_SAME_OWNER,STORE_SIP),FILTER_VALIDATE_BOOLEAN); + } + + if(isset($lockSameOwnerSip)) { + $lockSameOwner = $lockSameOwnerSip; + } + + if(!$lockSameOwner){ + $msg = "The record has already been locked by you but it will be skipped."; + $status = API_ANSWER_STATUS_CONFLICT_ALLOW_FORCE; + } else{ + $msg = "The record has already been locked by you (maybe in another browser tab) $until"; + $status = ($recordDirty[F_DIRTY_MODE] == DIRTY_MODE_EXCLUSIVE) ? API_ANSWER_STATUS_CONFLICT : API_ANSWER_STATUS_CONFLICT_ALLOW_FORCE; + } } else { if (empty($recordDirty[DIRTY_FE_USER])) { @@ -252,7 +266,7 @@ class Dirty { $msgUser = "user '" . $recordDirty[DIRTY_FE_USER] . "'"; } - $msg = "The record has already been locked by $msgUser at $at."; + $msg = "The record has already been locked by $msgUser $until ."; // Mandatory lock on Record or current Form? if ($recordDirty[F_DIRTY_MODE] == DIRTY_MODE_EXCLUSIVE || $currentFormDirtyMode == DIRTY_MODE_EXCLUSIVE) { @@ -292,10 +306,10 @@ class Dirty { # Dirty workaround: setting the 'expired timestamp' minus 1 second guarantees that the client ask for relock always if the timeout is expired. $expire = date('Y-m-d H:i:s', strtotime("+" . $tableVars[F_RECORD_LOCK_TIMEOUT_SECONDS] - 1 . " seconds")); // Write 'dirty' record - $this->dbArray[$this->dbIndexQfq]->sql("INSERT INTO `Dirty` (`sip`, `tableName`, `recordId`, `expire`, `recordHashMd5`, `tabUniqId`, `feUser`, `qfqUserSessionCookie`, `dirtyMode`, `remoteAddress`, `created`) " . - "VALUES ( ?,?,?,?,?,?,?,?,?,?,? )", ROW_REGULAR, + $this->dbArray[$this->dbIndexQfq]->sql("INSERT INTO `Dirty` (`sip`, `tableName`, `recordId`, `expire`, `recordHashMd5`, `tabUniqId`, `feUser`, `qfqUserSessionCookie`, `dirtyMode`, `remoteAddress`, `browser`, `created`) " . + "VALUES ( ?,?,?,?,?,?,?,?,?,?,?,? )", ROW_REGULAR, [$s, $tableName, $recordId, $expire, $recordHashMd5, $tabUniqId, $feUser, $this->client[CLIENT_COOKIE_QFQ], $formDirtyMode, - $this->client[CLIENT_REMOTE_ADDRESS], date('YmdHis')]); + $this->client[CLIENT_REMOTE_ADDRESS], Support::getBrowser(),date('YmdHis')]); return [API_STATUS => API_ANSWER_STATUS_SUCCESS, API_MESSAGE => '', API_LOCK_TIMEOUT => $tableVars[F_RECORD_LOCK_TIMEOUT_SECONDS]]; @@ -371,8 +385,8 @@ class Dirty { } else { $msgUser = (empty($recordDirty[DIRTY_FE_USER])) ? "another user" : "user '" . $recordDirty[DIRTY_FE_USER] . "'"; } - $msgAt = "at " . $recordDirty[COLUMN_CREATED] . " from " . $recordDirty[DIRTY_REMOTE_ADDRESS]; - $msg = "The record has been locked by $msgUser $msgAt"; + $until = "until " . $recordDirty[COLUMN_EXPIRE]; + $msg = "The record has been locked by $msgUser $until ." ; // Is the dirtyRecord mine? if ($recordDirty[DIRTY_QFQ_USER_SESSION_COOKIE] == $this->client[CLIENT_COOKIE_QFQ]) { diff --git a/extension/Classes/Core/Helper/Support.php b/extension/Classes/Core/Helper/Support.php index 998e341d3..29e6bc05c 100644 --- a/extension/Classes/Core/Helper/Support.php +++ b/extension/Classes/Core/Helper/Support.php @@ -1685,4 +1685,33 @@ class Support { throw new \CodeException("Invalid wget configuration. Maybe wget command is incorrect or not supported.", ERROR_INVALID_WGET_CMD); } } + + /** + * Get name of browser that is being used. + * + * @return string + */ + public static function getBrowser() + { + $user_agent = $_SERVER['HTTP_USER_AGENT']; + $browser = "N/A"; + + $browsers = [ + '/msie/i' => 'Internet explorer', + '/firefox/i' => 'Firefox', + '/safari/i' => 'Safari', + '/chrome/i' => 'Chrome', + '/edge/i' => 'Edge', + '/opera/i' => 'Opera', + '/mobile/i' => 'Mobile browser', + ]; + + foreach ($browsers as $regex => $value) { + if (preg_match($regex, $user_agent)) { + $browser = $value; + } + } + + return $browser; + } } \ No newline at end of file diff --git a/extension/Classes/Core/QuickFormQuery.php b/extension/Classes/Core/QuickFormQuery.php index 8db8e248d..2555cfedd 100644 --- a/extension/Classes/Core/QuickFormQuery.php +++ b/extension/Classes/Core/QuickFormQuery.php @@ -580,7 +580,14 @@ class QuickFormQuery { $dirty = new Dirty(false, $this->dbIndexData, $this->dbIndexQfq); $recordDirty = array(); $rcLockFound = $dirty->getCheckDirty($this->formSpec[F_TABLE_NAME], $recordId, $recordDirty, $msg); - if (($rcLockFound == LOCK_FOUND_CONFLICT || $rcLockFound == LOCK_FOUND_OWNER) && $recordDirty[F_DIRTY_MODE] == DIRTY_MODE_EXCLUSIVE) { + + $lockSameOwner = (bool) $this->store::getVar(SYSTEM_LOCK_SAME_OWNER,STORE_SYSTEM); + $lockSameOwnerForm = filter_var($this->formSpec[SYSTEM_LOCK_SAME_OWNER],FILTER_VALIDATE_BOOLEAN); + if(isset($lockSameOwnerForm)) { + $lockSameOwner = $lockSameOwnerForm; + } + + if (($rcLockFound == LOCK_FOUND_CONFLICT || $rcLockFound == LOCK_FOUND_OWNER) && $recordDirty[F_DIRTY_MODE] == DIRTY_MODE_EXCLUSIVE && $lockSameOwner) { $this->formSpec[F_MODE_GLOBAL] = F_MODE_READONLY; } } diff --git a/extension/Classes/Core/Store/Config.php b/extension/Classes/Core/Store/Config.php index f2055b4ab..2740cf2f6 100644 --- a/extension/Classes/Core/Store/Config.php +++ b/extension/Classes/Core/Store/Config.php @@ -445,6 +445,7 @@ class Config { SYSTEM_RENDER => SYSTEM_RENDER_SINGLE, SYSTEM_DATE_FORMAT => 'yyyy-mm-dd', + SYSTEM_LOCK_SAME_OWNER => 'true', SYSTEM_SHOW_DEBUG_INFO => SYSTEM_SHOW_DEBUG_INFO_AUTO, SYSTEM_REPORT_MIN_PHP_VERSION => SYSTEM_REPORT_MIN_PHP_VERSION_AUTO, SYSTEM_MAIL_LOG_PATHFILENAME => '', diff --git a/extension/Classes/Sql/qfqDefaultTables.sql b/extension/Classes/Sql/qfqDefaultTables.sql index a79c046ac..b90f3b192 100644 --- a/extension/Classes/Sql/qfqDefaultTables.sql +++ b/extension/Classes/Sql/qfqDefaultTables.sql @@ -140,6 +140,7 @@ CREATE TABLE IF NOT EXISTS `Dirty` `qfqUserSessionCookie` VARCHAR(255) NOT NULL, `dirtyMode` ENUM ('exclusive', 'advisory', 'none') NOT NULL DEFAULT 'exclusive', `remoteAddress` VARCHAR(45) NOT NULL, + `browser` VARCHAR(32) NOT NULL, `modified` DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP, `created` DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP, diff --git a/extension/Tests/Unit/Core/Store/StoreTest.php b/extension/Tests/Unit/Core/Store/StoreTest.php index 02ad8efd4..9d65bf50a 100644 --- a/extension/Tests/Unit/Core/Store/StoreTest.php +++ b/extension/Tests/Unit/Core/Store/StoreTest.php @@ -355,6 +355,7 @@ class StoreTest extends TestCase { SYSTEM_CMD_PDFTOCAIRO => 'pdftocairo', SYSTEM_CMD_WKHTMLTOPDF => '/opt/wkhtmltox/bin/wkhtmltopdf', SYSTEM_DATE_FORMAT => 'yyyy-mm-dd', + SYSTEM_LOCK_SAME_OWNER => 'true', SYSTEM_THROW_GENERAL_ERROR => 'no', SYSTEM_SQL_LOG_MODE => SQL_LOG_MODE_MODIFY, SYSTEM_SHOW_DEBUG_INFO => SYSTEM_SHOW_DEBUG_INFO_AUTO, @@ -440,7 +441,6 @@ class StoreTest extends TestCase { // SYSTEM_DO_NOT_LOG_COLUMN => SYSTEM_DO_NOT_LOG_COLUMN_DEFAULT, SYSTEM_PROTECTED_FOLDER_CHECK => 1, SYSTEM_CMD_WGET => 'wget >/dev/null 2>&1' - ]; $body = json_encode([ diff --git a/extension/ext_conf_template.txt b/extension/ext_conf_template.txt index b0c6998e0..a47fb6d64 100644 --- a/extension/ext_conf_template.txt +++ b/extension/ext_conf_template.txt @@ -10,6 +10,9 @@ baseUrl = # cat=config/date; type=string; label=Date format:Default is 'dd.mm.yyyy'. Possible options: 'yyyy-mm-dd', 'dd.mm.yyyy' dateFormat = dd.mm.yyyy +# cat=form-config/config; type=boolean; label=Lock Same Owner:Options: a) 'true' (default): The user needs to wait for the "Dirty Record Lock" to expire or delete the dirty record directly in the database. b) 'false': The "Dirty Record Lock" can be removed when triggered by the same user. +lockSameOwner = true + # cat=config/config; type=boolean; label=Show edit inline reports. In the frontend, for every QFQ Report record an edit symbol is shown. Click on it will open a window to edit the QFQ report. editInlineReports = 1 -- GitLab