From f17f22755ca1048ccc43431cd4192a75777339ad Mon Sep 17 00:00:00 2001 From: Laurent Destailleur Date: Mon, 22 Aug 2022 15:06:09 +0200 Subject: [PATCH] Clean code using of oldcopy --- htdocs/contrat/class/contrat.class.php | 6 ++++-- htdocs/core/class/commonobject.class.php | 17 ++++++++++++----- htdocs/fichinter/class/fichinter.class.php | 9 ++++----- htdocs/product/class/product.class.php | 1 + htdocs/product/stock/class/productlot.class.php | 4 +--- htdocs/resource/class/dolresource.class.php | 1 + htdocs/ticket/class/ticket.class.php | 17 +++++++++-------- 7 files changed, 32 insertions(+), 23 deletions(-) diff --git a/htdocs/contrat/class/contrat.class.php b/htdocs/contrat/class/contrat.class.php index 8dbcde19d0d..63bedcfdcbc 100644 --- a/htdocs/contrat/class/contrat.class.php +++ b/htdocs/contrat/class/contrat.class.php @@ -3132,11 +3132,13 @@ class ContratLigne extends CommonObjectLine } } + // $this->oldcopy should have been set by the caller of update (here properties were already modified) + if (empty($this->oldcopy)) { + $this->oldcopy = dol_clone($this); + } $this->db->begin(); - $this->oldcopy = dol_clone($this); - // Update request $sql = "UPDATE ".MAIN_DB_PREFIX."contratdet SET"; $sql .= " fk_contrat=".((int) $this->fk_contrat).","; diff --git a/htdocs/core/class/commonobject.class.php b/htdocs/core/class/commonobject.class.php index 721e23e2803..e587e5bfd1d 100644 --- a/htdocs/core/class/commonobject.class.php +++ b/htdocs/core/class/commonobject.class.php @@ -8918,11 +8918,12 @@ abstract class CommonObject /** - * Function to prepare a part of the query for insert. - * Note $this->${field} are set by the page that make the createCommon or the updateCommon. - * $this->${field} should be a clean value. The page can run + * Function to prepare a part of the query for insert by returning an array with all properties of object. * - * @return array + * Note $this->${field} are set by the page that make the createCommon() or the updateCommon(). + * $this->${field} should be a clean and string value (so date are formated for SQL insert). + * + * @return array Array with all values of each properties to update */ protected function setSaveQuery() { @@ -8950,7 +8951,7 @@ abstract class CommonObject } } elseif ($this->isInt($info) || $this->isFloat($info)) { if ($field == 'entity' && is_null($this->{$field})) { - $queryarray[$field] = $conf->entity; + $queryarray[$field] = ((int) $conf->entity); } else { // $this->{$field} may be null, '', 0, '0', 123, '123' if ((isset($this->{$field}) && $this->{$field} != '') || !empty($info['notnull'])) { @@ -9378,6 +9379,11 @@ abstract class CommonObject $now = dol_now(); + // $this->oldcopy should have been set by the caller of update + //if (empty($this->oldcopy)) { + // $this->oldcopy = dol_clone($this); + //} + $fieldvalues = $this->setSaveQuery(); if (array_key_exists('date_modification', $fieldvalues) && empty($fieldvalues['date_modification'])) { @@ -9423,6 +9429,7 @@ abstract class CommonObject $sql = 'UPDATE '.$this->db->prefix().$this->table_element.' SET '.implode(', ', $tmp).' WHERE rowid='.((int) $this->id); $this->db->begin(); + if (!$error) { $res = $this->db->query($sql); if (!$res) { diff --git a/htdocs/fichinter/class/fichinter.class.php b/htdocs/fichinter/class/fichinter.class.php index b832bceb822..61c127cab9b 100644 --- a/htdocs/fichinter/class/fichinter.class.php +++ b/htdocs/fichinter/class/fichinter.class.php @@ -524,6 +524,8 @@ class Fichinter extends CommonObject dol_syslog(get_class($this)."::setDraft", LOG_DEBUG); + $this->oldcopy = dol_clone($this); + $this->db->begin(); $sql = "UPDATE ".MAIN_DB_PREFIX."fichinter"; @@ -532,10 +534,6 @@ class Fichinter extends CommonObject $resql = $this->db->query($sql); if ($resql) { - if (!$error) { - $this->oldcopy = dol_clone($this); - } - if (!$error) { // Call trigger $result = $this->call_trigger('FICHINTER_UNVALIDATE', $user); @@ -1411,6 +1409,8 @@ class Fichinter extends CommonObject $this->db->begin(); + $this->oldcopy = dol_clone($this); + $sql = "UPDATE ".MAIN_DB_PREFIX.$this->table_element." SET ref_client = ".(empty($ref_client) ? 'NULL' : "'".$this->db->escape($ref_client)."'"); $sql .= " WHERE rowid = ".((int) $this->id); @@ -1422,7 +1422,6 @@ class Fichinter extends CommonObject } if (!$error) { - $this->oldcopy = dol_clone($this); $this->ref_client = $ref_client; } diff --git a/htdocs/product/class/product.class.php b/htdocs/product/class/product.class.php index ed43e792d87..a865a1f2343 100644 --- a/htdocs/product/class/product.class.php +++ b/htdocs/product/class/product.class.php @@ -1050,6 +1050,7 @@ class Product extends CommonObject } if ($result >= 0) { + // $this->oldcopy should have been set by the caller of update (here properties were already modified) if (empty($this->oldcopy)) { $this->oldcopy = dol_clone($this); } diff --git a/htdocs/product/stock/class/productlot.class.php b/htdocs/product/stock/class/productlot.class.php index 3b169055bfa..d2f21ac372e 100644 --- a/htdocs/product/stock/class/productlot.class.php +++ b/htdocs/product/stock/class/productlot.class.php @@ -384,9 +384,7 @@ class Productlot extends CommonObject $this->import_key = trim($this->import_key); } - // Check parameters - // Put here code to add a control on parameters values - + // $this->oldcopy should have been set by the caller of update (here properties were already modified) if (empty($this->oldcopy)) { $this->oldcopy = dol_clone($this); } diff --git a/htdocs/resource/class/dolresource.class.php b/htdocs/resource/class/dolresource.class.php index 2188d3b0a23..46371fead0f 100644 --- a/htdocs/resource/class/dolresource.class.php +++ b/htdocs/resource/class/dolresource.class.php @@ -262,6 +262,7 @@ class Dolresource extends CommonObject $this->country_id = 0; } + // $this->oldcopy should have been set by the caller of update (here properties were already modified) if (empty($this->oldcopy)) { $this->oldcopy = dol_clone($this); } diff --git a/htdocs/ticket/class/ticket.class.php b/htdocs/ticket/class/ticket.class.php index f6a4773c58f..07cdbeac7b3 100644 --- a/htdocs/ticket/class/ticket.class.php +++ b/htdocs/ticket/class/ticket.class.php @@ -870,9 +870,10 @@ class Ticket extends CommonObject global $conf, $langs, $hookmanager; $error = 0; - if (empty($notrigger)) { - $this->oldcopy = dol_clone($this, 1); - } + // $this->oldcopy should have been set by the caller of update (here properties were already modified) + //if (empty($this->oldcopy)) { + // $this->oldcopy = dol_clone($this); + //} // Clean parameters if (isset($this->ref)) { @@ -1463,13 +1464,12 @@ class Ticket extends CommonObject $error = 0; if ($this->statut != self::STATUS_CANCELED) { // no closed - if (empty($notrigger)) { - $this->oldcopy = dol_clone($this, 1); - } + $this->oldcopy = dol_clone($this); + $this->db->begin(); $sql = "UPDATE ".MAIN_DB_PREFIX."ticket"; - $sql .= " SET fk_statut = ".Ticket::STATUS_READ.", date_read='".$this->db->idate(dol_now())."'"; + $sql .= " SET fk_statut = ".Ticket::STATUS_READ.", date_read = '".$this->db->idate(dol_now())."'"; $sql .= " WHERE rowid = ".((int) $this->id); dol_syslog(get_class($this)."::markAsRead"); @@ -1518,10 +1518,11 @@ class Ticket extends CommonObject global $conf, $langs; $error = 0; - $this->db->begin(); $this->oldcopy = dol_clone($this); + $this->db->begin(); + $sql = "UPDATE ".MAIN_DB_PREFIX."ticket"; if ($id_assign_user > 0) { $sql .= " SET fk_user_assign=".((int) $id_assign_user).", fk_statut = ".Ticket::STATUS_ASSIGNED;