From d1f598b2111de2a7f06c2ca47234452cc5c39b85 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?C=C3=B4me=20Chilliet?= <come@opensides.be>
Date: Tue, 12 Jul 2016 09:02:42 +0200
Subject: [PATCH] Fixes #4893 Refactored tabs saving and checking
---
html/class_passwordRecovery.inc | 9 ++--
include/class_logging.inc | 6 +--
include/class_management.inc | 8 ++-
include/class_plugin.inc | 3 ++
include/functions.inc | 5 +-
.../simpleplugin/class_simpleManagement.inc | 5 +-
include/simpleplugin/class_simplePlugin.inc | 4 +-
include/simpleplugin/class_simpleTabs.inc | 51 ++++++++++++-------
setup/class_setupStep_Migrate.inc | 7 +--
9 files changed, 53 insertions(+), 45 deletions(-)
diff --git a/html/class_passwordRecovery.inc b/html/class_passwordRecovery.inc
index 30a0ab46c..46c52d9dd 100644
--- a/html/class_passwordRecovery.inc
+++ b/html/class_passwordRecovery.inc
@@ -590,14 +590,13 @@ class passwordRecovery extends standAlonePage {
);
/* Is there any problem with entered passwords? */
- $error = $userTabs->check();
- if (!empty($error)) {
- $this->message = $error;
+ $userTabs->save_object();
+ $errors = $userTabs->save();
+ if (!empty($errors)) {
+ $this->message = $errors;
return;
}
- $userTabs->save_object();
- $userTabs->save();
fusiondirectory_log("User ".$this->uid." password has been changed");
/* Send the mail */
$mail_body = sprintf($this->mail2_body, $this->uid);
diff --git a/include/class_logging.inc b/include/class_logging.inc
index 1656b9ea7..0b0906c46 100644
--- a/include/class_logging.inc
+++ b/include/class_logging.inc
@@ -146,10 +146,8 @@ class logging {
$baseObject->fdAuditAttributes = $entry['changes'];
$baseObject->fdAuditResult = $entry['result'];
$baseObject->base = $config->current['BASE'];
- $message = $tabObject->check();
- if (count($message) == 0) {
- $tabObject->save();
- } else {
+ $message = $tabObject->save();
+ if (!empty($message)) {
msg_dialog::displayChecks($message);
}
}
diff --git a/include/class_management.inc b/include/class_management.inc
index 9a1a45a14..abd99c113 100644
--- a/include/class_management.inc
+++ b/include/class_management.inc
@@ -773,12 +773,11 @@ class management
{
if ($this->tabObject instanceOf simpleTabs) {
$this->tabObject->save_object();
- $msgs = $this->tabObject->check();
+ $msgs = $this->tabObject->save();
if (count($msgs)) {
msg_dialog::displayChecks($msgs);
return;
} else {
- $this->tabObject->save();
@DEBUG (DEBUG_TRACE, __LINE__, __FUNCTION__, __FILE__, $this->dns, 'Entry saved!');
$this->remove_lock();
$this->closeDialogs();
@@ -808,13 +807,12 @@ class management
{
if ($this->tabObject instanceOf simpleTabs) {
$this->tabObject->save_object();
- $msgs = $this->tabObject->check();
+ $msgs = $this->tabObject->save();
if (count($msgs)) {
msg_dialog::displayChecks($msgs);
return "";
} else {
- $this->tabObject->save();
- @DEBUG (DEBUG_TRACE, __LINE__, __FUNCTION__, __FILE__, $this->dns, "Modifications applied!");
+ @DEBUG (DEBUG_TRACE, __LINE__, __FUNCTION__, __FILE__, $this->dns, 'Modifications applied!');
$this->tabObject->re_init();
}
}
diff --git a/include/class_plugin.inc b/include/class_plugin.inc
index 92592bf3d..a071b77f6 100644
--- a/include/class_plugin.inc
+++ b/include/class_plugin.inc
@@ -355,7 +355,10 @@ class plugin
}
}
}
+ }
+ function prepareNextCleanup()
+ {
/* Update saved attributes and ensure that next cleanups will be successful too */
foreach ($this->attrs as $name => $value) {
$this->saved_attributes[$name] = $value;
diff --git a/include/functions.inc b/include/functions.inc
index fc9905190..5bbad2c0e 100644
--- a/include/functions.inc
+++ b/include/functions.inc
@@ -2805,12 +2805,11 @@ function change_password ($dn, $password, $hash = "")
$userTab->userPassword,
$userTab->attributesAccess['userPassword']->isLocked()
);
- $error = $userTabs->check();
+ $userTabs->save_object();
+ $error = $userTabs->save();
if (!empty($error)) {
return $error;
}
- $userTabs->save_object();
- $userTabs->save();
return TRUE;
}
diff --git a/include/simpleplugin/class_simpleManagement.inc b/include/simpleplugin/class_simpleManagement.inc
index aa77fb36f..fdd9565b6 100644
--- a/include/simpleplugin/class_simpleManagement.inc
+++ b/include/simpleplugin/class_simpleManagement.inc
@@ -498,15 +498,14 @@ class simpleManagement extends management
if ($cancel) {
$msgs = array();
} else {
- $msgs = $this->tabObject->check();
+ $msgs = $this->tabObject->save();
}
if (count($msgs)) {
msg_dialog::displayChecks($msgs);
return;
} else {
if (!$cancel) {
- $this->tabObject->save();
- @DEBUG (DEBUG_TRACE, __LINE__, __FUNCTION__, __FILE__, $this->dn, "Template applied!");
+ @DEBUG (DEBUG_TRACE, __LINE__, __FUNCTION__, __FILE__, $this->dn, 'Template applied!');
}
del_lock($this->dn);
if (empty($this->dns)) {
diff --git a/include/simpleplugin/class_simplePlugin.inc b/include/simpleplugin/class_simplePlugin.inc
index 23c13ff6b..9af36d81c 100644
--- a/include/simpleplugin/class_simplePlugin.inc
+++ b/include/simpleplugin/class_simplePlugin.inc
@@ -688,6 +688,7 @@ class simplePlugin extends plugin
if (!empty($errors)) {
return $errors;
}
+ $this->prepareNextCleanup();
/* Post hooks and logging */
$this->post_save();
return;
@@ -957,11 +958,10 @@ class simplePlugin extends plugin
$info = "";
if (isset($_POST['edit_finish'])) {
/* Perform checks */
- $message = $tabObject->check();
+ $message = $tabObject->save();
/* No errors, save object */
if (count($message) == 0) {
- $tabObject->save();
del_lock($entry_dn);
session::un_set('edit');
diff --git a/include/simpleplugin/class_simpleTabs.inc b/include/simpleplugin/class_simpleTabs.inc
index ad6569241..0b0329c12 100644
--- a/include/simpleplugin/class_simpleTabs.inc
+++ b/include/simpleplugin/class_simpleTabs.inc
@@ -332,7 +332,7 @@ class simpleTabs
*
* \param boolean $ignore_account false
*/
- function check()
+ protected function check()
{
global $config;
$messages = array();
@@ -353,7 +353,7 @@ class simpleTabs
$current_set = FALSE;
/* Check all plugins */
- foreach ($this->by_object as $key => &$obj) {
+ foreach ($this->by_object as $key => $obj) {
if (($obj->is_account || $obj->ignore_account) && (!$obj->is_template)) {
@DEBUG (DEBUG_TRACE, __LINE__, __FUNCTION__, __FILE__, $key, "Checking");
@@ -373,7 +373,6 @@ class simpleTabs
$obj->pl_notify = FALSE;
}
}
- unset($obj);
return $messages;
}
@@ -385,46 +384,62 @@ class simpleTabs
*/
function save()
{
+ $messages = $this->check();
+ if (!empty($messages)) {
+ return $messages;
+ }
+
$baseobject = $this->getBaseObject();
$new_dn = $baseobject->compute_dn();
@DEBUG (DEBUG_TRACE, __LINE__, __FUNCTION__, __FILE__, $new_dn, "Saving");
+ $errors = array();
+ $iscreation = ($this->dn == 'new');
+
/* Move ? */
if ($this->dn != $new_dn) {
/* Write entry on new 'dn' */
- if ($this->dn != 'new') {
+ if ($creation) {
+ /* use the new one */
+ $this->dn = $new_dn;
+ } else {
if ($baseobject->move($this->dn, $new_dn)) {
$this->dn = $new_dn;
} else {
- msg_dialog::display(_('Error'), sprintf(_('Move from "%s" to "%s" failed'), $this->dn, $new_dn), ERROR_DIALOG);
+ $errors[] = sprintf(_('Move from "%s" to "%s" failed'), $this->dn, $new_dn);
}
- } else {
- /* use the new one */
- $this->dn = $new_dn;
}
}
/* Save all plugins */
- foreach ($this->by_object as $key => &$obj) {
- @DEBUG (DEBUG_TRACE, __LINE__, __FUNCTION__, __FILE__, $key, "Saving");
+ $first = TRUE;
+ foreach ($this->by_object as $key => $obj) {
+ @DEBUG (DEBUG_TRACE, __LINE__, __FUNCTION__, __FILE__, $key, 'Saving');
$obj->dn = $this->dn;
- if (!$obj instanceof plugin && !$obj instanceOf management) {
- trigger_error("Something went wrong while saving ".$obj->dn.". Object class '".get_class($obj)."'.");
+ if (!($obj instanceof plugin) && !($obj instanceOf management)) {
+ trigger_error('Something went wrong while saving '.$obj->dn.'. Class "'.get_class($obj).'".');
} else {
if ($obj->is_account || $obj->ignore_account) {
- if ($obj->save() == 1) {
- return 1;
- }
+ $result = $obj->save();
} else {
- $obj->remove_from_parent();
+ $result = $obj->remove_from_parent();
}
+ if (!empty($result)) {
+ if ($creation && $first)
+ /* If the fail of main tab fails for a creation, cancel the save of other tabs */
+ return $result;
+ }
+ $errors = array_merge($errors, $result);
+ }
+ }
+ if ($first) {
+ $first = FALSE;
}
}
- unset($obj);
- return 0;
+ return $errors;
}
/*!
diff --git a/setup/class_setupStep_Migrate.inc b/setup/class_setupStep_Migrate.inc
index 3847f7518..83c4253be 100644
--- a/setup/class_setupStep_Migrate.inc
+++ b/setup/class_setupStep_Migrate.inc
@@ -929,14 +929,11 @@ class Step_Migrate extends setupStep
$_POST[$tabObject->current.'_posted'] = TRUE;
$_POST['dialog_refresh'] = TRUE;
$tabObject->save_object();
- $errors = $tabObject->check();
+ $errors = $tabObject->save();
if (!empty($errors)) {
- foreach ($errors as $error) {
- msg_dialog::display(_('Error'), $error, ERROR_DIALOG);
- }
+ msg_dialog::displayChecks($errors);
return FALSE;
}
- $tabObject->save();
$admindn = $tabObject->dn;
/* Assigning role */
--
GitLab