Infolog: Fix could not change project after b125e1b2fd

infolog/inc/class.infolog_bo.inc.php

@@ -979,7 +979,7 @@ class infolog_bo
 
 		if (($info_id = $this->so->write($to_write, $check_modified, $purge_cfs, !isset($old))))
 		{
-			if (!isset($values['info_type']) || $status_only || empty($values['caldav_url']))
+			if(!isset($values['info_type']) || $status_only || empty($values['caldav_url']))
 			{
 				$values = $this->read($info_id, true, 'server', $ignore_acl);
 			}
@@ -987,21 +987,27 @@ class infolog_bo
 			$values['info_id'] = $info_id;
 			$to_write['info_id'] = $info_id;
 
-			// if the info responbsible array is not passed, fetch it from old.
+			// if the info responsible array is not passed, fetch it from old.
-			if (!array_key_exists('info_responsible',$values)) $values['info_responsible'] = $old['info_responsible'];
+			if(!array_key_exists('info_responsible', $values))
-			if (!is_array($values['info_responsible']))		// this should not happen, bug it does ;-)
 			{
-				$values['info_responsible'] = $values['info_responsible'] ? explode(',',$values['info_responsible']) : array();
+				$values['info_responsible'] = $old['info_responsible'];
+			}
+			if(!is_array($values['info_responsible']))        // this should not happen, bug it does ;-)
+			{
+				$values['info_responsible'] = $values['info_responsible'] ? explode(',', $values['info_responsible']) : array();
 				$to_write['info_responsible'] = $values['info_responsible'];
 			}
 
 			// writing links for a new entry
-			if (!$old && is_array($to_write['link_to']['to_id']) && count($to_write['link_to']['to_id']))
+			if(!$old && is_array($to_write['link_to']['to_id']) && count($to_write['link_to']['to_id']))
 			{
 				//echo "<p>writing links for new entry $info_id</p>\n"; _debug_array($content['link_to']['to_id']);
-				Link::link('infolog',$info_id,$to_write['link_to']['to_id']);
+				Link::link('infolog', $info_id, $to_write['link_to']['to_id']);
 				$values['link_to']['to_id'] = $info_id;
 			}
+		}
+		if($values['info_id'] && $info_id !== false)
+		{
 			$this->write_check_links($to_write);
 			if(!$values['info_link_id'] || $values['info_link_id'] != $to_write['info_link_id'])
 			{
@@ -1044,7 +1050,7 @@ class infolog_bo
 				$this->tracking = new infolog_tracking($this);
 			}
 
-			if ($old && ($missing_fields = array_diff_key($old,$values)))
+			if($old && ($missing_fields = array_diff_key($old, $values)))
 			{
 				// Some custom fields (multiselect with nothing selected) will be missing,
 				// and that's OK.  Don't put them back.
@@ -1055,20 +1061,24 @@ class infolog_bo
 						unset($missing_fields[$field]);
 					}
 				}
-				$values = array_merge($values,$missing_fields);
+				$values = array_merge($values, $missing_fields);
 			}
 			// Add keys missing in the $to_write array
-			if (($missing_fields = array_diff_key($values,$to_write)))
+			if(($missing_fields = array_diff_key($values, $to_write)))
 			{
-				$to_write = array_merge($to_write,$missing_fields);
+				$to_write = array_merge($to_write, $missing_fields);
 			}
-			$this->tracking->track($to_write,$old,$this->user,$values['info_status'] == 'deleted' || $old['info_status'] == 'deleted',
+			$this->tracking->track($to_write, $old, $this->user, $values['info_status'] == 'deleted' || $old['info_status'] == 'deleted',
-				null,$skip_notification);
+								   null, $skip_notification
+			);
 
 			// Clear access cache after notifications, it may get modified as notifications switches user
 			unset(static::$access_cache[$info_id]);
 
-			if ($info_from_set) $values['info_from'] = '';
+			if($info_from_set)
+			{
+				$values['info_from'] = '';
+			}
 
 			// Change new values back to user time before sending them back
 			if($user2server)
@@ -1076,7 +1086,7 @@ class infolog_bo
 				$this->time2time($values);
 			}
 			// merge changes (keeping extra values from the UI)
