refactor: 🔨 update user deletion logic#35
Merged
Merged
Conversation
Change delete methods to void and throw UserNotFoundException for non-existent users.
Add assertion to verify user is null after deletion in DeleteUserActionTest.
There was a problem hiding this comment.
Pull request overview
Refactors user deletion to be exception-driven (via UserNotFoundException) rather than boolean-return based, and updates the DELETE user endpoint behavior/tests to treat non-existent users as 404s.
Changes:
- Updated
UserService::delete()andUserRepository::delete()to returnvoid, relying onUserNotFoundExceptionfor missing users. - Updated
DeleteUserActionintegration tests to assert 404 on missing users and verify successful deletions actually remove the user. - Adjusted in-memory repository delete signature to
voidand updated unit tests accordingly.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Unit/Application/Users/Services/UserServiceTest.php | Updates unit tests to expect UserNotFoundException and removes boolean assertions for deletion. |
| tests/Integration/Application/Users/Actions/DeleteUserActionTest.php | Updates integration expectations: verifies deletion removes the user and missing user deletion returns 404. |
| app/Infrastructure/Persistence/User/InMemoryUserRepository.php | Changes delete contract to void and removes “return bool” behavior. |
| app/Domain/User/UserRepository.php | Updates repository interface delete signature to void. |
| app/Application/Users/Services/UserService.php | Refactors delete flow to throw on missing user via find() before deleting. |
| app/Application/Users/Actions/DeleteUserAction.php | Updates PHPDoc/imports to reflect exception-based deletion semantics. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+7
to
9
| use App\Application\Users\Exceptions\UserNotFoundException; | ||
| use JsonException; | ||
| use Psr\Http\Message\ResponseInterface as Response; |
Comment on lines
55
to
63
| /** | ||
| * Removes the user with the given ID from the in-memory store. | ||
| * | ||
| * @param int $id The unique identifier of the user to delete. | ||
| * @return bool True if the user was found and removed, false otherwise. | ||
| * @param int $id The unique identifier of the user to delete. | ||
| */ | ||
| public function delete(int $id): bool | ||
| public function delete(int $id): void | ||
| { | ||
| if (!isset($this->store[$id])) { | ||
| return false; | ||
| } | ||
|
|
||
| unset($this->store[$id]); | ||
|
|
||
| return true; | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request refactors the user deletion logic to use exceptions for error handling instead of boolean return values, and updates the API to return a 404 status when attempting to delete a non-existent user. It also updates the related tests to reflect these changes and improves consistency across the service, repository, and action layers.
Exception-based error handling and API response changes:
UserService::delete,UserRepository::delete, andInMemoryUserRepository::deletemethods now throw aUserNotFoundExceptionif the user does not exist, and no longer return a boolean value. ([[1]](https://github.com/andrewdyer/slim-api-template/pull/35/files#diff-beb1cb379a135c4ff5e6e0c96ed596174a4d6e56e0d5344eee01b5958f83c68aL59-R65),[[2]](https://github.com/andrewdyer/slim-api-template/pull/35/files#diff-02e860bdbd2e3ffb321d5ad1db64ae85e728a46c526ce7e2fb61bb1deaabd5f3L28-R29),[[3]](https://github.com/andrewdyer/slim-api-template/pull/35/files#diff-b7e7cb95b3a539a32d908bea86a52cae88b6e09ba3ed6b85f2a8cfc6ea2694cbL59-L69))DeleteUserActionnow documents that it throwsUserNotFoundExceptionand, when a user does not exist, returns a 404 response instead of a 204. ([[1]](https://github.com/andrewdyer/slim-api-template/pull/35/files#diff-345543f6b028aa0aa07c2d1010c868d175732a1cb7976dbee7dfe88d503d66baR7),[[2]](https://github.com/andrewdyer/slim-api-template/pull/35/files#diff-345543f6b028aa0aa07c2d1010c868d175732a1cb7976dbee7dfe88d503d66baR20))Test updates:
UserNotFoundExceptionwhen deleting a non-existent user, and to confirm that users are actually deleted. ([[1]](https://github.com/andrewdyer/slim-api-template/pull/35/files#diff-9402d46bb8c19067b8da4b4537cd6397eb515d43ea78bc53053c9504c7b7dfefR24-R38),[[2]](https://github.com/andrewdyer/slim-api-template/pull/35/files#diff-35b14a7057f21075e430b4092d72a86913d29bf042af066561d51791531db3caL209-R227))