From 557d2758d2950d78269a043b49f39d5e1fb57550 Mon Sep 17 00:00:00 2001 From: thanhson Date: Wed, 21 Feb 2024 15:05:01 +0700 Subject: [PATCH 1/3] fix AggregateFilterable --- src/Rules/AggregateFilterable.php | 18 +++++++++++------- src/Rules/IsNestedField.php | 5 +++++ 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/src/Rules/AggregateFilterable.php b/src/Rules/AggregateFilterable.php index 4e23412..4b3ee5c 100644 --- a/src/Rules/AggregateFilterable.php +++ b/src/Rules/AggregateFilterable.php @@ -63,15 +63,19 @@ public function resource($resource) */ protected function buildValidationRules($attribute, $value) { - if (is_null($this->resource)) { - return []; + $rules = []; + + foreach ($this->data['search']['aggregates'] as $aggregate) { + $resource = $this->resource->relation($aggregate['relation'])?->resource(); + + if (is_null($this->resource)) { + continue; + } + + array_push($rules, (new SearchRules($resource, app()->make(RestRequest::class), false))->filtersRules($resource, $attribute)); } - return (new SearchRules($this->resource, app()->make(RestRequest::class), false)) - ->filtersRules( - $this->resource, - $attribute - ); + return $rules; } /** diff --git a/src/Rules/IsNestedField.php b/src/Rules/IsNestedField.php index 72b6442..b48eb06 100644 --- a/src/Rules/IsNestedField.php +++ b/src/Rules/IsNestedField.php @@ -43,4 +43,9 @@ public function validate(string $attribute, mixed $value, Closure $fail): void $fail('The '.$attribute.' field is not valid.'); } } + + public function __toString() + { + return 'nested_field'; + } } From c40158dc027e8216b0fc5845aa5223b09b403a4c Mon Sep 17 00:00:00 2001 From: Gautier Deleglise Date: Thu, 22 Feb 2024 18:59:55 +0100 Subject: [PATCH 2/3] :recycle: get the right resource on the build aggregate filter rules --- src/Rules/AggregateFilterable.php | 18 ++++--- src/Rules/SearchRules.php | 4 +- .../SearchAggregatesOperationsTest.php | 48 +++++++++++++++++++ .../BelongsToManyRelationFactory.php | 1 + ...create_belongs_to_many_relations_table.php | 1 + .../Rest/Resources/BelongsToManyResource.php | 1 + 6 files changed, 60 insertions(+), 13 deletions(-) diff --git a/src/Rules/AggregateFilterable.php b/src/Rules/AggregateFilterable.php index 4b3ee5c..2bae83f 100644 --- a/src/Rules/AggregateFilterable.php +++ b/src/Rules/AggregateFilterable.php @@ -63,19 +63,17 @@ public function resource($resource) */ protected function buildValidationRules($attribute, $value) { - $rules = []; + $aggregateResource = $this->resource->relation($value['relation'])?->resource(); - foreach ($this->data['search']['aggregates'] as $aggregate) { - $resource = $this->resource->relation($aggregate['relation'])?->resource(); - - if (is_null($this->resource)) { - continue; - } - - array_push($rules, (new SearchRules($resource, app()->make(RestRequest::class), false))->filtersRules($resource, $attribute)); + if (is_null($aggregateResource)) { + return []; } - return $rules; + return (new SearchRules($this->resource, app()->make(RestRequest::class), false)) + ->filtersRules( + $aggregateResource, + $attribute.'.filters' + ); } /** diff --git a/src/Rules/SearchRules.php b/src/Rules/SearchRules.php index ee1e8d0..7707ac1 100644 --- a/src/Rules/SearchRules.php +++ b/src/Rules/SearchRules.php @@ -297,11 +297,9 @@ protected function aggregatesRules(\Lomkit\Rest\Http\Resource $resource, string $prefix.'.*' => [ AggregateField::make() ->resource($resource), - ], - $prefix.'.*.filters' => [ AggregateFilterable::make() ->resource($resource), - ], + ] ]; } } diff --git a/tests/Feature/Controllers/SearchAggregatesOperationsTest.php b/tests/Feature/Controllers/SearchAggregatesOperationsTest.php index d5c8550..11fc72b 100644 --- a/tests/Feature/Controllers/SearchAggregatesOperationsTest.php +++ b/tests/Feature/Controllers/SearchAggregatesOperationsTest.php @@ -603,6 +603,54 @@ public function test_getting_a_list_of_resources_aggregating_by_sum_number_with_ ); } + public function test_getting_a_list_of_resources_aggregating_by_sum_number_with_other_number_filter(): void + { + $matchingModel = ModelFactory::new() + ->has( + BelongsToManyRelationFactory::new() + ->count(20) + ) + ->create()->fresh(); + $matchingModel2 = ModelFactory::new() + ->has( + BelongsToManyRelationFactory::new() + ->count(20) + ) + ->create()->fresh(); + + Gate::policy(Model::class, GreenPolicy::class); + Gate::policy(BelongsToManyRelation::class, GreenPolicy::class); + + $response = $this->post( + '/api/models/search', + [ + 'search' => [ + 'aggregates' => [ + [ + 'relation' => 'belongsToManyRelation', + 'type' => 'sum', + 'field' => 'number', + 'filters' => [ + ['field' => 'other_number', 'operator' => '<', 'value' => 200], + ], + ], + ], + ], + ], + ['Accept' => 'application/json'] + ); + + $this->assertResourcePaginated( + $response, + [$matchingModel, $matchingModel2], + new ModelResource(), + [ + ['belongs_to_many_relation_sum_number' => $matchingModel->belongsToManyRelation()->orderBy('belongs_to_many_relations.number', 'desc')->where('belongs_to_many_relations.other_number', '<', 200)->sum('belongs_to_many_relations.number')], + ['belongs_to_many_relation_sum_number' => $matchingModel2->belongsToManyRelation()->orderBy('belongs_to_many_relations.number', 'desc')->where('belongs_to_many_relations.other_number', '<', 200)->sum('belongs_to_many_relations.number')], + ] + ); + } + public function test_getting_a_list_of_resources_aggregating_by_count_with_filters(): void { $matchingModel = ModelFactory::new() diff --git a/tests/Support/Database/Factories/BelongsToManyRelationFactory.php b/tests/Support/Database/Factories/BelongsToManyRelationFactory.php index b644093..3d8d357 100644 --- a/tests/Support/Database/Factories/BelongsToManyRelationFactory.php +++ b/tests/Support/Database/Factories/BelongsToManyRelationFactory.php @@ -23,6 +23,7 @@ public function definition() { return [ 'number' => fake()->numberBetween(-5000, 5000), + 'other_number' => fake()->numberBetween(-5000, 5000), ]; } } diff --git a/tests/Support/Database/migrations/2023_02_00_000000_create_belongs_to_many_relations_table.php b/tests/Support/Database/migrations/2023_02_00_000000_create_belongs_to_many_relations_table.php index 6ccc0d8..8e4f97a 100644 --- a/tests/Support/Database/migrations/2023_02_00_000000_create_belongs_to_many_relations_table.php +++ b/tests/Support/Database/migrations/2023_02_00_000000_create_belongs_to_many_relations_table.php @@ -15,6 +15,7 @@ public function up() Schema::create('belongs_to_many_relations', function (Blueprint $table) { $table->id(); $table->integer('number')->default(0); + $table->integer('other_number')->default(0); $table->timestamps(); }); } diff --git a/tests/Support/Rest/Resources/BelongsToManyResource.php b/tests/Support/Rest/Resources/BelongsToManyResource.php index 5948318..058a31f 100644 --- a/tests/Support/Rest/Resources/BelongsToManyResource.php +++ b/tests/Support/Rest/Resources/BelongsToManyResource.php @@ -26,6 +26,7 @@ public function fields(RestRequest $request): array return [ 'id', 'number', + 'other_number' ]; } } From b2142b0901b635b2e8cf2d7b23fa91ad59292477 Mon Sep 17 00:00:00 2001 From: Gautier Deleglise Date: Thu, 22 Feb 2024 19:01:30 +0100 Subject: [PATCH 3/3] :white_check_mark: style CI --- src/Rules/SearchRules.php | 2 +- .../Support/Database/Factories/BelongsToManyRelationFactory.php | 2 +- tests/Support/Rest/Resources/BelongsToManyResource.php | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Rules/SearchRules.php b/src/Rules/SearchRules.php index 7707ac1..be75077 100644 --- a/src/Rules/SearchRules.php +++ b/src/Rules/SearchRules.php @@ -299,7 +299,7 @@ protected function aggregatesRules(\Lomkit\Rest\Http\Resource $resource, string ->resource($resource), AggregateFilterable::make() ->resource($resource), - ] + ], ]; } } diff --git a/tests/Support/Database/Factories/BelongsToManyRelationFactory.php b/tests/Support/Database/Factories/BelongsToManyRelationFactory.php index 3d8d357..8606bcc 100644 --- a/tests/Support/Database/Factories/BelongsToManyRelationFactory.php +++ b/tests/Support/Database/Factories/BelongsToManyRelationFactory.php @@ -22,7 +22,7 @@ class BelongsToManyRelationFactory extends Factory public function definition() { return [ - 'number' => fake()->numberBetween(-5000, 5000), + 'number' => fake()->numberBetween(-5000, 5000), 'other_number' => fake()->numberBetween(-5000, 5000), ]; } diff --git a/tests/Support/Rest/Resources/BelongsToManyResource.php b/tests/Support/Rest/Resources/BelongsToManyResource.php index 058a31f..0f0ead9 100644 --- a/tests/Support/Rest/Resources/BelongsToManyResource.php +++ b/tests/Support/Rest/Resources/BelongsToManyResource.php @@ -26,7 +26,7 @@ public function fields(RestRequest $request): array return [ 'id', 'number', - 'other_number' + 'other_number', ]; } }