fix edit user to log old values and only run if there is any change

admin/inc/class.admin_account.inc.php

@@ -44,7 +44,7 @@ class admin_account
 				$GLOBALS['egw']->acl->check('account_access', 4, 'admin');
 			//error_log(__METHOD__."() contact_id=$content[contact_id], account_id=$content[account_id], deny_edit=".array2string($deny_edit));
 
-			if (!$content['account_id'] && $deny_edit) return;	// no right to add new Api\Accounts, should not happen by AB ACL
+			if (!$content['account_id'] && $deny_edit) return;	// no right to add new accounts, should not happen by AB ACL
 
 			// load our translations
 			Api\Translation::add_app('admin');
@@ -59,7 +59,7 @@ class admin_account
 				}
 				if ($account['account_expires'] == -1) $account['account_expires'] = '';
 				unset($account['account_pwd']);	// do NOT send to client
-				$account['memberships'] = array_keys($account['memberships']);
+				$account['account_groups'] = array_keys($account['memberships']);
 				$acl = new Acl($content['account_id']);
 				$acl->read_repository();
 				$account['anonymous'] = $acl->check('anonymous', 1, 'phpgwapi');
@@ -76,7 +76,7 @@ class admin_account
 			{
 				$account = array(
 					'account_status' => 'A',
-					'memberships' => array(),
+					'account_groups' => array(),
 					'anonymous' => false,
 					'changepassword' => true,	//old default: (bool)$GLOBALS['egw_info']['server']['change_pwd_every_x_days'],
 					'mustchangepassword' => false,
@@ -111,9 +111,12 @@ class admin_account
 				'label' => 'Account',
 				'data' => $account,
 				// save old values to only trigger save, if one of the following values change (contact data get saved anyway)
-				'preserve' => array('old_account' => array_intersect_key($account, array_flip(array(
+				'preserve' => empty($content['id']) ? array() :
-					'account_lid', 'account_status', 'memberships', 'anonymous', 'changepassword',
+					array('old_account' => array_intersect_key($account, array_flip(array(
-					'mustchangepassword', 'account_primary_group', 'homedirectory', 'loginshell')))),
+						'account_lid', 'account_status', 'account_groups', 'anonymous', 'changepassword',
+						'mustchangepassword', 'account_primary_group', 'homedirectory', 'loginshell',
+						'account_expires', 'account_firstname', 'account_lastname', 'account_email'))),
+						'deny_edit' => $deny_edit),
 				'readonlys' => $readonlys,
 				'pre_save_callback' => $deny_edit ? null : 'admin_account::addressbook_pre_save',
 			);
@@ -129,7 +132,15 @@ class admin_account
 	 */
 	public static function addressbook_pre_save(&$content)
 	{
-		if ($content['old_account'] && $content['old_account'] == array_diff_key($content, $content['old_account']))
+		if (!isset($content['mustchangepassword']))
+		{
+			$content['mustchangepassword'] = true;	// was readonly because already set
+		}
+		$content['account_firstname'] = $content['n_given'];
+		$content['account_lastname'] = $content['n_family'];
+		$content['account_email'] = $content['email'];
+		if ($content['deny_edit'] ||
+			$content['old_account'] && !($old = array_diff_assoc($content['old_account'], $content)))
 		{
 			return '';	// no need to save account data, if nothing changed
 		}
@@ -140,9 +151,9 @@ class admin_account
 			'n_given' => 'account_firstname',
 			'n_family' => 'account_lastname',
 			'email' => 'account_email',
-			'memberships' => 'account_groups',
+			'account_groups',
 			// copy following fields to account
-			'account_lid', 'account_id',
+			'account_lid',
 			'changepassword', 'anonymous', 'mustchangepassword',
 			'account_passwd', 'account_passwd_2',
 			'account_primary_group',
@@ -153,6 +164,14 @@ class admin_account
 		{
 			if (is_int($c_name)) $c_name = $a_name;
 
+			// only record real changes
+			if (isset($content['old_account']) &&
+				(!isset($content[$c_name]) && $c_name !== 'account_expires' || // account_expires is not set when empty!
+				$content['old_account'][$a_name] == $content[$c_name]))
+			{
+				continue;	// no change --> no need to log setting it to identical value
+			}
+
 			switch($a_name)
 			{
 				case 'account_expires':
@@ -172,12 +191,12 @@ class admin_account
 			}
 		}
 		// Make sure primary group is in account groups
-		if($account['account_primary_group'] && !array_search($account['account_primary_group'], (array)$account['account_groups']))
+		if ($account['account_primary_group'] && !in_array($account['account_primary_group'], (array)$account['account_groups']))
 		{
 			$account['account_groups'][] = $account['account_primary_group'];
 		}
 
-		$cmd = new admin_cmd_edit_user((int)$content['account_id'], $account);
+		$cmd = new admin_cmd_edit_user((int)$content['account_id'], $account, null, null, $old);
 		$cmd->run();
 
 		Api\Json\Response::get()->call('egw.refresh', '', 'admin', $cmd->account, $content['account_id'] ? 'edit' : 'add');

admin/inc/class.admin_cmd_edit_user.inc.php

@@ -5,9 +5,8 @@
  * @link http://www.egroupware.org
  * @author Ralf Becker <RalfBecker-AT-outdoor-training.de>
  * @package admin
- * @copyright (c) 2007-16 by Ralf Becker <RalfBecker-AT-outdoor-training.de>
+ * @copyright (c) 2007-18 by Ralf Becker <RalfBecker-AT-outdoor-training.de>
  * @license http://opensource.org/licenses/gpl-license.php GPL - GNU General Public License
- * @version $Id$
  */
 
 use EGroupware\Api;
@@ -24,8 +23,9 @@ class admin_cmd_edit_user extends admin_cmd_change_pw
 	 * @param array $set =null array with all data to change
 	 * @param string $password =null password
 	 * @param boolean $run_addaccount_hook =null default run addaccount for new Api\Accounts and editaccount for existing ones
+	 * @param array $old =null array to log old values of $set
 	 */
-	function __construct($account,$set=null,$password=null,$run_addaccount_hook=null)
+	function __construct($account, $set=null, $password=null, $run_addaccount_hook=null, array $old=null)
 	{
 		if (!is_array($account))
 		{
@@ -35,6 +35,7 @@ class admin_cmd_edit_user extends admin_cmd_change_pw
 				'set' => $set,
 				'password' => is_null($password) ? $set['account_passwd'] : $password,
 				'run_addaccount_hook' => $run_addaccount_hook,
+				'old' => $old,
 			);
 		}
 		admin_cmd::__construct($account);

admin/inc/class.admin_config.inc.php

@@ -5,9 +5,8 @@
  * @link http://www.egroupware.org
  * @author Ralf Becker <rb@stylite.de>
  * @package admin
- * @copyright (c) 2016 by Ralf Becker <rb@stylite.de>
+ * @copyright (c) 2016-18 by Ralf Becker <rb@stylite.de>
  * @license http://opensource.org/licenses/gpl-license.php GPL - GNU General Public License
- * @version $Id$
  */
 
 use EGroupware\Api;
@@ -65,8 +64,8 @@ class admin_config
 			$base = $GLOBALS['egw_info']['server']['files_dir'].'/anon-images';
 			foreach ($files as $file)
 			{
-				$parts = explode('anon_images.php?src=', $file);
+				$parts2 = explode('anon_images.php?src=', $file);
-				$parts = explode('&', $parts[1]);
+				$parts = explode('&', $parts2[1]);
 				$path = $base.'/'.urldecode($parts[0]);
 				if (is_writable(dirname($base)) && file_exists($path))
 				{

admin/templates/default/account.xet

@@ -61,7 +61,7 @@
 				</row>
 				<row>
 					<description value="Groups" for="groups"/>
-					<select-account account_type="groups" id="memberships" multiple="true" class="et2_fullWidth" span="4" tags="true"/>
+					<select-account account_type="groups" id="account_groups" multiple="true" class="et2_fullWidth" span="4" tags="true"/>
 					<description/>
 				</row>
 				<row disabled="!@account_id">

admin/templates/default/config.xet

@@ -2,7 +2,17 @@
 <!DOCTYPE overlay PUBLIC "-//EGroupware GmbH//eTemplate 2//EN" "http://www.egroupware.org/etemplate2.dtd">
 <!-- $Id$ -->
 <overlay>
-	<template id="admin.config" template="" lang="" group="0" version="16.1">
+	<template id="admin.config" template="" lang="" group="0" version="18.1">
+		<tabbox id="tabs" width="100%">
+			<tabs>
+				<tab id="config" label="Configuration"/>
+			</tabs>
+			<tabpanels>
+				<template id="admin.config.config"/>
+			</tabpanels>
+		</tabbox>
+	</template>
+	<template id="admin.config.config" template="" lang="" group="0" version="16.1">
 		<grid width="100%" class="admin-config egwGridView_grid">
 			<columns>
 				<column width="70%"/>
@@ -19,7 +29,7 @@
 				<row>
 					<description value="How should EMail addresses for new users be constructed?" label="%s:"/>
 					<vbox>
-					<select id="newsettings[email_address_format]">
+						<select id="newsettings[email_address_format]">
 							<option value="first-dot-last">{Firstname}.{Lastname}@domain.com</option>
 							<option value="first-last">{Firstname}{Lastname}@domain.com</option>
 							<option value="first-underscore-last">{Firstname}_{Lastname}@domain.com</option>