From 5e36e2f033dcc9c038982e516bfdbb67d535ad96 Mon Sep 17 00:00:00 2001 From: nathangray Date: Fri, 15 Sep 2017 11:24:06 -0600 Subject: [PATCH] Infolog - fix bugs with contact & project ID fighting --- infolog/inc/class.infolog_bo.inc.php | 42 +++++++++++++++++++++-- infolog/inc/class.infolog_ui.inc.php | 2 +- infolog/test/ContactTest.php | 6 ++-- infolog/test/SetProjectManagerTest.php | 46 +++++++++++++++++++++++--- 4 files changed, 85 insertions(+), 11 deletions(-) diff --git a/infolog/inc/class.infolog_bo.inc.php b/infolog/inc/class.infolog_bo.inc.php index 6766b1c6dc..5ead712438 100644 --- a/infolog/inc/class.infolog_bo.inc.php +++ b/infolog/inc/class.infolog_bo.inc.php @@ -460,6 +460,11 @@ class infolog_bo { $info['old_pm_id'] = $info['pm_id'] = $id; } + else + { + // Link might be contact, check others + $this->get_pm_id($info); + } $info['info_link'] = $info['info_contact'] = array( 'app' => $app, 'id' => $id, @@ -469,13 +474,28 @@ class infolog_bo //echo " title='$title'

\n"; return $info['blur_title'] = $title; } - $info['info_link'] = $info['info_contact'] = array('id' => 'none', 'title' => $info['info_from']); + + // Set ID to 'none' instead of unset to make it seem like there's a value + $info['info_link'] = $info['info_contact'] = $info['info_from'] ? array('id' => 'none', 'title' => $info['info_from']) : null; $info['info_link_id'] = 0; // link might have been deleted $info['info_custom_from'] = (int)!!$info['info_from']; return False; } + /** + * Find projectmanager ID from linked project(s) + * + * @param Array $info + */ + public function get_pm_id(&$info) + { + $pm_links = Link::get_links('infolog',$info['info_id'],'projectmanager'); + + $old_pm_id = is_array($pm_links) ? array_shift($pm_links) : $info['old_pm_id']; + if (!isset($info['pm_id']) && $old_pm_id) $info['pm_id'] = $old_pm_id; + } + /** * Create a subject from a description: truncate it and add ' ...' */ @@ -1051,7 +1071,12 @@ class infolog_bo protected function write_check_links(&$values) { $old_link_id = (int)$values['info_link_id']; - if($values['info_contact']) + if($values['info_contact'] && !( + is_array($values['info_contact']) && $values['info_contact']['id'] == 'none' + ) || ( + is_array($values['info_contact']) && $values['info_contact']['id'] == 'none' && + array_key_exists('search', $values['info_contact']) + )) { if(is_array($values['info_contact'])) { @@ -1106,6 +1131,19 @@ class infolog_bo else { unset($values['info_link_id']); + $values['info_from'] = null; + } + if($values['info_id'] && $values['old_pm_id'] !== $values['pm_id']) + { + // Project has changed, but link is not to project + if($values['pm_id']) + { + $link_id = Link::link('infolog', $values['info_id'], 'projectmanager', $values['pm_id']); + if(!$values['info_link_id']) + { + $values['info_link_id'] = $link_id; + } + } } if ($old_link_id && $old_link_id != $values['info_link_id']) { diff --git a/infolog/inc/class.infolog_ui.inc.php b/infolog/inc/class.infolog_ui.inc.php index e7b81ac3a0..a28706dd95 100644 --- a/infolog/inc/class.infolog_ui.inc.php +++ b/infolog/inc/class.infolog_ui.inc.php @@ -2347,7 +2347,7 @@ class infolog_ui if (!isset($content['info_contact']) || empty($content['info_contact']) || $content['info_contact'] === 'copy:') { $linkinfos = Link::get_link($info_link_id); - $content['info_contact'] = $linkinfos['link_app1']=='infolog'? + $content['info_contact'] = $linkinfos['link_app1']=='infolog'? array('app' => $linkinfos['link_app2'], 'id' => $linkinfos['link_id2']): array('app' => $linkinfos['link_app1'], 'id' => $linkinfos['link_id1']); if ($content['info_contact']['app'] == 'projectmanager') diff --git a/infolog/test/ContactTest.php b/infolog/test/ContactTest.php index 63cce28230..dbb10e6254 100644 --- a/infolog/test/ContactTest.php +++ b/infolog/test/ContactTest.php @@ -63,15 +63,15 @@ class ContactTest extends \EGroupware\Api\AppTest 'search'=> 'Free text' ) ); - + $info = $this->getTestInfolog($content); // Skipping notifications - save initial state $this->info_id = $this->bo->write($info, true, true, true, true); - + // Read it back to check $saved = $this->bo->read($this->info_id); - + $this->assertEquals($content['contact']['search'], $saved['info_from']); $this->assertEquals(0, $saved['info_link_id']); diff --git a/infolog/test/SetProjectManagerTest.php b/infolog/test/SetProjectManagerTest.php index 3c094d16b0..5ca3e1ad2c 100644 --- a/infolog/test/SetProjectManagerTest.php +++ b/infolog/test/SetProjectManagerTest.php @@ -190,12 +190,48 @@ class SetProjectManagerTest extends \EGroupware\Api\AppTest $this->checkElements(); } + /** + * Test free text in the contact field + */ + public function testFreeContact() + { + $info = $this->getTestInfolog(); + + // Set up the test - just set info_contact + $info['info_contact'] = array( + 'app' => null, + 'id' => null, + 'search' => 'Free text' + ); + // Set project by pm_id + $info['pm_id'] = $this->pm_id; + + $this->info_id = $this->bo->write($info); + $this->assertArrayHasKey('info_id', $info, 'Could not make test infolog'); + $this->assertThat($this->info_id, + $this->logicalAnd( + $this->isType('integer'), + $this->greaterThan(0) + ) + ); + + // Check infolog has pm_id properly set + $this->assertEquals($this->pm_id, $info['pm_id']); + + // Force links to run notification now so we get valid testing - it + // usually waits until Egw::on_shutdown(); + Api\Link::run_notifies(); + + // Check project + $this->checkElements(); + } + public function testLoadWithProject() { // Saving the infolog should try to send a notification $this->bo->tracking->expects($this->once()) ->method('track') - ->with($this->callback(function($subject) { return $subject['pm_id'] == $this->pm_id;})); + ->will($this->returnCallback(function($subject) { return $subject['pm_id'] == $this->pm_id;})); $info = $this->getTestInfolog(); @@ -209,7 +245,7 @@ class SetProjectManagerTest extends \EGroupware\Api\AppTest $this->greaterThan(0) ) ); - + // Check infolog has pm_id properly set $this->assertEquals($this->pm_id, $info['pm_id']); @@ -268,7 +304,7 @@ class SetProjectManagerTest extends \EGroupware\Api\AppTest // Check pm_id is gone $this->assertNull($info['pm_id'], 'Project was not removed'); - + // Force links to run notification now so we get valid testing - it // usually waits until Egw::on_shutdown(); Api\Link::run_notifies(); @@ -322,7 +358,7 @@ class SetProjectManagerTest extends \EGroupware\Api\AppTest // Check contact was cleared $this->assertNull($info['info_contact'], 'Contact was not cleared'); - + // Check pm_id is gone $this->assertNull($info['pm_id'], 'Project was not removed'); @@ -393,7 +429,7 @@ class SetProjectManagerTest extends \EGroupware\Api\AppTest $this->assertEquals($expected_count, $element_count, "Incorrect number of elements"); } - + /** * Fully delete a project and its elements, no matter what state or settings */