-			$values_in = array_merge($values_in,$values);
+			$values_in = array_merge($values_in, $values);
 
 			// Update modified timestamp of parent
 			if($values['info_id_parent'] && $touch_modified)
@@ -1180,7 +1190,7 @@ class infolog_bo
 			if($values['pm_id'])
 			{
 				$link_id = Link::link('infolog', $values['info_id'], 'projectmanager', $values['pm_id']);
-				if(!$values['info_link_id'])
+				if(!$values['info_link_id'] || !Link::get_link($values['info_link_id']))
 				{
 					$values['info_link_id'] = $link_id;
 				}

infolog/inc/class.infolog_ui.inc.php

@@ -2213,33 +2213,40 @@ class infolog_ui
 		if($content['info_type'] != $content['old_type'])
 		{
 			$content['no_notifications'] = in_array($content['info_type'], !is_array($this->prefs['no_notification_types']) ?
-					explode(',',$this->prefs['no_notification_types']):
+				explode(',', $this->prefs['no_notification_types']) :
-					$this->prefs['no_notification_types']
+				$this->prefs['no_notification_types']
 			);
 		}
 
-		$content['info_anz_subs'] = (int)$content['info_anz_subs'];	// gives javascript error if empty!
+		$content['info_anz_subs'] = (int)$content['info_anz_subs'];    // gives javascript error if empty!
 
-		$old_pm_id = $content['pm_id'] ?: (is_array($pm_links) ? array_shift($pm_links) : $content['old_pm_id']);
+		if(is_array($pm_links))
+		{
+			$old_pm_id = $content['pm_id'] && in_array($content['pm_id'], $pm_links) ? $content['pm_id'] : array_shift($pm_links);
+		}
+		else
+		{
+			$old_pm_id = $content['old_pm_id'];
+		}
 		unset($content['old_pm_id']);
 
-		if ($info_id && $this->bo->history)
+		if($info_id && $this->bo->history)
 		{
 			$content['history'] = array(
-				'id'  => $info_id,
+				'id'             => $info_id,
-				'app' => 'infolog',
+				'app'            => 'infolog',
 				'status-widgets' => array(
-					'Ty' => $types,
+					'Ty'        => $types,
 					//'Li',	// info_link_id
-					'parent' => 'link-entry:infolog',
+					'parent'    => 'link-entry:infolog',
-					'Ca' => 'select-cat',
+					'Ca'        => 'select-cat',
-					'Pr' => $this->bo->enums['priority'],
+					'Pr'        => $this->bo->enums['priority'],
-					'Ow' => 'select-account',
+					'Ow'        => 'select-account',
 					//'Ac',	//	info_access: private||public
-					'St' => (array)$this->bo->status[$content['info_type']]+array('deleted' => 'deleted'),
+					'St'        => (array)$this->bo->status[$content['info_type']] + array('deleted' => 'deleted'),
-					'Pe' => 'select-percent',
+					'Pe'        => 'select-percent',
-					'Co' => 'date-time',
+					'Co'        => 'date-time',
-					'st' => 'date-time',
+					'st'        => 'date-time',
 					'Mo' => 'date-time',
 					'En' => 'date',
 					'Re' => 'select-account',

infolog/tests/DoubleLinkPMTest.php

@@ -37,6 +37,7 @@ class DoubleLinkPMTest extends \EGroupware\Api\EtemplateTest
 	{
 		parent::setUp();
 
+		$_GET = $_POST = $_REQUEST = array();
 		$this->ui = new \infolog_ui();
 
 		$this->ui->tmpl = $this->createPartialMock(Etemplate::class, array('exec'));
@@ -53,7 +54,8 @@ class DoubleLinkPMTest extends \EGroupware\Api\EtemplateTest
 		// Make sure projects are not there first
 		$pm_numbers = array(
 			'TEST 1',
-			'TEST 2'
+			'TEST 2',
+			'TEST 3'
 		);
 		foreach($pm_numbers as $number)
 		{
@@ -67,7 +69,6 @@ class DoubleLinkPMTest extends \EGroupware\Api\EtemplateTest
 		$this->makeProject("1");
 
 		// Make another project, we need 2
-		$this->pm_bo->data = array();
 		$this->makeProject("2");
 	}
 
@@ -81,12 +82,15 @@ class DoubleLinkPMTest extends \EGroupware\Api\EtemplateTest
 			$this->bo->delete($this->info_id, False, False, True);
 		}
 
-		// Remove the test project
+		// Remove the test projects
 		$this->deleteProject();
 
 		$this->bo = null;
 		$this->pm_bo = null;
 
+		// Clean up the request
+		$_GET = $_POST = $_REQUEST = array();
+
 		parent::tearDown();
 	}
 
@@ -101,10 +105,8 @@ class DoubleLinkPMTest extends \EGroupware\Api\EtemplateTest
 		$first_project = $this->pm_id[0];
 		$second_project = $this->pm_id[1];
 
+		// Create our test infolog
 		$info = $this->getTestInfolog();
-		// Set project by ID
-		//$info['pm_id'] = $first_project;
-
 		$this->info_id = $this->bo->write($info);
 		$this->assertIsInt($this->info_id);
 		$this->assertGreaterThan(0, $this->info_id);
@@ -114,19 +116,26 @@ class DoubleLinkPMTest extends \EGroupware\Api\EtemplateTest
 		Link::run_notifies();
 
 		// Fake opening the edit dialog, important not to pass an array to accurately copy normal behaviour
+		// Set the initial project via the select
 		$this->ui->edit($this->info_id);
-		// Set button 'apply' to save, but not try to close the window since
-		// that would fail
 		$content = self::$mocked_exec_result;
-
 		$content['pm_id'] = $first_project;
-		$content['button'] = array('apply' => true);
+		// pm_id has an on change submit, so submit without button
 		$this->ui->edit($content);
 
 		// Force links to run notification now so we get valid testing - it
 		// usually waits until Egw::on_shutdown();
 		Link::run_notifies();
-		// Now load it again
+
+		// Then they click apply
+		// Set button 'apply' to save, but not try to close the window since
+		// that would fail
+		$content = self::$mocked_exec_result;
+		$content['button'] = ['apply' => true];
+		$this->ui->edit($content);
+		Link::run_notifies();
+
+		// Now load the infolog entry
 		$info = $this->bo->read($this->info_id);
 
 		// Check original pm_id is there
@@ -149,6 +158,43 @@ class DoubleLinkPMTest extends \EGroupware\Api\EtemplateTest
 		$this->checkElements($first_project);
 		// Check infolog is in second project
 		$this->checkElements($second_project);
+
+		// Check changing project
+		$this->checkWeCanChangeTheProject();
+	}
+
+	/**
+	 * Test that after messing with the links in testInProjectLinkAnother(),
+	 * we can just change the pm_id and have it stored correctly
+	 */
+	protected function checkWeCanChangeTheProject()
+	{
+		// Make another project
+		$this->makeProject('3');
+		$new_project = $this->pm_id[2];
+
+		// Fake opening the edit dialog, important not to pass an array to accurately copy normal behaviour
+		$this->ui->edit($this->info_id);
+		$content = self::$mocked_exec_result;
+
+		$content['pm_id'] = $new_project;
+		$content['button'] = array('apply' => true);
+
+		// Set button 'apply' to save, but not try to close the window since
+		// that would fail
+		$this->ui->edit($content);
+
+		// Force links to run notification now so we get valid testing - it
+		// usually waits until Egw::on_shutdown();
+		Link::run_notifies();
+
+		// Now load it again
+		$info = $this->bo->read($this->info_id);
+
+		// Check new pm_id is there
+		$this->assertNotNull($info['pm_id'], 'Project was not set');
+		$this->assertNotEquals((string)$this->pm_id[0], (string)$info['pm_id'], 'Project did not get changed');
+		$this->assertEquals($new_project, $info['pm_id'], 'Project went missing');
 	}
 
 	public function mockExec($method, $content, $sel_options, $readonlys, $preserve)
@@ -251,6 +297,7 @@ class DoubleLinkPMTest extends \EGroupware\Api\EtemplateTest
 		try
 		{
 			$result = true;
+			$this->pm_bo->data = array();
 			$result = $this->pm_bo->save($project, true, false);
 		}
 		catch (\Exception $e)