fix: support partial updates in PATCH /api/v1/users/{id} / ユーザー更新APIの部分更新対応 - #547
Merged
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## feat/user #547 +/- ##
===============================================
+ Coverage 62.51% 62.72% +0.21%
- Complexity 1615 1619 +4
===============================================
Files 136 136
Lines 8329 8374 +45
===============================================
+ Hits 5207 5253 +46
+ Misses 3122 3121 -1
🚀 New features to boost your workflow:
|
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.
Motivation / 目的
PATCH /api/v1/users/{id}に一部のフィールドだけを送ると、UpdateUserCommand::fromArrayがすべての必須キー(first_name,last_name,email,age_range,subscription_tier)を要求していたためInvalidArgumentExceptionが発生していた。この例外はUserController::updateUserの汎用catchに落ちて500 Internal Server Errorとして返っていた。Sending only a subset of fields to
PATCH /api/v1/users/{id}causedUpdateUserCommand::fromArrayto throwInvalidArgumentException, since it required every field (first_name,last_name,email,age_range,subscription_tier) to be present. That exception fell throughUserController::updateUser's generic catch block and surfaced as a500 Internal Server Error.PATCHの意味的にも部分更新を許可すべきという判断のもと、更新対象フィールドのみ送れば良い設計に変更した。
また、修正の過程で別の問題も見つかった。
UserController::updateUserはcatch (\Exception)のみで、\Error系(例: 無効なage_rangeを渡した際にAgeRange::from()が投げる\ValueError)を一切キャッチできず、生のLaravelデバッグエラーページがそのまま返っていた(createUserは既に\Throwableをキャッチしており一貫性がなかった)。これも合わせて修正した。While fixing this, we also found that
UserController::updateUseronly caught\Exception, so\Error-family exceptions (e.g. the\ValueErrorthrown byAgeRange::from()on an invalidage_rangevalue) were never caught, leaking Laravel's raw debug error page instead of a clean JSON response (createUseralready caught\Throwable, so this was an inconsistency). Fixed that too.What I have done / 実施内容
UpdateUserCommand:id以外の全フィールドを任意(nullable)にし、fromArrayはidの存在のみを必須チェックするように変更UpdateUserUseCase:handle()の冒頭で既存ユーザーをfindByIdで取得し、コマンドで指定されなかったフィールドは既存値をそのまま使う(未指定=変更なし)ようにマージするロジックを追加。ユーザーが存在しない場合は従来通りUser not found.を投げるUserController::updateUserのcatch (\Exception)をcatch (\Throwable)に変更し、\ValueError等でも正しくJSONのInternal Server Errorを返すように修正UpdateUserCommandTest/UpdateUserUseCaseTestを部分更新の挙動に合わせて更新UpdateUserTestのtest_update_user_returns_500_when_required_key_is_missing(今回の設計変更で前提が崩れた既存テスト)を削除し、代わりに無効なage_rangeを送った際にクリーンなJSON 500が返ることを確認するテストに置き換えTest Results / テスト結果
UpdateUserCommandTest::test_fromArray_creates_command_with_valid_dataUpdateUserCommandTest::test_fromArray_sets_subscription_expires_at_when_providedUpdateUserCommandTest::test_fromArray_throws_when_id_is_missingUpdateUserCommandTest::test_fromArray_allows_optional_key_to_be_omitted(first_name/last_name/email/age_range/subscription_tier)UpdateUserUseCaseTest::test_handle_passes_entity_to_repository_and_returns_dtoUpdateUserUseCaseTest::test_handle_keeps_existing_value_when_field_is_omittedUpdateUserUseCaseTest::test_handle_throws_when_user_not_foundUpdateUserTest::test_update_user_returns_200_with_updated_dataUpdateUserTest::test_update_user_returns_404_when_user_not_foundUpdateUserTest::test_update_user_returns_500_with_json_when_age_range_is_invalidPATCH /api/v1/users/1にfirst_nameのみ送信し、他フィールドが保持されたまま更新されることを確認LoginTest/LogoutTest/MeTestの回帰なしを確認済み