Skip to content

Commit 17bd8a4

Browse files
authored
[6.0] Fix DELETE endpoints to return 404 for non-existent resources (#46161)
* Verb delete return 404 for not existent * cs * contact test too * wrong endpoint
1 parent 3d8f63e commit 17bd8a4

File tree

4 files changed

+43
-10
lines changed

4 files changed

+43
-10
lines changed

libraries/src/MVC/Controller/ApiController.php

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -319,11 +319,24 @@ public function delete($id = null)
319319
];
320320
echo new JsonResponse($body);
321321
$this->app->close();
322-
} else {
323-
throw new \RuntimeException(Text::_('JLIB_APPLICATION_ERROR_DELETE'), 500);
324322
}
325323
}
326324

325+
// If the model has set a 404 status code in the session, we return a 404 http status codee
326+
if ($this->app->getSession()->get('http_status_code_404', false)) {
327+
$this->app->getSession()->clear('http_status_code_404');
328+
$this->app->setHeader('status', 404, true);
329+
$this->app->setHeader('Content-Type', 'application/json');
330+
$this->app->sendHeaders();
331+
$body = [
332+
'status' => 'not found',
333+
'code' => 404,
334+
'message' => 'Resource not found',
335+
];
336+
echo new JsonResponse($body);
337+
$this->app->close();
338+
}
339+
327340
$this->app->setHeader('status', 204);
328341
}
329342

libraries/src/MVC/Model/AdminModel.php

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -976,11 +976,9 @@ public function delete(&$pks)
976976
}
977977
if (Factory::getApplication()->isClient('api')) {
978978
$session = Factory::getApplication()->getSession();
979-
$session->set('http_status_code_409', true);
979+
$session->set('http_status_code_404', true);
980980
}
981981

982-
Log::add(Text::_('JLIB_APPLICATION_ERROR_DELETE_NOT_PERMITTED'), Log::WARNING, 'jerror');
983-
984982
return true;
985983
}
986984
}

tests/System/integration/api/com_banners/Banners.cy.js

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -49,9 +49,20 @@ describe('Test that banners API endpoint', () => {
4949
.then((banner) => cy.api_delete(`/banners/${banner.id}`));
5050
});
5151

52-
it('check correct response for delete a not existent contact', () => {
53-
cy.api_delete('/banners/9999')
54-
.then((result) => expect(result.status).to.eq(204));
52+
it('check correct response for delete a not existent banner', () => {
53+
cy.api_getBearerToken().then((token) => {
54+
cy.request({
55+
method: 'DELETE',
56+
url: `/api/index.php/v1/banners/9999`,
57+
headers: {
58+
Authorization: `Bearer ${token}`,
59+
},
60+
failOnStatusCode: false
61+
}).then((response) => {
62+
expect(response.status).to.equal(404);
63+
expect(response.body.data.message).to.include('Resource not found');
64+
});
65+
});
5566
});
5667

5768
it('cannot delete a banner that is not trashed', () => {

tests/System/integration/api/com_contact/Contacts.cy.js

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,8 +47,19 @@ describe('Test that contacts API endpoint', () => {
4747
});
4848

4949
it('check correct response for delete a not existent contact', () => {
50-
cy.api_delete('/contacts/9999')
51-
.then((result) => expect(result.status).to.eq(204));
50+
cy.api_getBearerToken().then((token) => {
51+
cy.request({
52+
method: 'DELETE',
53+
url: `/api/index.php/v1/contacts/9999`,
54+
headers: {
55+
Authorization: `Bearer ${token}`,
56+
},
57+
failOnStatusCode: false
58+
}).then((response) => {
59+
expect(response.status).to.equal(404);
60+
expect(response.body.data.message).to.include('Resource not found');
61+
});
62+
});
5263
});
5364

5465
it('can submit a contact form', () => {

0 commit comments

Comments
 (0)