From 3d2502c630dde907450e8d5d59039f079eac7de6 Mon Sep 17 00:00:00 2001 From: nathan Date: Tue, 9 Nov 2021 14:02:10 -0700 Subject: [PATCH] Infolog: Fix could not change project after b125e1b2fd680c69c7d80e2c48fefaa7a4241d35 --- infolog/inc/class.infolog_bo.inc.php | 42 ++++++++++------- infolog/inc/class.infolog_ui.inc.php | 39 +++++++++------- infolog/tests/DoubleLinkPMTest.php | 69 +++++++++++++++++++++++----- 3 files changed, 107 insertions(+), 43 deletions(-) diff --git a/infolog/inc/class.infolog_bo.inc.php b/infolog/inc/class.infolog_bo.inc.php index 8c11fa8ce5..15beb6bfbb 100644 --- a/infolog/inc/class.infolog_bo.inc.php +++ b/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 (!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 ;-) + // if the info responsible array is not passed, fetch it from old. + if(!array_key_exists('info_responsible', $values)) { - $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 "

writing links for new entry $info_id

\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', - null,$skip_notification); + $this->tracking->track($to_write, $old, $this->user, $values['info_status'] == 'deleted' || $old['info_status'] == 'deleted', + 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; } diff --git a/infolog/inc/class.infolog_ui.inc.php b/infolog/inc/class.infolog_ui.inc.php index ffc868f5d1..9a1a3647b8 100644 --- a/infolog/inc/class.infolog_ui.inc.php +++ b/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']): - $this->prefs['no_notification_types'] + explode(',', $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, - 'app' => 'infolog', + 'id' => $info_id, + 'app' => 'infolog', 'status-widgets' => array( - 'Ty' => $types, + 'Ty' => $types, //'Li', // info_link_id - 'parent' => 'link-entry:infolog', - 'Ca' => 'select-cat', - 'Pr' => $this->bo->enums['priority'], - 'Ow' => 'select-account', + 'parent' => 'link-entry:infolog', + 'Ca' => 'select-cat', + 'Pr' => $this->bo->enums['priority'], + 'Ow' => 'select-account', //'Ac', // info_access: private||public - 'St' => (array)$this->bo->status[$content['info_type']]+array('deleted' => 'deleted'), - 'Pe' => 'select-percent', - 'Co' => 'date-time', - 'st' => 'date-time', + 'St' => (array)$this->bo->status[$content['info_type']] + array('deleted' => 'deleted'), + 'Pe' => 'select-percent', + 'Co' => 'date-time', + 'st' => 'date-time', 'Mo' => 'date-time', 'En' => 'date', 'Re' => 'select-account', diff --git a/infolog/tests/DoubleLinkPMTest.php b/infolog/tests/DoubleLinkPMTest.php index a548a31317..c14517e90d 100644 --- a/infolog/tests/DoubleLinkPMTest.php +++ b/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)