Return consistent status codes from REST API delete operations #1907
Conversation
Update PHPDoc and @noinspection tags accordingly.
The test case was defined with a data provider, but did not actually use it...
Based on the default value for $g_user_login_valid_regex - `vboctor@localhost` -> a TLD is required - `user!` -> only letters, numbers, `-.+_` and space are allowed
Update the test case to prepare for change of returned status code to 404 when trying to delete a non-existing user. Remove the assertion from RestBase::tearDown(), the check for a successful deletion should be done in the actual test cases. Issue #32858
Covering - attach a new tag - attach existing tag - detach tag - detach unattached tag - detach non-existing tag - attach/detach on non-existing issue Issue #32858
Fixes #32858
Fixes #32858
Fixes #32232, #32858
|
Note: there are currently no test cases for REST API /projects endpoint |
There was a problem hiding this comment.
Thanks @dregad
The API tests shouldn't call internal APIs like tag_exists(). The need to do so is a symptom of missing APIs and couples the API tests with the codebase. Please remove such calls and identify gaps that we need with todos or issues in bug tracker - e.g. if we are missing a Get Tags API or equivalent.
I can hear the argument about such usage being an indicator that the API has missing endpoints and needs improvement. On the other hand, I strongly disagree that the internal calls should be removed before the new endpoints are available. Doing so means reducing the tests's coverage and quality, which does not make sense. I'm OK to flag them with a TODO, but getting rid of them is not a good idea. Some of these usages, e.g. the loop to find a non-existing Id or ensuring a tag name's uniqueness, make absolutely no sense to implement with repeated REST calls; it adds no value, and is less efficient. |
|
Opened #32863 to track adding CRUD endpoints for Tags management. |
Fixes #32858 + code cleanup
Follows up on discussion in #1905