From f6a3d5c3fb64971a748acd1b2ee2e0866ac91d93 Mon Sep 17 00:00:00 2001 From: Art4 Date: Mon, 8 Jul 2024 14:58:14 +0200 Subject: [PATCH 01/24] extract CustomField::listing() --- src/Redmine/Api/CustomField.php | 30 +++++++++++++++++++----------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/src/Redmine/Api/CustomField.php b/src/Redmine/Api/CustomField.php index 54c1366b..19ce29c9 100644 --- a/src/Redmine/Api/CustomField.php +++ b/src/Redmine/Api/CustomField.php @@ -111,19 +111,11 @@ public function listing($forceUpdate = false, array $params = []) { @trigger_error('`' . __METHOD__ . '()` is deprecated since v2.7.0, use `' . __CLASS__ . '::listNames()` instead.', E_USER_DEPRECATED); - if (empty($this->customFields) || $forceUpdate) { - $this->customFields = $this->list($params); - } - $ret = []; - foreach ($this->customFields['custom_fields'] as $e) { - $ret[$e['name']] = (int) $e['id']; - } - - return $ret; + return $this->doListing($forceUpdate, $params); } /** - * Get a tracket id given its name. + * Get a custom field id given its name. * * @param string|int $name customer field name * @param array $params optional parameters to be passed to the api (offset, limit, ...) @@ -132,11 +124,27 @@ public function listing($forceUpdate = false, array $params = []) */ public function getIdByName($name, array $params = []) { - $arr = $this->listing(false, $params); + $arr = $this->doListing(false, $params); + if (!isset($arr[$name])) { return false; } return $arr[(string) $name]; } + + private function doListing($forceUpdate = false, array $params = []) + { + if (empty($this->customFields) || $forceUpdate) { + $this->customFields = $this->list($params); + } + + $ret = []; + + foreach ($this->customFields['custom_fields'] as $e) { + $ret[$e['name']] = (int) $e['id']; + } + + return $ret; + } } From 10768a92555a230303472ef20873932bed82c75f Mon Sep 17 00:00:00 2001 From: Art4 Date: Mon, 8 Jul 2024 15:01:36 +0200 Subject: [PATCH 02/24] Deprecate CustomField::getIdByName() --- CHANGELOG.md | 1 + src/Redmine/Api/CustomField.php | 5 +++++ tests/Unit/Api/CustomFieldTest.php | 29 +++++++++++++++++++++++++++++ 3 files changed, 35 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3ab50f15..d7bdbbdf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Deprecated - `Redmine\Api\CustomField::listing()` is deprecated, use `\Redmine\Api\CustomField::listNames()` instead. +- `Redmine\Api\CustomField::getIdByName()` is deprecated, use `\Redmine\Api\CustomField::listNames()` instead. - `Redmine\Api\Group::listing()` is deprecated, use `\Redmine\Api\Group::listNames()` instead. - `Redmine\Api\IssueCategory::listing()` is deprecated, use `\Redmine\Api\IssueCategory::listNamesByProject()` instead. - `Redmine\Api\IssueStatus::listing()` is deprecated, use `\Redmine\Api\IssueStatus::listNames()` instead. diff --git a/src/Redmine/Api/CustomField.php b/src/Redmine/Api/CustomField.php index 19ce29c9..7855de3b 100644 --- a/src/Redmine/Api/CustomField.php +++ b/src/Redmine/Api/CustomField.php @@ -117,6 +117,9 @@ public function listing($forceUpdate = false, array $params = []) /** * Get a custom field id given its name. * + * @deprecated v2.7.0 Use listNames() instead. + * @see CustomField::listNames() + * * @param string|int $name customer field name * @param array $params optional parameters to be passed to the api (offset, limit, ...) * @@ -124,6 +127,8 @@ public function listing($forceUpdate = false, array $params = []) */ public function getIdByName($name, array $params = []) { + @trigger_error('`' . __METHOD__ . '()` is deprecated since v2.7.0, use `' . __CLASS__ . '::listNames()` instead.', E_USER_DEPRECATED); + $arr = $this->doListing(false, $params); if (!isset($arr[$name])) { diff --git a/tests/Unit/Api/CustomFieldTest.php b/tests/Unit/Api/CustomFieldTest.php index e6a152a8..0e8d22cc 100644 --- a/tests/Unit/Api/CustomFieldTest.php +++ b/tests/Unit/Api/CustomFieldTest.php @@ -349,4 +349,33 @@ public function testGetIdByNameMakesGetRequest() $this->assertFalse($api->getIdByName('CustomField 1')); $this->assertSame(5, $api->getIdByName('CustomField 5')); } + + public function testGetIdByNameTriggersDeprecationWarning() + { + $client = $this->createMock(Client::class); + $client->method('requestGet') + ->willReturn(true); + $client->method('getLastResponseBody') + ->willReturn('{"custom_fields":[{"id":1,"name":"CustomField 1"},{"id":5,"name":"CustomField 5"}]}'); + $client->method('getLastResponseContentType') + ->willReturn('application/json'); + + $api = new CustomField($client); + + // PHPUnit 10 compatible way to test trigger_error(). + set_error_handler( + function ($errno, $errstr): bool { + $this->assertSame( + '`Redmine\Api\CustomField::getIdByName()` is deprecated since v2.7.0, use `Redmine\Api\CustomField::listNames()` instead.', + $errstr, + ); + + restore_error_handler(); + return true; + }, + E_USER_DEPRECATED, + ); + + $api->getIdByName('CustomField 5'); + } } From f09ec2810c0557b02eaa933195f06c52b41b954f Mon Sep 17 00:00:00 2001 From: Art4 Date: Mon, 8 Jul 2024 15:16:03 +0200 Subject: [PATCH 03/24] extract IssueCategory::listing() --- src/Redmine/Api/IssueCategory.php | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/src/Redmine/Api/IssueCategory.php b/src/Redmine/Api/IssueCategory.php index 25a05eb9..8e19f457 100644 --- a/src/Redmine/Api/IssueCategory.php +++ b/src/Redmine/Api/IssueCategory.php @@ -137,15 +137,7 @@ public function listing($project, $forceUpdate = false) { @trigger_error('`' . __METHOD__ . '()` is deprecated since v2.7.0, use `' . __CLASS__ . '::listNamesByProject()` instead.', E_USER_DEPRECATED); - if (true === $forceUpdate || empty($this->issueCategories)) { - $this->issueCategories = $this->listByProject($project); - } - $ret = []; - foreach ($this->issueCategories['issue_categories'] as $e) { - $ret[$e['name']] = (int) $e['id']; - } - - return $ret; + return $this->doListing($project, $forceUpdate); } /** @@ -158,7 +150,7 @@ public function listing($project, $forceUpdate = false) */ public function getIdByName($project, $name) { - $arr = $this->listing($project); + $arr = $this->doListing($project); if (!isset($arr[$name])) { return false; } @@ -283,4 +275,19 @@ public function remove($id, array $params = []) return $this->lastResponse->getContent(); } + + private function doListing($project, $forceUpdate = false) + { + if (true === $forceUpdate || empty($this->issueCategories)) { + $this->issueCategories = $this->listByProject($project); + } + + $ret = []; + + foreach ($this->issueCategories['issue_categories'] as $e) { + $ret[$e['name']] = (int) $e['id']; + } + + return $ret; + } } From 71655dc38afb757e2fcd296e9b5396c1f640ef1f Mon Sep 17 00:00:00 2001 From: Art4 Date: Mon, 8 Jul 2024 15:46:39 +0200 Subject: [PATCH 04/24] Replace IssueCategory::getIdByName() with listNamesByProject() --- src/Redmine/Api/Issue.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/Redmine/Api/Issue.php b/src/Redmine/Api/Issue.php index 1c98dceb..38ecc7c8 100644 --- a/src/Redmine/Api/Issue.php +++ b/src/Redmine/Api/Issue.php @@ -355,7 +355,11 @@ private function cleanParams(array $params = []) if (isset($params['category']) && isset($params['project_id'])) { $issueCategoryApi = $this->getIssueCategoryApi(); - $params['category_id'] = $issueCategoryApi->getIdByName($params['project_id'], $params['category']); + $params['category_id'] = array_search( + $params['category'], + $issueCategoryApi->listNamesByProject($params['project_id']), + true, + ); unset($params['category']); } From 2edd582d53bd4417115141b6a8d8913601957bcf Mon Sep 17 00:00:00 2001 From: Art4 Date: Mon, 8 Jul 2024 16:58:16 +0200 Subject: [PATCH 05/24] fix tests --- tests/Unit/Api/IssueTest.php | 52 +++++++++++++++++++++++------------- 1 file changed, 34 insertions(+), 18 deletions(-) diff --git a/tests/Unit/Api/IssueTest.php b/tests/Unit/Api/IssueTest.php index edce151c..5a7e335d 100644 --- a/tests/Unit/Api/IssueTest.php +++ b/tests/Unit/Api/IssueTest.php @@ -6,7 +6,10 @@ use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\TestCase; use Redmine\Api\Issue; +use Redmine\Api\IssueCategory; use Redmine\Client\Client; +use Redmine\Http\HttpClient; +use Redmine\Http\Response; use Redmine\Tests\Fixtures\MockClient; /** @@ -142,7 +145,7 @@ public function testCreateWithClientCleansParameters() $response = ''; $parameters = [ 'project' => 'Project Name', - 'category' => 'Category Name', + 'category' => 'Category 5 Name', 'status' => 'Status Name', 'tracker' => 'Tracker Name', 'assigned_to' => 'Assigned to User Name', @@ -150,12 +153,12 @@ public function testCreateWithClientCleansParameters() ]; // Create the used mock objects - $getIdByNameApi = $this->createMock('Redmine\Api\Project'); - $getIdByNameApi->expects($this->exactly(3)) + $projectApi = $this->createMock('Redmine\Api\Project'); + $projectApi->expects($this->exactly(1)) ->method('getIdByName') - ->willReturn('cleanedValue'); - $issueCategoryGetIdByNameApi = $this->createMock('Redmine\Api\IssueCategory'); - $issueCategoryGetIdByNameApi->expects($this->exactly(1)) + ->willReturn(1); + $getIdByNameApi = $this->createMock('Redmine\Api\Tracker'); + $getIdByNameApi->expects($this->exactly(2)) ->method('getIdByName') ->willReturn('cleanedValue'); $getIdByUsernameApi = $this->createMock('Redmine\Api\User'); @@ -163,32 +166,45 @@ public function testCreateWithClientCleansParameters() ->method('getIdByUsername') ->willReturn('cleanedValue'); + $httpClient = $this->createMock(HttpClient::class); + $httpClient->expects($this->exactly(1)) + ->method('request') + ->willReturn( + $this->createConfiguredMock( + Response::class, + [ + 'getStatusCode' => 200, + 'getContentType' => 'application/json', + 'getContent' => '{"issue_categories":[{"id":5,"name":"Category 5 Name"}]}', + ], + ), + ) + ; + $client = $this->createMock(Client::class); $client->expects($this->exactly(5)) ->method('getApi') ->willReturnMap( [ - ['project', $getIdByNameApi], - ['issue_category', $issueCategoryGetIdByNameApi], + ['project', $projectApi], + ['issue_category', new IssueCategory($httpClient)], ['issue_status', $getIdByNameApi], ['tracker', $getIdByNameApi], ['user', $getIdByUsernameApi], ], - ); + ) + ; $client->expects($this->once()) ->method('requestPost') ->with( '/issues.xml', - $this->logicalAnd( - $this->stringStartsWith('' . "\n" . ''), - $this->stringEndsWith('' . "\n"), - $this->stringContains('cleanedValue'), - $this->stringContains('cleanedValue'), - $this->stringContains('cleanedValue'), - $this->stringContains('cleanedValue'), - $this->stringContains('cleanedValue'), - $this->stringContains('cleanedValue'), + $this->stringEqualsStringIgnoringLineEndings( + <<< XML + + 15cleanedValuecleanedValuecleanedValuecleanedValue + + XML, ), ) ->willReturn(true); From 0f8257aa3bea40bf33c28b952fe2a498fb6d715b Mon Sep 17 00:00:00 2001 From: Art4 Date: Mon, 8 Jul 2024 17:03:31 +0200 Subject: [PATCH 06/24] Fix support with PHPUnit 9 --- tests/Unit/Api/IssueTest.php | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/tests/Unit/Api/IssueTest.php b/tests/Unit/Api/IssueTest.php index 5a7e335d..ba611805 100644 --- a/tests/Unit/Api/IssueTest.php +++ b/tests/Unit/Api/IssueTest.php @@ -199,13 +199,11 @@ public function testCreateWithClientCleansParameters() ->method('requestPost') ->with( '/issues.xml', - $this->stringEqualsStringIgnoringLineEndings( - <<< XML - - 15cleanedValuecleanedValuecleanedValuecleanedValue + <<< XML + + 15cleanedValuecleanedValuecleanedValuecleanedValue - XML, - ), + XML, ) ->willReturn(true); $client->expects($this->exactly(1)) From 0142a0c14a9698b275aecdd09850f44ac7ef45c4 Mon Sep 17 00:00:00 2001 From: Art4 Date: Tue, 9 Jul 2024 10:42:58 +0200 Subject: [PATCH 07/24] Deprecate IssueCategory::getIdByName() --- CHANGELOG.md | 1 + src/Redmine/Api/IssueCategory.php | 6 ++++++ tests/Unit/Api/IssueCategoryTest.php | 29 ++++++++++++++++++++++++++++ 3 files changed, 36 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1cabca63..5729075c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `Redmine\Api\CustomField::getIdByName()` is deprecated, use `\Redmine\Api\CustomField::listNames()` instead. - `Redmine\Api\Group::listing()` is deprecated, use `\Redmine\Api\Group::listNames()` instead. - `Redmine\Api\IssueCategory::listing()` is deprecated, use `\Redmine\Api\IssueCategory::listNamesByProject()` instead. +- `Redmine\Api\IssueCategory::getIdByName()` is deprecated, use `\Redmine\Api\IssueCategory::listNamesByProject()` instead. - `Redmine\Api\IssueStatus::listing()` is deprecated, use `\Redmine\Api\IssueStatus::listNames()` instead. - `Redmine\Api\Project::listing()` is deprecated, use `\Redmine\Api\Project::listNames()` instead. - `Redmine\Api\Role::listing()` is deprecated, use `\Redmine\Api\Role::listNames()` instead. diff --git a/src/Redmine/Api/IssueCategory.php b/src/Redmine/Api/IssueCategory.php index 8e19f457..058b8ca3 100644 --- a/src/Redmine/Api/IssueCategory.php +++ b/src/Redmine/Api/IssueCategory.php @@ -143,6 +143,9 @@ public function listing($project, $forceUpdate = false) /** * Get a category id given its name and related project. * + * @deprecated v2.7.0 Use listNamesByProject() instead. + * @see IssueCategory::listNamesByProject() + * * @param string|int $project project id or literal identifier * @param string $name * @@ -150,7 +153,10 @@ public function listing($project, $forceUpdate = false) */ public function getIdByName($project, $name) { + @trigger_error('`' . __METHOD__ . '()` is deprecated since v2.7.0, use `' . __CLASS__ . '::listNamesByProject()` instead.', E_USER_DEPRECATED); + $arr = $this->doListing($project); + if (!isset($arr[$name])) { return false; } diff --git a/tests/Unit/Api/IssueCategoryTest.php b/tests/Unit/Api/IssueCategoryTest.php index 5c011f2d..2bdbba84 100644 --- a/tests/Unit/Api/IssueCategoryTest.php +++ b/tests/Unit/Api/IssueCategoryTest.php @@ -281,4 +281,33 @@ public function testGetIdByNameMakesGetRequest() $this->assertFalse($api->getIdByName(5, 'IssueCategory 1')); $this->assertSame(5, $api->getIdByName(5, 'IssueCategory 5')); } + + public function testGetIdByNameTriggersDeprecationWarning() + { + $client = $this->createMock(Client::class); + $client->method('requestGet') + ->willReturn(true); + $client->method('getLastResponseBody') + ->willReturn('{"issue_categories":[{"id":1,"name":"IssueCategory 1"},{"id":5,"name":"IssueCategory 5"}]}'); + $client->method('getLastResponseContentType') + ->willReturn('application/json'); + + $api = new IssueCategory($client); + + // PHPUnit 10 compatible way to test trigger_error(). + set_error_handler( + function ($errno, $errstr): bool { + $this->assertSame( + '`Redmine\Api\IssueCategory::getIdByName()` is deprecated since v2.7.0, use `Redmine\Api\IssueCategory::listNamesByProject()` instead.', + $errstr, + ); + + restore_error_handler(); + return true; + }, + E_USER_DEPRECATED, + ); + + $api->getIdByName(5, 'Category Name'); + } } From 3194e30cb7649df7537f22278116bcc9349179ff Mon Sep 17 00:00:00 2001 From: Art4 Date: Tue, 9 Jul 2024 10:56:53 +0200 Subject: [PATCH 08/24] Extract IssueStatus::listing() --- src/Redmine/Api/IssueStatus.php | 28 ++++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/src/Redmine/Api/IssueStatus.php b/src/Redmine/Api/IssueStatus.php index 022b1e43..3d298eea 100644 --- a/src/Redmine/Api/IssueStatus.php +++ b/src/Redmine/Api/IssueStatus.php @@ -110,15 +110,7 @@ public function listing($forceUpdate = false) { @trigger_error('`' . __METHOD__ . '()` is deprecated since v2.7.0, use `' . __CLASS__ . '::listNames()` instead.', E_USER_DEPRECATED); - if (empty($this->issueStatuses) || $forceUpdate) { - $this->issueStatuses = $this->list(); - } - $ret = []; - foreach ($this->issueStatuses['issue_statuses'] as $e) { - $ret[$e['name']] = (int) $e['id']; - } - - return $ret; + return $this->doListing($forceUpdate); } /** @@ -130,11 +122,27 @@ public function listing($forceUpdate = false) */ public function getIdByName($name) { - $arr = $this->listing(); + $arr = $this->doListing(false); + if (!isset($arr[$name])) { return false; } return $arr[(string) $name]; } + + private function doListing(bool $forceUpdate) + { + if (empty($this->issueStatuses) || $forceUpdate) { + $this->issueStatuses = $this->list(); + } + + $ret = []; + + foreach ($this->issueStatuses['issue_statuses'] as $e) { + $ret[$e['name']] = (int) $e['id']; + } + + return $ret; + } } From 057de974276c29ed05457b19c84e189d84b99884 Mon Sep 17 00:00:00 2001 From: Art4 Date: Tue, 9 Jul 2024 11:11:25 +0200 Subject: [PATCH 09/24] Replace IssueStatus::getIdByName() with IssueStatus::listNames() --- src/Redmine/Api/Issue.php | 8 +++++-- tests/Unit/Api/IssueTest.php | 45 +++++++++++++++++++++--------------- 2 files changed, 33 insertions(+), 20 deletions(-) diff --git a/src/Redmine/Api/Issue.php b/src/Redmine/Api/Issue.php index 38ecc7c8..5572f2ec 100644 --- a/src/Redmine/Api/Issue.php +++ b/src/Redmine/Api/Issue.php @@ -319,7 +319,7 @@ public function setIssueStatus($id, $status) $issueStatusApi = $this->getIssueStatusApi(); return $this->update($id, [ - 'status_id' => $issueStatusApi->getIdByName($status), + 'status_id' => array_search($status, $issueStatusApi->listNames(), true), ]); } @@ -366,7 +366,11 @@ private function cleanParams(array $params = []) if (isset($params['status'])) { $issueStatusApi = $this->getIssueStatusApi(); - $params['status_id'] = $issueStatusApi->getIdByName($params['status']); + $params['status_id'] = array_search( + $params['status'], + $issueStatusApi->listNames(), + true, + ); unset($params['status']); } diff --git a/tests/Unit/Api/IssueTest.php b/tests/Unit/Api/IssueTest.php index ba611805..675bbb8d 100644 --- a/tests/Unit/Api/IssueTest.php +++ b/tests/Unit/Api/IssueTest.php @@ -7,9 +7,11 @@ use PHPUnit\Framework\TestCase; use Redmine\Api\Issue; use Redmine\Api\IssueCategory; +use Redmine\Api\IssueStatus; use Redmine\Client\Client; use Redmine\Http\HttpClient; use Redmine\Http\Response; +use Redmine\Tests\Fixtures\AssertingHttpClient; use Redmine\Tests\Fixtures\MockClient; /** @@ -146,7 +148,7 @@ public function testCreateWithClientCleansParameters() $parameters = [ 'project' => 'Project Name', 'category' => 'Category 5 Name', - 'status' => 'Status Name', + 'status' => 'Status 6 Name', 'tracker' => 'Tracker Name', 'assigned_to' => 'Assigned to User Name', 'author' => 'Author Name', @@ -158,7 +160,7 @@ public function testCreateWithClientCleansParameters() ->method('getIdByName') ->willReturn(1); $getIdByNameApi = $this->createMock('Redmine\Api\Tracker'); - $getIdByNameApi->expects($this->exactly(2)) + $getIdByNameApi->expects($this->exactly(1)) ->method('getIdByName') ->willReturn('cleanedValue'); $getIdByUsernameApi = $this->createMock('Redmine\Api\User'); @@ -166,20 +168,27 @@ public function testCreateWithClientCleansParameters() ->method('getIdByUsername') ->willReturn('cleanedValue'); - $httpClient = $this->createMock(HttpClient::class); - $httpClient->expects($this->exactly(1)) - ->method('request') - ->willReturn( - $this->createConfiguredMock( - Response::class, - [ - 'getStatusCode' => 200, - 'getContentType' => 'application/json', - 'getContent' => '{"issue_categories":[{"id":5,"name":"Category 5 Name"}]}', - ], - ), - ) - ; + $httpClient = AssertingHttpClient::create( + $this, + [ + 'GET', + '/projects/1/issue_categories.json', + 'application/json', + '', + 200, + 'application/json', + '{"issue_categories":[{"id":5,"name":"Category 5 Name"}]}', + ], + [ + 'GET', + '/issue_statuses.json', + 'application/json', + '', + 200, + 'application/json', + '{"issue_statuses":[{"id":6,"name":"Status 6 Name"}]}', + ], + ); $client = $this->createMock(Client::class); $client->expects($this->exactly(5)) @@ -188,7 +197,7 @@ public function testCreateWithClientCleansParameters() [ ['project', $projectApi], ['issue_category', new IssueCategory($httpClient)], - ['issue_status', $getIdByNameApi], + ['issue_status', new IssueStatus($httpClient)], ['tracker', $getIdByNameApi], ['user', $getIdByUsernameApi], ], @@ -201,7 +210,7 @@ public function testCreateWithClientCleansParameters() '/issues.xml', <<< XML - 15cleanedValuecleanedValuecleanedValuecleanedValue + 156cleanedValuecleanedValuecleanedValue XML, ) From 56bdb6b14fcd7dabb4a812395b5259a29d7694b4 Mon Sep 17 00:00:00 2001 From: Art4 Date: Tue, 9 Jul 2024 11:13:47 +0200 Subject: [PATCH 10/24] Deprecate IssueStatus::getIdByName() --- src/Redmine/Api/IssueStatus.php | 5 +++++ tests/Unit/Api/IssueStatusTest.php | 29 +++++++++++++++++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/src/Redmine/Api/IssueStatus.php b/src/Redmine/Api/IssueStatus.php index 3d298eea..a587705c 100644 --- a/src/Redmine/Api/IssueStatus.php +++ b/src/Redmine/Api/IssueStatus.php @@ -116,12 +116,17 @@ public function listing($forceUpdate = false) /** * Get a status id given its name. * + * @deprecated v2.7.0 Use listNames() instead. + * @see IssueStatus::listNames() + * * @param string $name * * @return int|false */ public function getIdByName($name) { + @trigger_error('`' . __METHOD__ . '()` is deprecated since v2.7.0, use `' . __CLASS__ . '::listNames()` instead.', E_USER_DEPRECATED); + $arr = $this->doListing(false); if (!isset($arr[$name])) { diff --git a/tests/Unit/Api/IssueStatusTest.php b/tests/Unit/Api/IssueStatusTest.php index e292ee0b..8ef04106 100644 --- a/tests/Unit/Api/IssueStatusTest.php +++ b/tests/Unit/Api/IssueStatusTest.php @@ -277,4 +277,33 @@ public function testGetIdByNameMakesGetRequest() $this->assertFalse($api->getIdByName('IssueStatus 1')); $this->assertSame(5, $api->getIdByName('IssueStatus 5')); } + + public function testGetIdByNameTriggersDeprecationWarning() + { + $client = $this->createMock(Client::class); + $client->method('requestGet') + ->willReturn(true); + $client->method('getLastResponseBody') + ->willReturn('{"issue_statuses":[{"id":1,"name":"IssueStatus 1"},{"id":5,"name":"IssueStatus 5"}]}'); + $client->method('getLastResponseContentType') + ->willReturn('application/json'); + + $api = new IssueStatus($client); + + // PHPUnit 10 compatible way to test trigger_error(). + set_error_handler( + function ($errno, $errstr): bool { + $this->assertSame( + '`Redmine\Api\IssueStatus::getIdByName()` is deprecated since v2.7.0, use `Redmine\Api\IssueStatus::listNames()` instead.', + $errstr, + ); + + restore_error_handler(); + return true; + }, + E_USER_DEPRECATED, + ); + + $api->getIdByName('IssueStatus 5'); + } } From aa78b8150ee337f66772cbb01f287aa125810b67 Mon Sep 17 00:00:00 2001 From: Art4 Date: Tue, 9 Jul 2024 11:26:09 +0200 Subject: [PATCH 11/24] Update CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5729075c..21182c03 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `Redmine\Api\IssueCategory::listing()` is deprecated, use `\Redmine\Api\IssueCategory::listNamesByProject()` instead. - `Redmine\Api\IssueCategory::getIdByName()` is deprecated, use `\Redmine\Api\IssueCategory::listNamesByProject()` instead. - `Redmine\Api\IssueStatus::listing()` is deprecated, use `\Redmine\Api\IssueStatus::listNames()` instead. +- `Redmine\Api\IssueStatus::getIdByName()` is deprecated, use `\Redmine\Api\IssueStatus::listNames()` instead. - `Redmine\Api\Project::listing()` is deprecated, use `\Redmine\Api\Project::listNames()` instead. - `Redmine\Api\Role::listing()` is deprecated, use `\Redmine\Api\Role::listNames()` instead. - `Redmine\Api\TimeEntryActivity::listing()` is deprecated, use `\Redmine\Api\TimeEntryActivity::listNames()` instead. From 516115e395159ced319d034053e1d06ab03f66ff Mon Sep 17 00:00:00 2001 From: Art4 Date: Tue, 9 Jul 2024 11:36:48 +0200 Subject: [PATCH 12/24] Extract Project::listing() --- src/Redmine/Api/CustomField.php | 2 +- src/Redmine/Api/IssueCategory.php | 4 ++-- src/Redmine/Api/Project.php | 28 ++++++++++++++++++---------- 3 files changed, 21 insertions(+), 13 deletions(-) diff --git a/src/Redmine/Api/CustomField.php b/src/Redmine/Api/CustomField.php index 7855de3b..da88d5b6 100644 --- a/src/Redmine/Api/CustomField.php +++ b/src/Redmine/Api/CustomField.php @@ -138,7 +138,7 @@ public function getIdByName($name, array $params = []) return $arr[(string) $name]; } - private function doListing($forceUpdate = false, array $params = []) + private function doListing(bool $forceUpdate, array $params) { if (empty($this->customFields) || $forceUpdate) { $this->customFields = $this->list($params); diff --git a/src/Redmine/Api/IssueCategory.php b/src/Redmine/Api/IssueCategory.php index 058b8ca3..b86f9df4 100644 --- a/src/Redmine/Api/IssueCategory.php +++ b/src/Redmine/Api/IssueCategory.php @@ -155,7 +155,7 @@ public function getIdByName($project, $name) { @trigger_error('`' . __METHOD__ . '()` is deprecated since v2.7.0, use `' . __CLASS__ . '::listNamesByProject()` instead.', E_USER_DEPRECATED); - $arr = $this->doListing($project); + $arr = $this->doListing($project, false); if (!isset($arr[$name])) { return false; @@ -282,7 +282,7 @@ public function remove($id, array $params = []) return $this->lastResponse->getContent(); } - private function doListing($project, $forceUpdate = false) + private function doListing($project, bool $forceUpdate) { if (true === $forceUpdate || empty($this->issueCategories)) { $this->issueCategories = $this->listByProject($project); diff --git a/src/Redmine/Api/Project.php b/src/Redmine/Api/Project.php index b224e067..4250aa9a 100755 --- a/src/Redmine/Api/Project.php +++ b/src/Redmine/Api/Project.php @@ -132,15 +132,7 @@ public function listing($forceUpdate = false, $reverse = true, array $params = [ { @trigger_error('`' . __METHOD__ . '()` is deprecated since v2.7.0, use `' . __CLASS__ . '::listNames()` instead.', E_USER_DEPRECATED); - if (true === $forceUpdate || empty($this->projects)) { - $this->projects = $this->list($params); - } - $ret = []; - foreach ($this->projects['projects'] as $e) { - $ret[(int) $e['id']] = $e['name']; - } - - return $reverse ? array_flip($ret) : $ret; + return $this->doListing($forceUpdate, $reverse, $params); } /** @@ -153,7 +145,8 @@ public function listing($forceUpdate = false, $reverse = true, array $params = [ */ public function getIdByName($name, array $params = []) { - $arr = $this->listing(false, true, $params); + $arr = $this->doListing(false, true, $params); + if (!isset($arr[$name])) { return false; } @@ -447,4 +440,19 @@ public function remove($id) return $this->lastResponse->getContent(); } + + private function doListing(bool $forceUpdate, bool $reverse, array $params) + { + if (true === $forceUpdate || empty($this->projects)) { + $this->projects = $this->list($params); + } + + $ret = []; + + foreach ($this->projects['projects'] as $e) { + $ret[(int) $e['id']] = $e['name']; + } + + return $reverse ? array_flip($ret) : $ret; + } } From 4dddac4e831d920314b8a5923d6a8e4e8f7dd206 Mon Sep 17 00:00:00 2001 From: Art4 Date: Tue, 9 Jul 2024 12:34:09 +0200 Subject: [PATCH 13/24] Replace Project::getIdByName() with listNames() --- src/Redmine/Api/Issue.php | 7 ++++++- tests/Unit/Api/Issue/CreateTest.php | 4 ++-- tests/Unit/Api/Issue/UpdateTest.php | 2 +- tests/Unit/Api/IssueTest.php | 18 ++++++++++++------ 4 files changed, 21 insertions(+), 10 deletions(-) diff --git a/src/Redmine/Api/Issue.php b/src/Redmine/Api/Issue.php index 5572f2ec..b437d733 100644 --- a/src/Redmine/Api/Issue.php +++ b/src/Redmine/Api/Issue.php @@ -348,7 +348,12 @@ private function cleanParams(array $params = []) if (isset($params['project'])) { $projectApi = $this->getProjectApi(); - $params['project_id'] = $projectApi->getIdByName($params['project']); + // TODO: project names are not unique; there could be collisions + $params['project_id'] = array_search( + $params['project'], + $projectApi->listNames(), + true, + ); unset($params['project']); } diff --git a/tests/Unit/Api/Issue/CreateTest.php b/tests/Unit/Api/Issue/CreateTest.php index f3f2d12c..04275625 100644 --- a/tests/Unit/Api/Issue/CreateTest.php +++ b/tests/Unit/Api/Issue/CreateTest.php @@ -299,7 +299,7 @@ public function testCreateWithHttpClientRetrievesProjectId() $this, [ 'GET', - '/projects.json', + '/projects.json?limit=100&offset=0', 'application/json', '', 200, @@ -450,7 +450,7 @@ public function testCreateWithClientCleansParameters() $this, [ 'GET', - '/projects.json', + '/projects.json?limit=100&offset=0', 'application/json', '', 200, diff --git a/tests/Unit/Api/Issue/UpdateTest.php b/tests/Unit/Api/Issue/UpdateTest.php index ffdc446c..3897ad6b 100644 --- a/tests/Unit/Api/Issue/UpdateTest.php +++ b/tests/Unit/Api/Issue/UpdateTest.php @@ -119,7 +119,7 @@ public function testUpdateCleansParameters() $this, [ 'GET', - '/projects.json', + '/projects.json?limit=100&offset=0', 'application/json', '', 200, diff --git a/tests/Unit/Api/IssueTest.php b/tests/Unit/Api/IssueTest.php index 675bbb8d..7003d87a 100644 --- a/tests/Unit/Api/IssueTest.php +++ b/tests/Unit/Api/IssueTest.php @@ -8,6 +8,7 @@ use Redmine\Api\Issue; use Redmine\Api\IssueCategory; use Redmine\Api\IssueStatus; +use Redmine\Api\Project; use Redmine\Client\Client; use Redmine\Http\HttpClient; use Redmine\Http\Response; @@ -146,7 +147,7 @@ public function testCreateWithClientCleansParameters() // Test values $response = ''; $parameters = [ - 'project' => 'Project Name', + 'project' => 'Project 1 Name', 'category' => 'Category 5 Name', 'status' => 'Status 6 Name', 'tracker' => 'Tracker Name', @@ -155,10 +156,6 @@ public function testCreateWithClientCleansParameters() ]; // Create the used mock objects - $projectApi = $this->createMock('Redmine\Api\Project'); - $projectApi->expects($this->exactly(1)) - ->method('getIdByName') - ->willReturn(1); $getIdByNameApi = $this->createMock('Redmine\Api\Tracker'); $getIdByNameApi->expects($this->exactly(1)) ->method('getIdByName') @@ -170,6 +167,15 @@ public function testCreateWithClientCleansParameters() $httpClient = AssertingHttpClient::create( $this, + [ + 'GET', + '/projects.json?limit=100&offset=0', + 'application/json', + '', + 200, + 'application/json', + '{"projects":[{"id":1,"name":"Project 1 Name"},{"id":2,"name":"Project 1 Name"}]}', + ], [ 'GET', '/projects/1/issue_categories.json', @@ -195,7 +201,7 @@ public function testCreateWithClientCleansParameters() ->method('getApi') ->willReturnMap( [ - ['project', $projectApi], + ['project', new Project($httpClient)], ['issue_category', new IssueCategory($httpClient)], ['issue_status', new IssueStatus($httpClient)], ['tracker', $getIdByNameApi], From 97fa124df405aefc518bbf071d1ba5804eec63b2 Mon Sep 17 00:00:00 2001 From: Art4 Date: Tue, 9 Jul 2024 12:39:02 +0200 Subject: [PATCH 14/24] Deprecate Project::getIdByName() --- CHANGELOG.md | 1 + src/Redmine/Api/Project.php | 5 +++++ tests/Unit/Api/ProjectTest.php | 29 +++++++++++++++++++++++++++++ 3 files changed, 35 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 21182c03..74aada9b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `Redmine\Api\IssueStatus::listing()` is deprecated, use `\Redmine\Api\IssueStatus::listNames()` instead. - `Redmine\Api\IssueStatus::getIdByName()` is deprecated, use `\Redmine\Api\IssueStatus::listNames()` instead. - `Redmine\Api\Project::listing()` is deprecated, use `\Redmine\Api\Project::listNames()` instead. +- `Redmine\Api\Project::getIdByName()` is deprecated, use `\Redmine\Api\Project::listNames()` instead. - `Redmine\Api\Role::listing()` is deprecated, use `\Redmine\Api\Role::listNames()` instead. - `Redmine\Api\TimeEntryActivity::listing()` is deprecated, use `\Redmine\Api\TimeEntryActivity::listNames()` instead. - `Redmine\Api\Tracker::listing()` is deprecated, use `\Redmine\Api\Tracker::listNames()` instead. diff --git a/src/Redmine/Api/Project.php b/src/Redmine/Api/Project.php index 4250aa9a..62f83f11 100755 --- a/src/Redmine/Api/Project.php +++ b/src/Redmine/Api/Project.php @@ -138,6 +138,9 @@ public function listing($forceUpdate = false, $reverse = true, array $params = [ /** * Get a project id given its name. * + * @deprecated v2.7.0 Use listNames() instead. + * @see Project::listNames() + * * @param string $name * @param array $params to allow offset/limit (and more) to be passed * @@ -145,6 +148,8 @@ public function listing($forceUpdate = false, $reverse = true, array $params = [ */ public function getIdByName($name, array $params = []) { + @trigger_error('`' . __METHOD__ . '()` is deprecated since v2.7.0, use `' . __CLASS__ . '::listNames()` instead.', E_USER_DEPRECATED); + $arr = $this->doListing(false, true, $params); if (!isset($arr[$name])) { diff --git a/tests/Unit/Api/ProjectTest.php b/tests/Unit/Api/ProjectTest.php index cf5101b4..1c05a772 100644 --- a/tests/Unit/Api/ProjectTest.php +++ b/tests/Unit/Api/ProjectTest.php @@ -280,6 +280,35 @@ public function testGetIdByNameMakesGetRequest() $this->assertSame(5, $api->getIdByName('Project 5')); } + public function testGetIdByNameTriggersDeprecationWarning() + { + $client = $this->createMock(Client::class); + $client->method('requestGet') + ->willReturn(true); + $client->method('getLastResponseBody') + ->willReturn('{"projects":[{"id":1,"name":"Project 1"},{"id":5,"name":"Project 5"}]}'); + $client->method('getLastResponseContentType') + ->willReturn('application/json'); + + $api = new Project($client); + + // PHPUnit 10 compatible way to test trigger_error(). + set_error_handler( + function ($errno, $errstr): bool { + $this->assertSame( + '`Redmine\Api\Project::getIdByName()` is deprecated since v2.7.0, use `Redmine\Api\Project::listNames()` instead.', + $errstr, + ); + + restore_error_handler(); + return true; + }, + E_USER_DEPRECATED, + ); + + $api->getIdByName('Project 1'); + } + public function testDeprecatedPrepareParamsXml() { $client = $this->createMock(Client::class); From 60d4b07408693c590f22f4fdd48d54b5195fb022 Mon Sep 17 00:00:00 2001 From: Art4 Date: Tue, 9 Jul 2024 12:46:25 +0200 Subject: [PATCH 15/24] Extract TimeEntryActivity::listing() --- src/Redmine/Api/TimeEntryActivity.php | 28 +++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/src/Redmine/Api/TimeEntryActivity.php b/src/Redmine/Api/TimeEntryActivity.php index 511d5e21..a2446acb 100644 --- a/src/Redmine/Api/TimeEntryActivity.php +++ b/src/Redmine/Api/TimeEntryActivity.php @@ -105,15 +105,7 @@ public function listing($forceUpdate = false) { @trigger_error('`' . __METHOD__ . '()` is deprecated since v2.7.0, use `' . __CLASS__ . '::listNames()` instead.', E_USER_DEPRECATED); - if (empty($this->timeEntryActivities) || $forceUpdate) { - $this->timeEntryActivities = $this->list(); - } - $ret = []; - foreach ($this->timeEntryActivities['time_entry_activities'] as $e) { - $ret[$e['name']] = (int) $e['id']; - } - - return $ret; + return $this->doListing((bool) $forceUpdate); } /** @@ -125,11 +117,27 @@ public function listing($forceUpdate = false) */ public function getIdByName($name) { - $arr = $this->listing(); + $arr = $this->doListing(false); + if (!isset($arr[$name])) { return false; } return $arr[(string) $name]; } + + private function doListing(bool $forceUpdate) + { + if (empty($this->timeEntryActivities) || $forceUpdate) { + $this->timeEntryActivities = $this->list(); + } + + $ret = []; + + foreach ($this->timeEntryActivities['time_entry_activities'] as $e) { + $ret[$e['name']] = (int) $e['id']; + } + + return $ret; + } } From 8132e67cce788e90fba9935438c14ff93faff40d Mon Sep 17 00:00:00 2001 From: Art4 Date: Tue, 9 Jul 2024 12:50:12 +0200 Subject: [PATCH 16/24] Deprecate TimeEntryActivity::getIdByName() --- CHANGELOG.md | 1 + src/Redmine/Api/TimeEntryActivity.php | 5 ++++ tests/Unit/Api/TimeEntryActivityTest.php | 29 ++++++++++++++++++++++++ 3 files changed, 35 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 74aada9b..7222a587 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,6 +33,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `Redmine\Api\Project::getIdByName()` is deprecated, use `\Redmine\Api\Project::listNames()` instead. - `Redmine\Api\Role::listing()` is deprecated, use `\Redmine\Api\Role::listNames()` instead. - `Redmine\Api\TimeEntryActivity::listing()` is deprecated, use `\Redmine\Api\TimeEntryActivity::listNames()` instead. +- `Redmine\Api\TimeEntryActivity::getIdByName()` is deprecated, use `\Redmine\Api\TimeEntryActivity::listNames()` instead. - `Redmine\Api\Tracker::listing()` is deprecated, use `\Redmine\Api\Tracker::listNames()` instead. - `Redmine\Api\User::listing()` is deprecated, use `\Redmine\Api\User::listLogins()` instead. - `Redmine\Api\Version::listing()` is deprecated, use `\Redmine\Api\Version::listNamesByProject()` instead. diff --git a/src/Redmine/Api/TimeEntryActivity.php b/src/Redmine/Api/TimeEntryActivity.php index a2446acb..93ea2a35 100644 --- a/src/Redmine/Api/TimeEntryActivity.php +++ b/src/Redmine/Api/TimeEntryActivity.php @@ -111,12 +111,17 @@ public function listing($forceUpdate = false) /** * Get a activities id given its name. * + * @deprecated v2.7.0 Use listNames() instead. + * @see Project::listNames() + * * @param string $name * * @return int|false */ public function getIdByName($name) { + @trigger_error('`' . __METHOD__ . '()` is deprecated since v2.7.0, use `' . __CLASS__ . '::listNames()` instead.', E_USER_DEPRECATED); + $arr = $this->doListing(false); if (!isset($arr[$name])) { diff --git a/tests/Unit/Api/TimeEntryActivityTest.php b/tests/Unit/Api/TimeEntryActivityTest.php index 841042cd..a300427c 100644 --- a/tests/Unit/Api/TimeEntryActivityTest.php +++ b/tests/Unit/Api/TimeEntryActivityTest.php @@ -221,4 +221,33 @@ public function testGetIdByNameMakesGetRequest() $this->assertFalse($api->getIdByName('TimeEntryActivities 1')); $this->assertSame(2, $api->getIdByName('TimeEntryActivities 2')); } + + public function testGetIdByNameTriggersDeprecationWarning() + { + $client = $this->createMock(Client::class); + $client->method('requestGet') + ->willReturn(true); + $client->method('getLastResponseBody') + ->willReturn('{"time_entry_activities":[{"id":1,"name":"TimeEntryActivity 1"},{"id":5,"name":"TimeEntryActivity 5"}]}'); + $client->method('getLastResponseContentType') + ->willReturn('application/json'); + + $api = new TimeEntryActivity($client); + + // PHPUnit 10 compatible way to test trigger_error(). + set_error_handler( + function ($errno, $errstr): bool { + $this->assertSame( + '`Redmine\Api\TimeEntryActivity::getIdByName()` is deprecated since v2.7.0, use `Redmine\Api\TimeEntryActivity::listNames()` instead.', + $errstr, + ); + + restore_error_handler(); + return true; + }, + E_USER_DEPRECATED, + ); + + $api->getIdByName('TimeEntryActivities 2'); + } } From 70f385b96057de71a6bb7cc71c5a0cf41698ece9 Mon Sep 17 00:00:00 2001 From: Art4 Date: Tue, 9 Jul 2024 12:54:53 +0200 Subject: [PATCH 17/24] Extract Tracker::listing() --- src/Redmine/Api/Tracker.php | 28 ++++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/src/Redmine/Api/Tracker.php b/src/Redmine/Api/Tracker.php index 1babf0d3..3e1e80cb 100644 --- a/src/Redmine/Api/Tracker.php +++ b/src/Redmine/Api/Tracker.php @@ -109,15 +109,7 @@ public function listing($forceUpdate = false) { @trigger_error('`' . __METHOD__ . '()` is deprecated since v2.7.0, use `' . __CLASS__ . '::listNames()` instead.', E_USER_DEPRECATED); - if (empty($this->trackers) || $forceUpdate) { - $this->trackers = $this->list(); - } - $ret = []; - foreach ($this->trackers['trackers'] as $e) { - $ret[$e['name']] = (int) $e['id']; - } - - return $ret; + return $this->doListing($forceUpdate); } /** @@ -129,11 +121,27 @@ public function listing($forceUpdate = false) */ public function getIdByName($name) { - $arr = $this->listing(); + $arr = $this->doListing(false); + if (!isset($arr[$name])) { return false; } return $arr[(string) $name]; } + + private function doListing(bool $forceUpdate) + { + if (empty($this->trackers) || $forceUpdate) { + $this->trackers = $this->list(); + } + + $ret = []; + + foreach ($this->trackers['trackers'] as $e) { + $ret[$e['name']] = (int) $e['id']; + } + + return $ret; + } } From b1e725a5d0744f392e0051c6aa6b142df1bcdc2e Mon Sep 17 00:00:00 2001 From: Art4 Date: Tue, 9 Jul 2024 13:02:25 +0200 Subject: [PATCH 18/24] Replace Tracker::getIdByName() with Tracker::listNames() --- src/Redmine/Api/Issue.php | 6 +++++- tests/Unit/Api/IssueTest.php | 20 +++++++++++++------- 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/src/Redmine/Api/Issue.php b/src/Redmine/Api/Issue.php index b437d733..e3dfe3a5 100644 --- a/src/Redmine/Api/Issue.php +++ b/src/Redmine/Api/Issue.php @@ -382,7 +382,11 @@ private function cleanParams(array $params = []) if (isset($params['tracker'])) { $trackerApi = $this->getTrackerApi(); - $params['tracker_id'] = $trackerApi->getIdByName($params['tracker']); + $params['tracker_id'] = array_search( + $params['tracker'], + $trackerApi->listNames(), + true, + ); unset($params['tracker']); } diff --git a/tests/Unit/Api/IssueTest.php b/tests/Unit/Api/IssueTest.php index 7003d87a..49b88ac8 100644 --- a/tests/Unit/Api/IssueTest.php +++ b/tests/Unit/Api/IssueTest.php @@ -9,6 +9,7 @@ use Redmine\Api\IssueCategory; use Redmine\Api\IssueStatus; use Redmine\Api\Project; +use Redmine\Api\Tracker; use Redmine\Client\Client; use Redmine\Http\HttpClient; use Redmine\Http\Response; @@ -150,16 +151,12 @@ public function testCreateWithClientCleansParameters() 'project' => 'Project 1 Name', 'category' => 'Category 5 Name', 'status' => 'Status 6 Name', - 'tracker' => 'Tracker Name', + 'tracker' => 'Tracker 2 Name', 'assigned_to' => 'Assigned to User Name', 'author' => 'Author Name', ]; // Create the used mock objects - $getIdByNameApi = $this->createMock('Redmine\Api\Tracker'); - $getIdByNameApi->expects($this->exactly(1)) - ->method('getIdByName') - ->willReturn('cleanedValue'); $getIdByUsernameApi = $this->createMock('Redmine\Api\User'); $getIdByUsernameApi->expects($this->exactly(2)) ->method('getIdByUsername') @@ -194,6 +191,15 @@ public function testCreateWithClientCleansParameters() 'application/json', '{"issue_statuses":[{"id":6,"name":"Status 6 Name"}]}', ], + [ + 'GET', + '/trackers.json', + 'application/json', + '', + 200, + 'application/json', + '{"trackers":[{"id":2,"name":"Tracker 2 Name"}]}', + ], ); $client = $this->createMock(Client::class); @@ -204,7 +210,7 @@ public function testCreateWithClientCleansParameters() ['project', new Project($httpClient)], ['issue_category', new IssueCategory($httpClient)], ['issue_status', new IssueStatus($httpClient)], - ['tracker', $getIdByNameApi], + ['tracker', new Tracker($httpClient)], ['user', $getIdByUsernameApi], ], ) @@ -216,7 +222,7 @@ public function testCreateWithClientCleansParameters() '/issues.xml', <<< XML - 156cleanedValuecleanedValuecleanedValue + 1562cleanedValuecleanedValue XML, ) From 1bacd639f9e3259e417ec52e1b18b664419fa823 Mon Sep 17 00:00:00 2001 From: Art4 Date: Tue, 9 Jul 2024 13:06:52 +0200 Subject: [PATCH 19/24] Deprecate Tracker::getIdByName() --- CHANGELOG.md | 1 + src/Redmine/Api/Tracker.php | 7 ++++++- tests/Unit/Api/TrackerTest.php | 29 +++++++++++++++++++++++++++++ 3 files changed, 36 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7222a587..58914efd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,6 +35,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `Redmine\Api\TimeEntryActivity::listing()` is deprecated, use `\Redmine\Api\TimeEntryActivity::listNames()` instead. - `Redmine\Api\TimeEntryActivity::getIdByName()` is deprecated, use `\Redmine\Api\TimeEntryActivity::listNames()` instead. - `Redmine\Api\Tracker::listing()` is deprecated, use `\Redmine\Api\Tracker::listNames()` instead. +- `Redmine\Api\Tracker::getIdByName()` is deprecated, use `\Redmine\Api\Tracker::listNames()` instead. - `Redmine\Api\User::listing()` is deprecated, use `\Redmine\Api\User::listLogins()` instead. - `Redmine\Api\Version::listing()` is deprecated, use `\Redmine\Api\Version::listNamesByProject()` instead. diff --git a/src/Redmine/Api/Tracker.php b/src/Redmine/Api/Tracker.php index 3e1e80cb..72a46f4a 100644 --- a/src/Redmine/Api/Tracker.php +++ b/src/Redmine/Api/Tracker.php @@ -113,7 +113,10 @@ public function listing($forceUpdate = false) } /** - * Get a tracket id given its name. + * Get a tracker id given its name. + * + * @deprecated v2.7.0 Use listNames() instead. + * @see Tracker::listNames() * * @param string|int $name tracker name * @@ -121,6 +124,8 @@ public function listing($forceUpdate = false) */ public function getIdByName($name) { + @trigger_error('`' . __METHOD__ . '()` is deprecated since v2.7.0, use `' . __CLASS__ . '::listNames()` instead.', E_USER_DEPRECATED); + $arr = $this->doListing(false); if (!isset($arr[$name])) { diff --git a/tests/Unit/Api/TrackerTest.php b/tests/Unit/Api/TrackerTest.php index dc46e170..5ba445b2 100644 --- a/tests/Unit/Api/TrackerTest.php +++ b/tests/Unit/Api/TrackerTest.php @@ -277,4 +277,33 @@ public function testGetIdByNameMakesGetRequest() $this->assertFalse($api->getIdByName('Tracker 1')); $this->assertSame(5, $api->getIdByName('Tracker 5')); } + + public function testGgetIdByNameTriggersDeprecationWarning() + { + $client = $this->createMock(Client::class); + $client->method('requestGet') + ->willReturn(true); + $client->method('getLastResponseBody') + ->willReturn('{"trackers":[{"id":1,"name":"Tracker 1"},{"id":5,"name":"Tracker 5"}]}'); + $client->method('getLastResponseContentType') + ->willReturn('application/json'); + + $api = new Tracker($client); + + // PHPUnit 10 compatible way to test trigger_error(). + set_error_handler( + function ($errno, $errstr): bool { + $this->assertSame( + '`Redmine\Api\Tracker::getIdByName()` is deprecated since v2.7.0, use `Redmine\Api\Tracker::listNames()` instead.', + $errstr, + ); + + restore_error_handler(); + return true; + }, + E_USER_DEPRECATED, + ); + + $api->getIdByName('Tracker 5'); + } } From 057a28946819a3e26a4df93a7071f9310a3c0ace Mon Sep 17 00:00:00 2001 From: Art4 Date: Tue, 9 Jul 2024 13:10:06 +0200 Subject: [PATCH 20/24] Extract User::listing() --- src/Redmine/Api/User.php | 32 ++++++++++++++++++++------------ 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/src/Redmine/Api/User.php b/src/Redmine/Api/User.php index d6d52d3f..d3c99254 100644 --- a/src/Redmine/Api/User.php +++ b/src/Redmine/Api/User.php @@ -130,17 +130,7 @@ public function listing($forceUpdate = false, array $params = []) { @trigger_error('`' . __METHOD__ . '()` is deprecated since v2.7.0, use `' . __CLASS__ . '::listLogins()` instead.', E_USER_DEPRECATED); - if (empty($this->users) || $forceUpdate) { - $this->users = $this->list($params); - } - $ret = []; - if (is_array($this->users) && isset($this->users['users'])) { - foreach ($this->users['users'] as $e) { - $ret[$e['login']] = (int) $e['id']; - } - } - - return $ret; + return $this->doListing($forceUpdate, $params); } /** @@ -167,7 +157,8 @@ public function getCurrentUser(array $params = []) */ public function getIdByUsername($username, array $params = []) { - $arr = $this->listing(false, $params); + $arr = $this->doListing(false, $params); + if (!isset($arr[$username])) { return false; } @@ -318,4 +309,21 @@ public function remove($id) return $this->lastResponse->getContent(); } + + private function doListing(bool $forceUpdate, array $params) + { + if (empty($this->users) || $forceUpdate) { + $this->users = $this->list($params); + } + + $ret = []; + + if (is_array($this->users) && isset($this->users['users'])) { + foreach ($this->users['users'] as $e) { + $ret[$e['login']] = (int) $e['id']; + } + } + + return $ret; + } } From 5913938078149254e4bb77d8432de460bfa0fd34 Mon Sep 17 00:00:00 2001 From: Art4 Date: Tue, 9 Jul 2024 13:20:51 +0200 Subject: [PATCH 21/24] Replace User::getIdByUsername() with User::listLogins() --- src/Redmine/Api/Issue.php | 12 ++++++++++-- tests/Unit/Api/Issue/CreateTest.php | 14 +++++++------- tests/Unit/Api/Issue/UpdateTest.php | 2 +- tests/Unit/Api/IssueTest.php | 23 ++++++++++++++--------- 4 files changed, 32 insertions(+), 19 deletions(-) diff --git a/src/Redmine/Api/Issue.php b/src/Redmine/Api/Issue.php index e3dfe3a5..fde13968 100644 --- a/src/Redmine/Api/Issue.php +++ b/src/Redmine/Api/Issue.php @@ -393,14 +393,22 @@ private function cleanParams(array $params = []) if (isset($params['assigned_to'])) { $userApi = $this->getUserApi(); - $params['assigned_to_id'] = $userApi->getIdByUsername($params['assigned_to']); + $params['assigned_to_id'] = array_search( + $params['assigned_to'], + $userApi->listLogins(), + true, + ); unset($params['assigned_to']); } if (isset($params['author'])) { $userApi = $this->getUserApi(); - $params['author_id'] = $userApi->getIdByUsername($params['author']); + $params['author_id'] = array_search( + $params['author'], + $userApi->listLogins(), + true, + ); unset($params['author']); } diff --git a/tests/Unit/Api/Issue/CreateTest.php b/tests/Unit/Api/Issue/CreateTest.php index 04275625..4510d2d3 100644 --- a/tests/Unit/Api/Issue/CreateTest.php +++ b/tests/Unit/Api/Issue/CreateTest.php @@ -410,12 +410,12 @@ public function testCreateWithHttpClientRetrievesUserId() $this, [ 'GET', - '/users.json', + '/users.json?limit=100&offset=0', 'application/json', '', 200, 'application/json', - '{"users":[{"login":"Author Name","id":5},{"login":"Assigned to User Name","id":6}]}', + '{"users":[{"login":"user_5","id":5},{"login":"user_6","id":6}]}', ], [ 'POST', @@ -432,7 +432,7 @@ public function testCreateWithHttpClientRetrievesUserId() $api = new Issue($client); // Perform the tests - $xmlElement = $api->create(['assigned_to' => 'Assigned to User Name', 'author' => 'Author Name']); + $xmlElement = $api->create(['assigned_to' => 'user_6', 'author' => 'user_5']); $this->assertInstanceOf(SimpleXMLElement::class, $xmlElement); $this->assertXmlStringEqualsXmlString( @@ -486,12 +486,12 @@ public function testCreateWithClientCleansParameters() ], [ 'GET', - '/users.json', + '/users.json?limit=100&offset=0', 'application/json', '', 200, 'application/json', - '{"users":[{"login":"Author Name","id":5},{"login":"Assigned to User Name","id":6}]}', + '{"users":[{"login":"user_5","id":5},{"login":"user_6","id":6}]}', ], [ 'POST', @@ -519,8 +519,8 @@ public function testCreateWithClientCleansParameters() 'category' => 'Category Name', 'status' => 'Status Name', 'tracker' => 'Tracker Name', - 'assigned_to' => 'Assigned to User Name', - 'author' => 'Author Name', + 'assigned_to' => 'user_6', + 'author' => 'user_5', ]; // Create the object under test diff --git a/tests/Unit/Api/Issue/UpdateTest.php b/tests/Unit/Api/Issue/UpdateTest.php index 3897ad6b..5a2e6da3 100644 --- a/tests/Unit/Api/Issue/UpdateTest.php +++ b/tests/Unit/Api/Issue/UpdateTest.php @@ -155,7 +155,7 @@ public function testUpdateCleansParameters() ], [ 'GET', - '/users.json', + '/users.json?limit=100&offset=0', 'application/json', '', 200, diff --git a/tests/Unit/Api/IssueTest.php b/tests/Unit/Api/IssueTest.php index 49b88ac8..b00d8c21 100644 --- a/tests/Unit/Api/IssueTest.php +++ b/tests/Unit/Api/IssueTest.php @@ -10,6 +10,7 @@ use Redmine\Api\IssueStatus; use Redmine\Api\Project; use Redmine\Api\Tracker; +use Redmine\Api\User; use Redmine\Client\Client; use Redmine\Http\HttpClient; use Redmine\Http\Response; @@ -152,16 +153,11 @@ public function testCreateWithClientCleansParameters() 'category' => 'Category 5 Name', 'status' => 'Status 6 Name', 'tracker' => 'Tracker 2 Name', - 'assigned_to' => 'Assigned to User Name', - 'author' => 'Author Name', + 'assigned_to' => 'user_3', + 'author' => 'user_4', ]; // Create the used mock objects - $getIdByUsernameApi = $this->createMock('Redmine\Api\User'); - $getIdByUsernameApi->expects($this->exactly(2)) - ->method('getIdByUsername') - ->willReturn('cleanedValue'); - $httpClient = AssertingHttpClient::create( $this, [ @@ -200,6 +196,15 @@ public function testCreateWithClientCleansParameters() 'application/json', '{"trackers":[{"id":2,"name":"Tracker 2 Name"}]}', ], + [ + 'GET', + '/users.json?limit=100&offset=0', + 'application/json', + '', + 200, + 'application/json', + '{"users":[{"id":3,"login":"user_3"},{"id":4,"login":"user_4"}]}', + ], ); $client = $this->createMock(Client::class); @@ -211,7 +216,7 @@ public function testCreateWithClientCleansParameters() ['issue_category', new IssueCategory($httpClient)], ['issue_status', new IssueStatus($httpClient)], ['tracker', new Tracker($httpClient)], - ['user', $getIdByUsernameApi], + ['user', new User($httpClient)], ], ) ; @@ -222,7 +227,7 @@ public function testCreateWithClientCleansParameters() '/issues.xml', <<< XML - 1562cleanedValuecleanedValue + 156234 XML, ) From 444192172d19ccc51f14009b537e16f5da766c3c Mon Sep 17 00:00:00 2001 From: Art4 Date: Tue, 9 Jul 2024 13:25:43 +0200 Subject: [PATCH 22/24] Deprecate User::getIdByUsername() --- CHANGELOG.md | 1 + src/Redmine/Api/User.php | 5 ++++ tests/Unit/Api/UserTest.php | 53 ++++++++++++++++++++++++++++--------- 3 files changed, 47 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 58914efd..3a3323d6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,6 +37,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `Redmine\Api\Tracker::listing()` is deprecated, use `\Redmine\Api\Tracker::listNames()` instead. - `Redmine\Api\Tracker::getIdByName()` is deprecated, use `\Redmine\Api\Tracker::listNames()` instead. - `Redmine\Api\User::listing()` is deprecated, use `\Redmine\Api\User::listLogins()` instead. +- `Redmine\Api\User::getIdByUsername()` is deprecated, use `\Redmine\Api\User::listLogins()` instead. - `Redmine\Api\Version::listing()` is deprecated, use `\Redmine\Api\Version::listNamesByProject()` instead. ## [v2.6.0](https://github.com/kbsali/php-redmine-api/compare/v2.5.0...v2.6.0) - 2024-03-25 diff --git a/src/Redmine/Api/User.php b/src/Redmine/Api/User.php index d3c99254..027b7aca 100644 --- a/src/Redmine/Api/User.php +++ b/src/Redmine/Api/User.php @@ -150,6 +150,9 @@ public function getCurrentUser(array $params = []) /** * Get a user id given its username. * + * @deprecated v2.7.0 Use listLogins() instead. + * @see User::listLogins() + * * @param string $username * @param array $params to allow offset/limit (and more) to be passed * @@ -157,6 +160,8 @@ public function getCurrentUser(array $params = []) */ public function getIdByUsername($username, array $params = []) { + @trigger_error('`' . __METHOD__ . '()` is deprecated since v2.7.0, use `' . __CLASS__ . '::listLogins()` instead.', E_USER_DEPRECATED); + $arr = $this->doListing(false, $params); if (!isset($arr[$username])) { diff --git a/tests/Unit/Api/UserTest.php b/tests/Unit/Api/UserTest.php index 3dc27123..5a327ae0 100644 --- a/tests/Unit/Api/UserTest.php +++ b/tests/Unit/Api/UserTest.php @@ -55,7 +55,7 @@ public function testGetCurrentUserReturnsClientGetResponse() public function testGetIdByUsernameMakesGetRequest() { // Test values - $response = '{"users":[{"id":5,"login":"User 5"}]}'; + $response = '{"users":[{"id":5,"login":"user_5"}]}'; // Create the used mock objects $client = $this->createMock(Client::class); @@ -79,8 +79,37 @@ public function testGetIdByUsernameMakesGetRequest() $api = new User($client); // Perform the tests - $this->assertFalse($api->getIdByUsername('User 1')); - $this->assertSame(5, $api->getIdByUsername('User 5')); + $this->assertFalse($api->getIdByUsername('user_1')); + $this->assertSame(5, $api->getIdByUsername('user_5')); + } + + public function testGetIdByUsernameTriggersDeprecationWarning() + { + $client = $this->createMock(Client::class); + $client->method('requestGet') + ->willReturn(true); + $client->method('getLastResponseBody') + ->willReturn('{"users":[{"id":1,"login":"user_1"},{"id":5,"login":"user_5"}]}'); + $client->method('getLastResponseContentType') + ->willReturn('application/json'); + + $api = new User($client); + + // PHPUnit 10 compatible way to test trigger_error(). + set_error_handler( + function ($errno, $errstr): bool { + $this->assertSame( + '`Redmine\Api\User::getIdByUsername()` is deprecated since v2.7.0, use `Redmine\Api\User::listLogins()` instead.', + $errstr, + ); + + restore_error_handler(); + return true; + }, + E_USER_DEPRECATED, + ); + + $api->getIdByUsername('user_5'); } /** @@ -189,10 +218,10 @@ public function testAllReturnsClientGetResponseWithParameters() public function testListingReturnsNameIdArray() { // Test values - $response = '{"users":[{"id":1,"login":"User 1"},{"id":5,"login":"User 5"}]}'; + $response = '{"users":[{"id":1,"login":"user_1"},{"id":5,"login":"user_5"}]}'; $expectedReturn = [ - 'User 1' => 1, - 'User 5' => 5, + 'user_1' => 1, + 'user_5' => 5, ]; // Create the used mock objects @@ -223,10 +252,10 @@ public function testListingReturnsNameIdArray() public function testListingCallsGetOnlyTheFirstTime() { // Test values - $response = '{"users":[{"id":1,"login":"User 1"},{"id":5,"login":"User 5"}]}'; + $response = '{"users":[{"id":1,"login":"user_1"},{"id":5,"login":"user_5"}]}'; $expectedReturn = [ - 'User 1' => 1, - 'User 5' => 5, + 'user_1' => 1, + 'user_5' => 5, ]; // Create the used mock objects @@ -258,10 +287,10 @@ public function testListingCallsGetOnlyTheFirstTime() public function testListingCallsGetEveryTimeWithForceUpdate() { // Test values - $response = '{"users":[{"id":1,"login":"User 1"},{"id":5,"login":"User 5"}]}'; + $response = '{"users":[{"id":1,"login":"user_1"},{"id":5,"login":"user_5"}]}'; $expectedReturn = [ - 'User 1' => 1, - 'User 5' => 5, + 'user_1' => 1, + 'user_5' => 5, ]; // Create the used mock objects From 64e8df4b90abc0317ba9a5df6b908263a86fa710 Mon Sep 17 00:00:00 2001 From: Art4 Date: Tue, 9 Jul 2024 13:31:37 +0200 Subject: [PATCH 23/24] Extract Version::listing() --- src/Redmine/Api/Version.php | 28 ++++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/src/Redmine/Api/Version.php b/src/Redmine/Api/Version.php index a7fd51ab..3b7a8502 100644 --- a/src/Redmine/Api/Version.php +++ b/src/Redmine/Api/Version.php @@ -139,15 +139,7 @@ public function listing($project, $forceUpdate = false, $reverse = true, array $ { @trigger_error('`' . __METHOD__ . '()` is deprecated since v2.7.0, use `' . __CLASS__ . '::listNamesByProject()` instead.', E_USER_DEPRECATED); - if (true === $forceUpdate || empty($this->versions)) { - $this->versions = $this->listByProject($project, $params); - } - $ret = []; - foreach ($this->versions['versions'] as $e) { - $ret[(int) $e['id']] = $e['name']; - } - - return $reverse ? array_flip($ret) : $ret; + return $this->doListing($project, $forceUpdate, $reverse, $params); } /** @@ -161,7 +153,8 @@ public function listing($project, $forceUpdate = false, $reverse = true, array $ */ public function getIdByName($project, $name, array $params = []) { - $arr = $this->listing($project, false, true, $params); + $arr = $this->doListing($project, false, true, $params); + if (!isset($arr[$name])) { return false; } @@ -319,4 +312,19 @@ public function remove($id) return $this->lastResponse->getContent(); } + + private function doListing($project, bool $forceUpdate, bool $reverse, array $params) + { + if (true === $forceUpdate || empty($this->versions)) { + $this->versions = $this->listByProject($project, $params); + } + + $ret = []; + + foreach ($this->versions['versions'] as $e) { + $ret[(int) $e['id']] = $e['name']; + } + + return $reverse ? array_flip($ret) : $ret; + } } From b11e318522c45d71d577edb84289f3979b83d234 Mon Sep 17 00:00:00 2001 From: Art4 Date: Tue, 9 Jul 2024 13:34:29 +0200 Subject: [PATCH 24/24] Deprecate Version::getIdByName() --- CHANGELOG.md | 1 + src/Redmine/Api/Version.php | 5 +++++ tests/Unit/Api/VersionTest.php | 29 +++++++++++++++++++++++++++++ 3 files changed, 35 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3a3323d6..e28570b7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,6 +39,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `Redmine\Api\User::listing()` is deprecated, use `\Redmine\Api\User::listLogins()` instead. - `Redmine\Api\User::getIdByUsername()` is deprecated, use `\Redmine\Api\User::listLogins()` instead. - `Redmine\Api\Version::listing()` is deprecated, use `\Redmine\Api\Version::listNamesByProject()` instead. +- `Redmine\Api\Version::getIdByName()` is deprecated, use `\Redmine\Api\Version::listNamesByProject()` instead. ## [v2.6.0](https://github.com/kbsali/php-redmine-api/compare/v2.5.0...v2.6.0) - 2024-03-25 diff --git a/src/Redmine/Api/Version.php b/src/Redmine/Api/Version.php index 3b7a8502..7d11cca2 100644 --- a/src/Redmine/Api/Version.php +++ b/src/Redmine/Api/Version.php @@ -145,6 +145,9 @@ public function listing($project, $forceUpdate = false, $reverse = true, array $ /** * Get an version id given its name and related project. * + * @deprecated v2.7.0 Use listNamesByProject() instead. + * @see Version::listNamesByProject() + * * @param string|int $project project id or literal identifier * @param string $name The version name * @param array $params optional parameters to be passed to the api (offset, limit, ...) @@ -153,6 +156,8 @@ public function listing($project, $forceUpdate = false, $reverse = true, array $ */ public function getIdByName($project, $name, array $params = []) { + @trigger_error('`' . __METHOD__ . '()` is deprecated since v2.7.0, use `' . __CLASS__ . '::listNamesByProject()` instead.', E_USER_DEPRECATED); + $arr = $this->doListing($project, false, true, $params); if (!isset($arr[$name])) { diff --git a/tests/Unit/Api/VersionTest.php b/tests/Unit/Api/VersionTest.php index 0ed55c06..7666934e 100644 --- a/tests/Unit/Api/VersionTest.php +++ b/tests/Unit/Api/VersionTest.php @@ -309,6 +309,35 @@ public function testGetIdByNameMakesGetRequest() $this->assertSame(5, $api->getIdByName(5, 'Version 5')); } + public function testGetIdByNameTriggersDeprecationWarning() + { + $client = $this->createMock(Client::class); + $client->method('requestGet') + ->willReturn(true); + $client->method('getLastResponseBody') + ->willReturn('{"versions":[{"id":1,"name":"Version 1"},{"id":5,"name":"Version 5"}]}'); + $client->method('getLastResponseContentType') + ->willReturn('application/json'); + + $api = new Version($client); + + // PHPUnit 10 compatible way to test trigger_error(). + set_error_handler( + function ($errno, $errstr): bool { + $this->assertSame( + '`Redmine\Api\Version::getIdByName()` is deprecated since v2.7.0, use `Redmine\Api\Version::listNamesByProject()` instead.', + $errstr, + ); + + restore_error_handler(); + return true; + }, + E_USER_DEPRECATED, + ); + + $api->getIdByName(5, 'Version 5'); + } + /** * Test validateSharing(). *