Infolog: Fix could not change project after b125e1b2fd680c69c7d80e2c48fefaa7a4241d35

infolog/inc/class.infolog_bo.inc.php

@@ -987,8 +987,11 @@ 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))
+			{
+				$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();
@@ -1002,6 +1005,9 @@ class infolog_bo
 				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'])
 			{
@@ -1063,12 +1069,16 @@ class infolog_bo
 				$to_write = array_merge($to_write, $missing_fields);
 			}
 			$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)
@@ -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

@@ -2220,7 +2220,14 @@ class infolog_ui
 
 		$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)

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)