Exception when version to delete does not exist

[dregad]

The initial implementation of VersionDeleteCommand (see #1883) did not throw an exception when given a non-existing version number, causing the REST API to succeed with HTTP 204 (version deleted) when it should in fact have failed.

The Command now throws a ClientException, resulting in the API returning a 404 (not found) in this situation.

Note: the version_exists() call before retrieving the project id with version_get_field() is not necessary, as the latter already perform this check (via version_cache_row()).

PHPUnit test has been updated accordingly.

Fixes #30415

[vboctor]

We can go either way:

• Returning 404 more strictly reflects the state and what work was done.
• Returning success reflects that desired outcome was achieved. This is more friendly to retries when a response fails to complete to the client. More idempotent. That is the path I have chosen at time of implementation.

We should probably aim to be consistent across deletes - e.g. deleting a version, project, issue, category, etc.

[dregad]

Thanks for the feedback. I get your point now - a bit strange to me, but I guess it makes sense.

I'll have a look at other delete endpoints to make sure we remain consistent

[dregad]

I tested the REST API DELETE endpoints, and we are not very consistent

Operation	Endpoint	Status code
Issue delete	DELETE /issues/999999	404
Issue Tag delete	DELETE /issues/1/tags/999999	200
Issue note	DELETE /issues/1/notes/999999	404 (400 if existing note of other issue)
Issue relationship	DELETE /issues/1/relationships/99999	200 ⚠️ This is a bug 404 with fix in #1906
Project delete	DELETE /projects/99999	403 (Access denied for deleting project.) NOTE: does not use ProjectDeleteCommand !
Project user delete	DELETE /projects/21/users/99999	404 (User 99999 not found)
Project version delete	DELETE /projects/21/versions/99999	204
User delete	DELETE /users/99999	204
User token delete	DELETE /users/1/token/99999	404 (Token doesn't exist)
User token delete own	DELETE /users/me/token/99999	404 (Token doesn't exist)

[vboctor]

Thanks for doing the analysis. Looks like 404 when deleting a non-existent entity wins.

[dregad]

OK, I'll merge this PR then.

#32858 has been opened to track the update of the remaining endpoints.