* Api: Fix changes in history log had a hash instead of user if the change was made after a share was opened.

api/src/Storage/History.php

@@ -124,11 +124,8 @@ class History
 	{
 		if($new_value != $old_value)
 		{
-			$share_with = '';
+			$share_with = static::get_share_with($this->appname, $record_id);
-			foreach(isset($GLOBALS['egw']->sharing) ? $GLOBALS['egw']->sharing : [] as $token => $share_obj)
+
-			{
-				$share_with .= $share_obj->get_share_with();
-			}
 			$this->db->insert(self::TABLE, array(
 				'history_record_id' => $record_id,
 				'history_appname'   => $this->appname,
@@ -150,11 +147,7 @@ class History
 	{
 		if($new_value != $old_value)
 		{
-			$share_with = '';
+			$share_with = static::get_share_with($appname, $id);
-			foreach(isset($GLOBALS['egw']->sharing) ? $GLOBALS['egw']->sharing : [] as $token => $share_obj)
-			{
-				$share_with .= $share_obj->get_share_with();
-			}
 			$GLOBALS['egw']->db->insert(self::TABLE, array(
 				'history_record_id' => $id,
 				'history_appname'   => $appname,
@@ -169,6 +162,32 @@ class History
 		}
 	}
 
+	/**
+	 * If a record was accessed via a share, we want to record who the entry was shared with, rather than the current
+	 * user.  Since multiple shares can be active at once, and they might not be for the current entry, we check to
+	 * see if the given entry was accessed via a share, and which share was used.
+	 * The share's share_with is recorded into the history for some hope of tracking who made the change.
+	 * share_with is a list of email addresses, and may be empty.
+	 *
+	 * @param $appname
+	 * @param $id
+	 *
+	 * @return string
+	 */
+	static function get_share_with($appname, $id)
+	{
+		$share_with = '';
+		foreach(isset($GLOBALS['egw']->sharing) ? $GLOBALS['egw']->sharing : [] as $token => $share_obj)
+		{
+			// Make sure share is of the correct type to access an entry, and it is the correct entry
+			if($share_obj instanceof Api\Link\Sharing && "$appname::$id" === $share_obj['share_path'])
+			{
+				$share_with .= $share_obj->get_share_with();
+			}
+		}
+		return $share_with;
+	}
+
 	/**
 	 * Search history-log
 	 *
@@ -181,7 +200,10 @@ class History
 	 */
 	function search($filter, $order = 'history_id', $sort = 'DESC', $limit = null)
 	{
-		if (!is_array($filter)) $filter = is_numeric($filter) ? array('history_record_id' => $filter) : array();
+		if(!is_array($filter))
+		{
+			$filter = is_numeric($filter) ? array('history_record_id' => $filter) : array();
+		}
 
 		if(!$order || !preg_match('/^[a-z0-9_]+$/i', $order) || !preg_match('/^(asc|desc)?$/i', $sort))
 		{
@@ -199,14 +221,21 @@ class History
 				unset($filter[$col]);
 			}
 		}
-		if (!isset($filter['history_appname'])) $filter['history_appname'] = $this->appname;
+		if(!isset($filter['history_appname']))
+		{
+			$filter['history_appname'] = $this->appname;
+		}
 
 		// do not try to read all history entries of an app
-		if (!$filter['history_record_id']) return array();
+		if(!$filter['history_record_id'])
+		{
+			return array();
+		}
 
 		$rows = array();
 		foreach($this->db->select(self::TABLE, '*', $filter, __LINE__, __FILE__,
-			isset($limit) ? 0 : false, $orderby, 'phpgwapi', $limit) as $row)
+								  isset($limit) ? 0 : false, $orderby, 'phpgwapi', $limit
+		) as $row)
 		{
 			$row['user_ts'] = $this->db->from_timestamp($row['history_timestamp']) + 3600 * $GLOBALS['egw_info']['user']['preferences']['common']['tz_offset'];
 			$rows[] = Api\Db::strip_array_keys($row, 'history_');
@@ -227,8 +256,10 @@ class History
 		$rows = array();
 		$filter['history_appname'] = $query['appname'];
 		$filter['history_record_id'] = $query['record_id'];
-		if(is_array($query['colfilter'])) {
+		if(is_array($query['colfilter']))
-			foreach($query['colfilter'] as $column => $value) {
+		{
+			foreach($query['colfilter'] as $column => $value)
+			{
 				$filter[$column] = $value;
 			}
 		}
@@ -247,7 +278,8 @@ class History
 				$to_or[] = 'history_status IN (' . implode(',', array_map(function ($str)
 															  {
 																  return $GLOBALS['egw']->db->quote('#' . $str);
-				}, array_keys($cfs))).')';
+															  }, array_keys($cfs))
+					) . ')';
 			}
 			$filter[] = '(' . implode(' OR ', $to_or) . ')';
 		}
@@ -274,7 +306,8 @@ class History
 		{
 			$_query[] = array(
 				'table' => Api\Vfs\Sqlfs\StreamWrapper::TABLE,
-				'cols' =>array('fs_id', 'fs_dir', "'filemanager'",'COALESCE(fs_modifier,fs_creator)',"'~file~'",'fs_name','fs_modified', 'fs_mime', '"" AS share_email'),
+				'cols'  => array('fs_id', 'fs_dir', "'filemanager'", 'COALESCE(fs_modifier,fs_creator)', "'~file~'",
+								 'fs_name', 'fs_modified', 'fs_mime', '"" AS share_email'),
 				'where' => array('fs_dir' => $file['ino'])
 			);
 		}
@@ -303,7 +336,8 @@ class History
 				))
 			{
 				// Larger text stored with full old / new value - calculate diff and just send that
-				$diff = new \Horde_Text_Diff('auto', array(explode("\n",$row['history_old_value']), explode("\n",$row['history_new_value'])));
+				$diff = new \Horde_Text_Diff('auto', array(explode("\n", $row['history_old_value']),
+														   explode("\n", $row['history_new_value'])));
 				$renderer = new \Horde_Text_Diff_Renderer_Unified();
 				$row['history_new_value'] = $renderer->render($diff);
 				$row['history_old_value'] = Tracking::DIFF_MARKER;
@@ -330,6 +364,14 @@ class History
 					$rows[$new_version]['old_value'] = $row['history_new_value'];
 				}
 			}
+
+			// TODO: This is just here to hide bad values before we clean them with an update.  If you're here, remove this IF block
+			// Clear invalid share_email values
+			if($row['share_email'] && stripos($row['share_email'], '@') === false)
+			{
+				$row['share_email'] = '';
+			}
+
 			$rows[] = Api\Db::strip_array_keys($row, 'history_');
 		}
 		$total = $GLOBALS['egw']->db->union($_query, __LINE__, __FILE__)->NumRows();