From 444192172d19ccc51f14009b537e16f5da766c3c Mon Sep 17 00:00:00 2001
From: Art4 <art4@wlabs.de>
Date: Tue, 9 Jul 2024 13:25:43 +0200
Subject: [PATCH] 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