[#40969] show correct error message in files tab

https://community.openproject.org/work_packages/40969

Signed-off-by: Artur Neumann <artur@jankaritech.com>

lib/Controller/OpenProjectAPIController.php

@@ -128,20 +128,34 @@ public function getNotifications(?string $since = null): DataResponse {
 	 * @NoAdminRequired
 	 *
 	 * @param ?string $searchQuery
+	 * @param ?int $fileId
 	 *
 	 * @return DataResponse
 	 */
-	public function getSearchedWorkPackages(?string $searchQuery = null): DataResponse {
+	public function getSearchedWorkPackages(?string $searchQuery = null, ?int $fileId = null): DataResponse {
 		if ($this->accessToken === '' || !OpenProjectAPIService::validateOpenProjectURL($this->openprojectUrl)) {
-			return new DataResponse('', 400);
+			return new DataResponse('invalid open project configuration', 401);
 		}
 		$result = $this->openprojectAPIService->searchWorkPackage(
-			$this->openprojectUrl, $this->accessToken, $this->refreshToken, $this->clientID, $this->clientSecret, $this->userId, $searchQuery
+			$this->openprojectUrl,
+			$this->accessToken,
+			$this->refreshToken,
+			$this->clientID,
+			$this->clientSecret,
+			$this->userId,
+			$searchQuery,
+			$fileId
 		);
+
 		if (!isset($result['error'])) {
 			$response = new DataResponse($result);
 		} else {
-			$response = new DataResponse($result, 401);
+			if (isset($result['statusCode'])) {
+				$statusCode = $result['statusCode'];
+			} else {
+				$statusCode = 400;
+			}
+			$response = new DataResponse($result, $statusCode);
 		}
 		return $response;
 	}

lib/Service/OpenProjectAPIService.php

@@ -241,19 +241,23 @@ public function getNotifications(string $url, string $accessToken,
 	 * @param string $query
 	 * @param int $offset
 	 * @param int $limit
-	 * @return array<string>
+	 * @return array<mixed>
 	 * @throws \Safe\Exceptions\JsonException
 	 */
 	public function searchWorkPackage(string $url, string $accessToken,
 							string $refreshToken, string $clientID, string $clientSecret, string $userId,
-							string $query, int $offset = 0, int $limit = 5): array {
+							string $query = null, int $fileId = null, int $offset = 0, int $limit = 5): array {
 		$resultsById = [];
+		$filters = [];
 
 		// search by description
-		$filters = [
-			["description" => ["operator" => "~", "values" => [$query]]],
-			["status" => ["operator" => "!", "values" => ["14"]]]
-		];
+		if ($fileId !== null) {
+			$filters[] = ["file_link_origin_id" => ["operator" => "=", "values" => [(string)$fileId]]];
+		}
+		if ($query !== null) {
+			$filters[] = ["description" => ["operator" => "~", "values" => [$query]]];
+			$filters[] = ["status" => ["operator" => "!", "values" => ["14"]]];
+		}
 		$params = [
 			'filters' => \Safe\json_encode($filters),
 			'sortBy' => '[["updatedAt", "desc"]]',
@@ -263,28 +267,38 @@ public function searchWorkPackage(string $url, string $accessToken,
 			$url, $accessToken, $refreshToken, $clientID, $clientSecret, $userId, 'work_packages', $params
 		);
 
+		if (isset($searchDescResult['error'])) {
+			return $searchDescResult;
+		}
+
 		if (isset($searchDescResult['_embedded'], $searchDescResult['_embedded']['elements'])) {
 			foreach ($searchDescResult['_embedded']['elements'] as $wp) {
 				$resultsById[$wp['id']] = $wp;
 			}
 		}
 		// search by subject
-		$filters = [
-			["subject" => ["operator" => "~", "values" => [$query]]],
-			["status" => ["operator" => "!", "values" => ["14"]]]
-		];
-		$params = [
-			'filters' => \Safe\json_encode($filters),
-			'sortBy' => '[["updatedAt", "desc"]]',
-			// 'limit' => $limit,
-		];
-		$searchSubjectResult = $this->request(
+		if ($query !== null) {
+			$filters = [
+				["subject" => ["operator" => "~", "values" => [$query]]],
+				["status" => ["operator" => "!", "values" => ["14"]]]
+			];
+			$params = [
+				'filters' => \Safe\json_encode($filters),
+				'sortBy' => '[["updatedAt", "desc"]]',
+				// 'limit' => $limit,
+			];
+			$searchSubjectResult = $this->request(
 			$url, $accessToken, $refreshToken, $clientID, $clientSecret, $userId, 'work_packages', $params
 		);
 
-		if (isset($searchSubjectResult['_embedded'], $searchSubjectResult['_embedded']['elements'])) {
-			foreach ($searchSubjectResult['_embedded']['elements'] as $wp) {
-				$resultsById[$wp['id']] = $wp;
+			if (isset($searchSubjectResult['error'])) {
+				return $searchSubjectResult;
+			}
+
+			if (isset($searchSubjectResult['_embedded'], $searchSubjectResult['_embedded']['elements'])) {
+				foreach ($searchSubjectResult['_embedded']['elements'] as $wp) {
+					$resultsById[$wp['id']] = $wp;
+				}
 			}
 		}
 

src/components/tab/EmptyContent.vue

@@ -7,6 +7,9 @@
 			<div class="title text-center">
 				{{ emptyContentMessage }}
 			</div>
+			<div class="error text-center">
+				{{ errorMessage }}
+			</div>
 			<br>
 			<div v-if="showConnectButton" class="connect-button">
 				<OAuthConnectButton :request-url="requestUrl" />
@@ -33,6 +36,10 @@ export default {
 			type: String,
 			required: true,
 		},
+		errorMessage: {
+			type: String,
+			default: '',
+		},
 	},
 	data() {
 		return {

src/views/ProjectsTab.vue

@@ -28,6 +28,7 @@
 			id="openproject-empty-content"
 			:state="state"
 			:request-url="requestUrl" />
+			:error-message="error" />
 	</div>
 </template>
 
@@ -77,7 +78,7 @@ export default {
 
 		async fetchWorkpackages(fileId) {
 			const req = {}
-			const url = generateUrl('/apps/integration_openproject/workpackages/' + fileId)
+			const url = generateUrl('/apps/integration_openproject/work_packages?fileId=' + fileId)
 			try {
 				const response = await axios.get(url, req)
 				if (!Array.isArray(response.data)) {
@@ -91,6 +92,13 @@ export default {
 				} else {
 					this.state = 'error'
 				}
+				if (error.response.data !== undefined && this.state === 'error') {
+					if (error.response.data.error !== undefined) {
+						this.error = error.response.data.error
+					} else {
+						this.error = error.response.data
+					}
+				}
 			}
 		},
 	},

tests/jest/components/tab/EmptyContent.spec.js

@@ -3,9 +3,19 @@ import { shallowMount, createLocalVue } from '@vue/test-utils'
 import EmptyContent from '../../../../src/components/tab/EmptyContent'
 const localVue = createLocalVue()
 
+function getWrapper(propsData = {}) {
+	return shallowMount(EmptyContent, {
+		localVue,
+		mocks: {
+			t: (msg) => msg,
+		},
+		propsData,
+	})
+}
 describe('EmptyContent.vue Test', () => {
 	let wrapper
 	const emptyContentMessageSelector = '.title'
+	const errorMessageSelector = '.error'
 	const connectButtonSelector = 'oauthconnectbutton-stub'
 
 	it.each([{
@@ -28,6 +38,7 @@ describe('EmptyContent.vue Test', () => {
 				requestUrl: 'http://openproject/oauth/',
 			},
 		})
+		wrapper = getWrapper({ state: cases.state })
 		expect(wrapper.find(connectButtonSelector).exists()).toBe(cases.viewed)
 	})
 	it.each([{
@@ -53,7 +64,18 @@ describe('EmptyContent.vue Test', () => {
 				requestUrl: 'http://openproject/oauth/',
 			},
 		})
+		wrapper = getWrapper({ state: cases.state })
 		expect(wrapper.find(emptyContentMessageSelector).exists()).toBeTruthy()
 		expect(wrapper.find(emptyContentMessageSelector).text()).toMatch(cases.message)
 	})
+	it('has an empty error by default', () => {
+		wrapper = getWrapper()
+		expect(wrapper.find(errorMessageSelector).exists()).toBeTruthy()
+		expect(wrapper.find(errorMessageSelector).text()).toMatch('')
+	})
+	it('shows the error', () => {
+		wrapper = getWrapper({ errorMessage: 'something went wrong' })
+		expect(wrapper.find(errorMessageSelector).exists()).toBeTruthy()
+		expect(wrapper.find(errorMessageSelector).text()).toMatch('something went wrong')
+	})
 })

tests/jest/views/ProjectsTab.spec.js

@@ -70,5 +70,17 @@ describe('ProjectsTab.vue Test', () => {
 			await wrapper.vm.update({ id: 123 })
 			expect(wrapper.vm.state).toBe(cases.AppState)
 		})
+		it.each([
+			{ HTTPStatus: 404, data: undefined, errorMessage: '' },
+			{ HTTPStatus: 404, data: 'some error', errorMessage: 'some error' },
+			{ HTTPStatus: 404, data: { error: 'some error' }, errorMessage: 'some error' },
+			{ HTTPStatus: 401, data: 'token error', errorMessage: '' },
+		])('sets the error message according to the response', async (cases) => {
+			const err = new Error()
+			err.response = { status: cases.HTTPStatus, data: cases.data }
+			axios.get.mockRejectedValueOnce(err)
+			await wrapper.vm.update({ id: 123 })
+			expect(wrapper.vm.error).toBe(cases.errorMessage)
+		})
 	})
 })

tests/lib/Controller/OpenProjectAPIControllerTest.php

@@ -175,43 +175,69 @@ public function testGetOpenProjectAvatarNoType() {
 	}
 
 	/**
+	 * @return array<mixed>
+	 */
+	public function searchWorkPackagesDataProvider() {
+		return [
+			['test', null, [['id' => 1], ['id' => 2], ['id' => 3], ['id' => 4], ['id' => 5]]],
+			['test', 9090, [['id' => 1], ['id' => 2], ['id' => 3], ['id' => 4], ['id' => 5]]],
+			[null, 9090, [['id' => 1], ['id' => 2], ['id' => 3], ['id' => 4], ['id' => 5]]]
+		];
+	}
+
+	/**
+	 * @param string|null $searchQuery
+	 * @param int|null $fileId
+	 * @param array<mixed> $expectedResponse
 	 * @return void
+	 * @dataProvider searchWorkPackagesDataProvider
 	 */
-	public function testGetSearchedWorkPackages():void {
+	public function testGetSearchedWorkPackages($searchQuery, $fileId, array $expectedResponse):void {
 		$this->getUserValueMock();
 		$service = $this->getMockBuilder(OpenProjectAPIService::class)
 			->disableOriginalConstructor()
 			->onlyMethods(['searchWorkPackage'])
 			->getMock();
 		$service->expects($this->once())
 			->method('searchWorkPackage')
-			->willReturn([['id' => 1], ['id' => 2], ['id' => 3], ['id' => 4], ['id' => 5]]);
+			->with(
+				$this->anything(),
+				$this->anything(),
+				$this->anything(),
+				$this->anything(),
+				$this->anything(),
+				$this->anything(),
+				$searchQuery,
+				$fileId
+			)
+			->willReturn($expectedResponse);
 
 		$controller = new OpenProjectAPIController(
 			'integration_openproject', $this->requestMock, $this->configMock, $service, 'test'
 		);
-		$response = $controller->getSearchedWorkPackages('test');
+		$response = $controller->getSearchedWorkPackages($searchQuery, $fileId);
 		$this->assertSame(200, $response->getStatus());
-		$this->assertSame([['id' => 1], ['id' => 2], ['id' => 3], ['id' => 4], ['id' => 5]], $response->getData());
+		$this->assertSame($expectedResponse, $response->getData());
 	}
 
+
 	/**
 	 * @return void
 	 */
-	public function testGetSearchedWorkPackagesErrorResponse(): void {
+	public function testGetSearchedWorkPackagesNoAccessToken(): void {
 		$this->getUserValueMock('');
 		$service = $this->createMock(OpenProjectAPIService::class);
 		$controller = new OpenProjectAPIController(
 			'integration_openproject', $this->requestMock, $this->configMock, $service, 'test'
 		);
 		$response = $controller->getSearchedWorkPackages('test');
-		$this->assertSame(400, $response->getStatus());
+		$this->assertSame(401, $response->getStatus());
 	}
 
 	/**
 	 * @return void
 	 */
-	public function testGetSearchedWorkPackagesNoAccessToken(): void {
+	public function testGetSearchedWorkPackagesErrorResponse(): void {
 		$this->getUserValueMock();
 		$service = $this->getMockBuilder(OpenProjectAPIService::class)
 			->disableOriginalConstructor()
@@ -224,7 +250,7 @@ public function testGetSearchedWorkPackagesNoAccessToken(): void {
 			'integration_openproject', $this->requestMock, $this->configMock, $service, 'test'
 		);
 		$response = $controller->getSearchedWorkPackages('test');
-		$this->assertSame(401, $response->getStatus());
+		$this->assertSame(400, $response->getStatus());
 		$this->assertSame(['error' => 'something went wrong'], $response->getData());
 	}