Restore BC in Membership::all()

kbsali · Dec 21, 2023 · 287cc08 · 287cc08
1 parent 8c44e5e
commit 287cc08
Show file tree

Hide file tree

Showing 3 changed files with 28 additions and 17 deletions.
diff --git a/src/Redmine/Api/Membership.php b/src/Redmine/Api/Membership.php
@@ -2,6 +2,7 @@
 
 namespace Redmine\Api;
 
+use Redmine\Exception;
 use Redmine\Exception\InvalidParameterException;
 use Redmine\Exception\MissingParameterException;
 use Redmine\Serializer\XmlSerializer;
@@ -53,13 +54,21 @@ final public function listByProject($projectIdentifier, array $params = []): arr
      * @param string|int $project project id or literal identifier
      * @param array      $params  optional parameters to be passed to the api (offset, limit, ...)
      *
-     * @return array list of memberships found
+     * @return array|string|false list of memberships found or error message or false
      */
     public function all($project, array $params = [])
     {
         @trigger_error('`'.__METHOD__.'()` is deprecated since v2.4.0, use `'.__CLASS__.'::listByProject()` instead.', E_USER_DEPRECATED);
 
-        return $this->listByProject(strval($project), $params);
+        try {
+            return $this->listByProject(strval($project), $params);
+        } catch (Exception $e) {
+            if ($this->client->getLastResponseBody() === '') {
+                return false;
+            }
+
+            return $e->getMessage();
+        }
     }
 
     /**

diff --git a/tests/Unit/Api/IssueStatusTest.php b/tests/Unit/Api/IssueStatusTest.php
@@ -53,9 +53,7 @@ public function testAllReturnsClientGetResponse($response, $responseType, $expec
         $client = $this->createMock(Client::class);
         $client->expects($this->exactly(1))
             ->method('requestGet')
-            ->with(
-                $this->stringStartsWith('/issue_statuses.json')
-            )
+            ->with('/issue_statuses.json')
             ->willReturn(true);
         $client->expects($this->atLeast(1))
             ->method('getLastResponseBody')

diff --git a/tests/Unit/Api/MembershipTest.php b/tests/Unit/Api/MembershipTest.php
@@ -46,34 +46,38 @@ function ($errno, $errstr): bool {
      * Test all().
      *
      * @covers ::all
+     * @dataProvider getAllData
      * @test
      */
-    public function testAllReturnsClientGetResponseWithProject()
+    public function testAllReturnsClientGetResponseWithProject($response, $responseType, $expectedResponse)
     {
-        // Test values
-        $response = '["API Response"]';
-        $expectedReturn = ['API Response'];
-
         // Create the used mock objects
         $client = $this->createMock(Client::class);
-        $client->expects($this->once())
+        $client->expects($this->exactly(1))
             ->method('requestGet')
-            ->with(
-                $this->stringStartsWith('/projects/5/memberships.json')
-            )
+            ->with('/projects/5/memberships.json')
             ->willReturn(true);
-        $client->expects($this->exactly(1))
+        $client->expects($this->atLeast(1))
             ->method('getLastResponseBody')
             ->willReturn($response);
         $client->expects($this->exactly(1))
             ->method('getLastResponseContentType')
-            ->willReturn('application/json');
+            ->willReturn($responseType);
 
         // Create the object under test
         $api = new Membership($client);
 
         // Perform the tests
-        $this->assertSame($expectedReturn, $api->all(5));
+        $this->assertSame($expectedResponse, $api->all(5));
+    }
+
+    public static function getAllData(): array
+    {
+        return [
+            'array response' => ['["API Response"]', 'application/json', ['API Response']],
+            'string response' => ['"string"', 'application/json', 'Could not convert response body into array: "string"'],
+            'false response' => ['', 'application/json', false],
+        ];
     }
 
     /**