diff --git a/app/Http/Controllers/CustomFilterController.php b/app/Http/Controllers/CustomFilterController.php index e42344638..39d8e6d96 100644 --- a/app/Http/Controllers/CustomFilterController.php +++ b/app/Http/Controllers/CustomFilterController.php @@ -145,6 +145,8 @@ class CustomFilterController extends Controller ->toArray(); if (! empty($existingKeywords)) { + DB::rollBack(); + return response()->json([ 'error' => 'Duplicate keywords found', 'message' => 'The following keywords already exist: '.implode(', ', $existingKeywords), @@ -314,6 +316,8 @@ class CustomFilterController extends Controller $keywordIds = collect($validatedData['keywords_attributes'])->pluck('id')->filter()->toArray(); if (count($keywordIds) && ! CustomFilterKeyword::whereCustomFilterId($filter->id)->whereIn('id', $keywordIds)->count()) { + DB::rollBack(); + return response()->json([ 'error' => 'Record not found', ], 404); @@ -336,6 +340,8 @@ class CustomFilterController extends Controller ->toArray(); if (! empty($existingKeywords)) { + DB::rollBack(); + return response()->json([ 'error' => 'Duplicate keywords found', 'message' => 'The following keywords already exist: '.implode(', ', $existingKeywords), @@ -396,6 +402,8 @@ class CustomFilterController extends Controller ->first(); if (! isset($keywordData['_destroy']) && $filter->keywords()->pluck('id')->search($keywordData['id']) === false) { + DB::rollBack(); + return response()->json([ 'error' => 'Duplicate keywords found', 'message' => 'The following keywords already exist: '.$keywordData['keyword'], @@ -427,6 +435,8 @@ class CustomFilterController extends Controller $maxKeywordsPerFilter = CustomFilter::getMaxKeywordsPerFilter(); if ($existingKeywordCount >= $maxKeywordsPerFilter) { + DB::rollBack(); + return response()->json([ 'error' => 'Keyword limit exceeded', 'message' => 'A filter can have a maximum of '.$maxKeywordsPerFilter.' keywords.', diff --git a/tests/Feature/Api/CustomFilterTransactionTest.php b/tests/Feature/Api/CustomFilterTransactionTest.php new file mode 100644 index 000000000..0b9db3351 --- /dev/null +++ b/tests/Feature/Api/CustomFilterTransactionTest.php @@ -0,0 +1,100 @@ +setAccessible(true); + $ref->setValue(null, null); + } +} + +beforeEach(function () { + Cache::flush(); + config(['instance.custom_filters.max_create_per_hour' => 100]); + config(['instance.custom_filters.max_updates_per_hour' => 100]); + config(['instance.custom_filters.max_filters_per_user' => 100]); + config(['instance.custom_filters.max_keywords_per_filter' => 100]); + resetCustomFilterTransactionTestLimitStatics(); +}); + +it('does not leave an open transaction when store() rejects a duplicate keyword', function () { + $user = User::factory()->create(); + $user->refresh(); + Passport::actingAs($user, ['read', 'write']); + + $filter = CustomFilter::create([ + 'title' => 'existing', + 'context' => ['home'], + 'action' => CustomFilter::ACTION_WARN, + 'profile_id' => $user->profile_id, + ]); + $filter->keywords()->create(['keyword' => 'alpha', 'whole_word' => true]); + + $levelBefore = DB::transactionLevel(); + + $this->postJson('/api/v2/filters', [ + 'title' => 'new filter', + 'context' => ['home'], + 'filter_action' => 'warn', + 'keywords_attributes' => [ + ['keyword' => 'alpha', 'whole_word' => true], + ], + ])->assertStatus(422)->assertJson(['error' => 'Duplicate keywords found']); + + expect(DB::transactionLevel())->toBe($levelBefore); +}); + +it('does not leave an open transaction when update() rejects a duplicate keyword', function () { + $user = User::factory()->create(); + $user->refresh(); + Passport::actingAs($user, ['read', 'write']); + + $other = CustomFilter::create([ + 'title' => 'other', + 'context' => ['home'], + 'action' => CustomFilter::ACTION_WARN, + 'profile_id' => $user->profile_id, + ]); + $other->keywords()->create(['keyword' => 'apple', 'whole_word' => true]); + + $target = CustomFilter::create([ + 'title' => 'target', + 'context' => ['home'], + 'action' => CustomFilter::ACTION_WARN, + 'profile_id' => $user->profile_id, + ]); + $target->keywords()->create(['keyword' => 'banana', 'whole_word' => true]); + + $levelBefore = DB::transactionLevel(); + + $this->putJson('/api/v2/filters/'.$target->id, [ + 'keywords_attributes' => [ + ['keyword' => 'apple', 'whole_word' => true], + ], + ])->assertStatus(422)->assertJson(['error' => 'Duplicate keywords found']); + + expect(DB::transactionLevel())->toBe($levelBefore); +});