-
Notifications
You must be signed in to change notification settings - Fork 0
Issue/rdt 440 #9
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Add issue spatie#175 link in selecting-fields.md
Update filtering.md
Bumps [dependabot/fetch-metadata](https://github.com/dependabot/fetch-metadata) from 2.1.0 to 2.2.0. - [Release notes](https://github.com/dependabot/fetch-metadata/releases) - [Commits](dependabot/fetch-metadata@v2.1.0...v2.2.0) --- updated-dependencies: - dependency-name: dependabot/fetch-metadata dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]>
…ependabot/fetch-metadata-2.2.0 Bump dependabot/fetch-metadata from 2.1.0 to 2.2.0
[DOCS] Update Frontend implementation with a new one
Update Documentation for php markdown
…turn-static AllowedFilter should return static rather than self
* [FEAT] add filter by operators * [TEST] add test cases for filter by operator * [FEAT] add dynamic operator to filer by operators * [TEST] test dynamic operator filter
--- For more details, open the [Copilot Workspace session](https://copilot-workspace.githubnext.com/spatie/laravel-query-builder?shareId=XXXX-XXXX-XXXX-XXXX).
…ifyEscapeChar. Fixes spatie#941 (spatie#968) * Fix styling * Removed explicit escaping for pgsql driver in FilterPartial#maybeSpecifyEscapeChar. Fixes spatie#941 Added mariadb driver in FilterPartial#maybeSpecifyEscapeChar phpdoc for param $driver. Also adjusted test in order to run with mariadb driver only if the installed version of illuminate/database dependency supports the driver. * Fix styling --------- Co-authored-by: Talpx1 <[email protected]> Co-authored-by: Alex Vanderbist <[email protected]>
…tie#976) Problem discussed here spatie#334
Fixed bug where the include count fails if a relation has the word `Count` in it. For example, it fails for `billingCountry` by removing the word `Country` and trying to load a `billing` relation.
Remove PHPUnit cache
Laravel 12.x Compatibility
Exclude `.github` folder and `.php_cs` from being included in composer installation
Enhance QueryBuilder with generics support for better type inference
update some links so point to v6 docs
Bumps [dependabot/fetch-metadata](https://github.com/dependabot/fetch-metadata) from 2.3.0 to 2.4.0. - [Release notes](https://github.com/dependabot/fetch-metadata/releases) - [Commits](dependabot/fetch-metadata@v2.3.0...v2.4.0) --- updated-dependencies: - dependency-name: dependabot/fetch-metadata dependency-version: 2.4.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]>
…dependabot/fetch-metadata-2.4.0 Bump dependabot/fetch-metadata from 2.3.0 to 2.4.0
# Conflicts: # src/Filters/FiltersPartial.php
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (2)
Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughDeze wijziging introduceert nieuwe filtermogelijkheden in de query builder, waaronder operator-gebaseerde filters en filtering op "belongs to"-relaties. Daarnaast zijn er configuratieopties toegevoegd voor naamgevingsconventies van relaties en velden. De testdekking is uitgebreid met nieuwe scenario's, en de afhankelijkheden zijn bijgewerkt voor compatibiliteit met Laravel 12.x. Documentatie en changelog zijn geüpdatet, en er zijn kleine aanpassingen gedaan aan de codeopmaak en importvolgorde. Changes
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 7
🧹 Nitpick comments (23)
README.md (2)
1-7: Overweeg om externe links te beveiligen en in een nieuw tabblad te openen
Het<a>-element verwijst naar een externe site. Voor een betere gebruikerservaring en om veiligheidsrisico’s (zoals tabnapping) te voorkomen, voegtarget="_blank"enrel="noopener noreferrer"toe.
Voorbeeld diff:- <a href="https://spatie.be/open-source?utm_source=github&utm_medium=banner&utm_campaign=laravel-query-builder"> + <a href="https://spatie.be/open-source?utm_source=github&utm_medium=banner&utm_campaign=laravel-query-builder" target="_blank" rel="noopener noreferrer">
9-9: Controleren of de H1-structuur behouden blijft
De<h1>is nu onderdeel van een HTML-blok; verifieer in verschillende weergaves (GitHub, docs-site) dat deze nog steeds als kopniveau 1 wordt herkend en niet als gewone tekst.src/Enums/FilterOperator.php (1)
15-18: MethodisDynamicmist return-typehint
Om de code explicieter en type-veiliger te maken, voeg: booltoe aan de methode-signature:- public function isDynamic() + public function isDynamic(): booldocs/introduction.md (3)
26-26: Verwijder de dubbele punt na ‘like’
Typografisch is het beter om de dubbele punt te schrappen bij “features like”:-[Read more about filtering features like: partial filters, exact filters, scope filters, ...] +[Read more about filtering features like partial filters, exact filters, scope filters, ...]🧰 Tools
🪛 LanguageTool
[typographical] ~26-~26: Do not use a colon (:) before a series that is introduced by a preposition (‘like’). Remove the colon or add a noun or a noun phrase after the preposition.
Context: ...`` [Read more about filtering features like: partial filters, exact filters, scope f...(RP_COLON)
38-38: Verwijder de dubbele punt en herziet formulering
Pas “include features like: including nested relationships” aan naar een vloeiender Engels, bijvoorbeeld:-[Read more about include features like: including nested relationships, including relationship count, ...] +[Read more about include features such as nested relationship inclusion, including relationship count, ...]🧰 Tools
🪛 LanguageTool
[uncategorized] ~38-~38: This verb may not be in the correct form. Consider using a different form for this context.
Context: ...irpostsloaded ``` [Read more about include features like: including nested relatio...(AI_EN_LECTOR_REPLACEMENT_VERB_FORM)
50-50: Verwijder de dubbele punt na ‘like’
Voor consistentie in de documentatie, verwijder de dubbele punt bij “sorting features like”:-[Read more about sorting features like: custom sorts, sort direction, ...] +[Read more about sorting features like custom sorts, sort direction, ...]🧰 Tools
🪛 LanguageTool
[typographical] ~50-~50: Do not use a colon (:) before a series that is introduced by a preposition (‘like’). Remove the colon or add a noun or a noun phrase after the preposition.
Context: ... ``` [Read more about sorting features like: custom sorts, sort direction, ...](http...(RP_COLON)
docs/advanced-usage/front-end-implementation.md (1)
14-14: Consistente lijststijl: voeg dubbele punt toe
Voor uniformiteit met andere items (bijv. “React:” en “Typescript:”) voeg een dubbele punt toe na “Typescript + React”:-- Typescript + React [react-query-builder]... +- Typescript + React: [react-query-builder]...src/Filters/FiltersPartial.php (1)
77-77: Correctie voor PostgreSQL LIKE queries escape karakter.De wijziging verwijdert PostgreSQL uit de lijst van databases die een expliciet escape karakter nodig hebben. Dit is een goede aanpassing omdat PostgreSQL anders omgaat met LIKE escaping dan SQLite en SQL Server.
Het PHPDoc-commentaar op regel 72 vermeldt nog steeds 'pgsql' als een van de mogelijke drivertypen en bevat een typefout ('sqlsrc' in plaats van 'sqlsrv'). Het zou goed zijn om deze documentatie bij te werken zodat deze overeenkomt met de implementatie:
- * @param 'sqlite'|'pgsql'|'sqlsrc'|'mysql' $driver + * @param 'sqlite'|'sqlsrv'|'mysql'|string $driverCHANGELOG.md (1)
105-105: Kleine typografische kwestie.De term "php markdown" zou correct geschreven moeten worden als "PHP Markdown" (met hoofdletters).
-* Update Documentation for php markdown by @chengkangzai in https://github.com/spatie/laravel-query-builder/pull/969 +* Update Documentation for PHP Markdown by @chengkangzai in https://github.com/spatie/laravel-query-builder/pull/969🧰 Tools
🪛 LanguageTool
[grammar] ~105-~105: Did you mean the formatting language “Markdown” (= proper noun)?
Context: ...pull/961 * Update Documentation for php markdown by @chengkangzai in https://github.com/...(MARKDOWN_NNP)
.github/workflows/run-tests.yml (1)
72-72: Mogelijke type-inconsistentie in de job.services.mysql.ports expressieEr is een potentiële type-mismatch in de toegang tot
job.services.mysql.ports[3306]. ActionLint verwacht een string maar krijgt een nummer.Het is aan te raden om dit formaat te controleren, hoewel dit vaak correct werkt in GitHub Actions. Als je problemen ondervindt, kun je overwegen dit naar een string te converteren of een andere syntax te gebruiken.
🧰 Tools
🪛 actionlint (1.7.4)
72-72: property access of object must be type of string but got "number"
(expression)
docs/features/filtering.md (2)
223-223: Ontbrekende komma in tekstIn de zin "When passing an array as a parameter you can access it, as an array, in the scope..." ontbreekt mogelijk een komma na "parameter".
-When passing an array as a parameter you can access it, as an array, in the scope by using the spread operator. +When passing an array as a parameter, you can access it, as an array, in the scope by using the spread operator.🧰 Tools
🪛 LanguageTool
[uncategorized] ~223-~223: Possible missing comma found.
Context: ...8-12-31 ``` When passing an array as a parameter you can access it, as an array, in the ...(AI_HYDRA_LEO_MISSING_COMMA)
242-246: Kleine spelfout in uitleg over route model bindingEr staat "Remeber" in plaats van "Remember" in de tekst over route model binding.
-If you use any other column aside `id` column for route model binding (ULID,UUID). Remeber to specify the value of the column used in route model binding +If you use any other column aside `id` column for route model binding (ULID,UUID). Remember to specify the value of the column used in route model bindingconfig/query-builder.php (2)
64-73: Onthoud consistent documentatie en type-consistentie voor nieuwe config-optieDe PHP-doc beschrijft dat de sleutel één van
snake_case,camelCaseofnoneverwacht, maar de default-waarde isfalse(boolean).
Dit leidt tot twee potentiële valkuilen:
- Type-inconsistentie → down-stream code moet nu zowel booleans als strings afhandelen.
- De inleidende paragraaf is grotendeels gekopieerd uit de vorige blokken en refereert nog steeds aan “snake case plural” i.p.v. tabelnamen, waardoor de bedoeling niet 100 % duidelijk is.
Overweeg daarom:
- 'convert_relation_table_name_strategy' => false, + // Mogelijke waarden: 'snake_case', 'camelCase', 'none'. + // Gebruik 'none' (of null) om de functionaliteit uit te schakelen, + // zodat het type altijd string|null i.p.v. bool|string is. + 'convert_relation_table_name_strategy' => 'none',en herschrijf de commentaar-regels zodat ze expliciet gaan over “relation table names” i.p.v. relation-namen.
75-81: Kruisverwijzing met eerder blok zou nuttig zijnJe introduceert nu ook
convert_field_names_to_snake_case. Een korte verwijzing naar het feit dat dit werkt in combinatie met de zojuist toegevoegde tabel-naam-strategie helpt toekomstige maintainers om de samenhang sneller te zien.tests/FieldsTest.php (2)
86-99: Config-wijzigingen resetten om test-interferentie te voorkomenDeze test zet runtime-config op:
config(['query-builder.convert_field_names_to_snake_case' => true]); config(['query-builder.convert_relation_table_name_strategy' => 'camelCase']);Laravel/Pest herstelt config niet automatisch tussen tests wanneer de applicatie niet telkens wordt herstart. Voeg daarom een teardown/reset toe of gebruik
Config::setbinnen eenrefreshApplication()-context om neveneffecten op volgende tests te vermijden.
218-238: Assert op volledige SELECT-string is fragielDeze verwachtingen vergelijken de ruwe SQL tot op spaties en alias-namen:
$this->assertQueryLogContains('select `related_through_pivot_models`.`id`, …');Kleine veranderingen in Laravel-versie, database-driver of query-optimizer kunnen de volgorde van kolommen, back-ticks of spaties wijzigen, waardoor de test onverwacht faalt.
Alternatief: parseer de query of gebruik meer tolerante asserts, b.v.
assertStringContainsString('fromrelated_through_pivot_models')gecombineerd met asserts op aanwezigheid van de belangrijkste kolomnamen.tests/FilterTest.php (2)
36-48: Instellen van custom query-parameter zonder herstel kan tot lekken leidenDe test wijzigt de configuratie-parameter:
config(['query-builder.parameters.filter' => 'custom_filter']);Zonder reset blijft deze waarde mogelijk actief voor volgende tests, wat onbedoelde falers kan opleveren. Overweeg gebruik van:
$this->beforeApplicationDestroyed(fn () => config()->offsetUnset('query-builder.parameters.filter'));of gebruik Laravel’s
refreshApplication()helper.
785-896: Operator-filter-tests missen negatieve controlesDe nieuwe operator-tests verifiëren alleen dat een query wel resultaten oplevert. Het zou de robuustheid verhogen om ook het spiegelbeeld te testen (bijv. voor
GREATER_THANcontroleren dat een kleiner salaris niét wordt teruggegeven). Zo voorkom je dat een foutieve implementatie die altijd alle records retourneert toch groen door de tests komt.src/Concerns/AddsFieldsToQuery.php (1)
43-50: Strategiekeuze ‘none’ ontbreekt in de conversielogica
Deconvert_relation_table_name_strategy-config accepteert volgens de nieuwe documentatie ook de waarde'none', maar alleen de varianten'camelCase'en'snake_case'worden hier afgevangen. Dit kan leiden tot onverwachte resultaten als een ontwikkelaar expliciet'none'instelt – de waarde wordt dan genegeerd in plaats van dat het model-tabelnaam ongewijzigd blijft.-if (config('query-builder.convert_relation_table_name_strategy', false) === 'camelCase') { +switch (config('query-builder.convert_relation_table_name_strategy', false)) { + case 'camelCase': $modelTableName = Str::camel($modelTableName); -} - -if (config('query-builder.convert_relation_table_name_strategy', false) === 'snake_case') { + break; + case 'snake_case': $modelTableName = Str::snake($modelTableName); -} + break; + // 'none' of ongeldige waarden laten het origineel ongemoeid +}src/Filters/FiltersBelongsTo.php (2)
56-66: Type-hints verruimen of exception-pad vereenvoudigen
getRelatedModelFromRelation()geeft in de header?Modelterug, maar kan in praktijk nooitnullretourneren; bij een ongeldige relatie wordt direct eenRelationNotFoundExceptiongegooid. Dit type-verschil kan verwarring wekken in IDE’s. Overweeg om de return-type naarModelte veranderen óf een explicietereturn null-pad toe te voegen.
68-82: Overbodige parameter$levelmaakt code onnodig complex
De recursieve helpergetModelFromRelation()accepteert$level, maar deze variabele wordt nergens gebruikt. Dit suggereert een resterend artefact van een eerdere implementatie. Het is beter om ongebruikt argumentatie te verwijderen om leesbaarheid te vergroten.-protected function getModelFromRelation(Model $model, string $relation, int $level = 0): ?Model +protected function getModelFromRelation(Model $model, string $relation): ?Model @@ - return $this->getModelFromRelation($firstRelatedModel, implode('.', array_slice($relationParts, 1)), $level + 1); + return $this->getModelFromRelation($firstRelatedModel, implode('.', array_slice($relationParts, 1)));src/AllowedFilter.php (1)
86-92: Parameter-volgorde wijkt af van bestaande patronen
De nieuwebelongsTo()-methode plaatst$internalNamevóór$arrayValueDelimiter, terwijl andere factory-methodes eerst de delimiter accepteren. Dit kan voor verwarring zorgen bij gebruikers die middels IDE-auto-complete parameters kopiëren. Overweeg een uniforme signatuur door$arrayValueDelimiterconsequent als voorlaatste argument te zetten.src/Filters/FiltersOperator.php (1)
31-38: Mogelijke scope-issue in closure bij array-filtersBinnen de closure wordt variabele
$valueuit de outer-scope meegegeven terwijl dezelfde naam binnen deforeachwordt overschreven door$item. Dit kan tot verwarring leiden; enkel$propertyhoeft worden ge-captured.- $query->where(function ($query) use ($value, $property) { + $query->where(function ($query) use ($property) { foreach ($value as $item) { $this->__invoke($query, $item, $property); } });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (35)
.gitattributes(1 hunks).github/workflows/dependabot-auto-merge.yml(1 hunks).github/workflows/run-tests.yml(1 hunks).gitignore(1 hunks).phpunit.cache/test-results(0 hunks)CHANGELOG.md(1 hunks)README.md(1 hunks)composer.json(1 hunks)config/query-builder.php(1 hunks)database/factories/AppendModelFactory.php(1 hunks)database/factories/TestModelFactory.php(0 hunks)docs/_index.md(1 hunks)docs/advanced-usage/front-end-implementation.md(2 hunks)docs/features/filtering.md(4 hunks)docs/features/selecting-fields.md(2 hunks)docs/introduction.md(4 hunks)phpstan.neon.dist(1 hunks)src/AllowedField.php(2 hunks)src/AllowedFilter.php(5 hunks)src/Concerns/AddsFieldsToQuery.php(2 hunks)src/Enums/FilterOperator.php(1 hunks)src/Filters/FiltersBelongsTo.php(1 hunks)src/Filters/FiltersExact.php(1 hunks)src/Filters/FiltersOperator.php(1 hunks)src/Filters/FiltersPartial.php(1 hunks)src/Filters/FiltersScope.php(1 hunks)src/Includes/IncludedCount.php(1 hunks)src/Includes/IncludedRelationship.php(2 hunks)src/QueryBuilder.php(2 hunks)tests/FieldsTest.php(5 hunks)tests/FilterTest.php(30 hunks)tests/RelationFilterTest.php(2 hunks)tests/SortTest.php(0 hunks)tests/TestCase.php(3 hunks)tests/TestClasses/Models/TestModel.php(2 hunks)
💤 Files with no reviewable changes (3)
- database/factories/TestModelFactory.php
- tests/SortTest.php
- .phpunit.cache/test-results
🧰 Additional context used
🧬 Code Graph Analysis (5)
database/factories/AppendModelFactory.php (1)
tests/TestClasses/Models/AppendModel.php (1)
AppendModel(8-25)
tests/RelationFilterTest.php (2)
tests/Pest.php (1)
createQueryFromFilterRequest(11-20)src/AllowedFilter.php (2)
AllowedFilter(18-207)operator(119-124)
src/Includes/IncludedCount.php (1)
src/AllowedInclude.php (1)
count(58-63)
src/Filters/FiltersBelongsTo.php (3)
src/Filters/FiltersPartial.php (1)
__invoke(14-44)src/Filters/FiltersExact.php (1)
__invoke(23-40)src/Filters/FiltersScope.php (1)
__invoke(23-44)
tests/TestClasses/Models/TestModel.php (2)
tests/TestClasses/Models/RelatedModel.php (2)
nestedRelatedModels(21-24)RelatedModel(10-30)tests/TestClasses/Models/NestedRelatedModel.php (1)
NestedRelatedModel(8-18)
🪛 LanguageTool
docs/introduction.md
[typographical] ~26-~26: Do not use a colon (:) before a series that is introduced by a preposition (‘like’). Remove the colon or add a noun or a noun phrase after the preposition.
Context: ...`` [Read more about filtering features like: partial filters, exact filters, scope f...
(RP_COLON)
[uncategorized] ~38-~38: This verb may not be in the correct form. Consider using a different form for this context.
Context: ...ir posts loaded ``` [Read more about include features like: including nested relatio...
(AI_EN_LECTOR_REPLACEMENT_VERB_FORM)
[typographical] ~50-~50: Do not use a colon (:) before a series that is introduced by a preposition (‘like’). Remove the colon or add a noun or a noun phrase after the preposition.
Context: ... ``` [Read more about sorting features like: custom sorts, sort direction, ...](http...
(RP_COLON)
docs/features/filtering.md
[uncategorized] ~223-~223: Possible missing comma found.
Context: ...8-12-31 ``` When passing an array as a parameter you can access it, as an array, in the ...
(AI_HYDRA_LEO_MISSING_COMMA)
CHANGELOG.md
[grammar] ~105-~105: Did you mean the formatting language “Markdown” (= proper noun)?
Context: ...pull/961 * Update Documentation for php markdown by @chengkangzai in https://github.com/...
(MARKDOWN_NNP)
🪛 actionlint (1.7.4)
.github/workflows/run-tests.yml
72-72: property access of object must be type of string but got "number"
(expression)
🔇 Additional comments (35)
database/factories/AppendModelFactory.php (1)
5-6: Imports geordend volgens conventies
De Laravel-core import (Factory) staat nu vóór de project-specifiekeAppendModelimport, conform de standaard import-groepering in dit project..gitignore (1)
6-6: Goedkeuring: negeer PHPUnit-cache
Deze wijziging voegt.phpunit.cachetoe aan.gitignore, waardoor de PHPUnit-cache niet in versiebeheer wordt opgenomen en de repository schoon blijft..gitattributes (2)
7-7: Goedkeuring: export-ignoreren van.githubmap
Voegt/.githubtoe aan export-ignore, zodat CI-workflowconfiguraties niet in distributiepakketten terechtkomen.
12-12: Goedkeuring: export-ignoreren van.php_cs.dist.php
Voegt.php_cs.dist.phptoe aan export-ignore, waardoor dit configuratiebestand niet in distributiepakketten wordt opgenomen.README.md (1)
1-16: Bevestig dat de introductietekst bewust is verwijderd
De oorspronkelijke beschrijvende paragraaf onder de kop is weggehaald. Dit kan de eerste indruk en context voor nieuwe gebruikers verminderen. Bevestig of dit intentional is of overweeg een beknopte toelichting opnieuw toe te voegen.docs/_index.md (1)
2-2: Documentatieversie correct bijgewerkt.De titel is bijgewerkt van "v5" naar "v6", wat consistent is met de versie-update van het pakket.
.github/workflows/dependabot-auto-merge.yml (1)
16-16: Goede update van de dependabot metadata actie.De update van
dependabot/fetch-metadatavan v2.1.0 naar v2.4.0 zorgt voor verbeterde functionaliteit en beveiliging bij het automatisch samenvoegen van afhankelijkheidsupdates.composer.json (2)
24-26: Laravel 12.0 ondersteuning toegevoegd.Prima aanpassing om ondersteuning toe te voegen voor Laravel 12.0 naast de bestaande ondersteuning voor 10.0 en 11.0.
33-35: Dev-afhankelijkheden bijgewerkt.Goede updates van test-gerelateerde afhankelijkheden om compatibiliteit met nieuwere versies te garanderen:
orchestra/testbenchondersteunt nu ook versie 10.0pestphp/pestondersteunt nu versie 3.7phpunit/phpunitondersteunt nu versie 11.5.3Let op:
nunomaduro/larastanis verwijderd uit derequire-devsectie, wat overeenkomt met de wijziging in het phpstan.neon.dist bestand.phpstan.neon.dist (1)
2-2: Larastan pad bijgewerkt.Het pad naar de Larastan extensie is bijgewerkt van
./vendor/nunomaduro/larastan/extension.neonnaar./vendor/larastan/larastan/extension.neon, wat overeenkomt met de verwijdering van het nunomaduro/larastan pakket uit composer.json.src/Enums/FilterOperator.php (1)
5-13: Nieuwe enum-gevallen correct gedefinieerd
DeFilterOperatorenum bevat alle vereiste operatoren (leeg teken,=,<>,<,>,<=,>=), wat zorgt voor een typeveilige representatie in de query builder.docs/advanced-usage/front-end-implementation.md (1)
3-3: Controleer navigatievolgorde
Deweightis verhoogd van 3 naar 6; bevestig dat dit de gewenste plaatsing in de documentatie-navigatie oplevert.docs/features/selecting-fields.md (2)
12-18: Voorbeeldcode verduidelijkt met PHP-block
Mooi dat je de GET-aanroep als commentaar en de PHP-syntaxmarkering hebt toegevoegd; dit verhoogt de duidelijkheid voor ontwikkelaars.
54-54: Klikbare issue-link is nuttig
De conversie van#175naar een Markdown-link verbetert de navigatie naar het bijbehorende GitHub-issue.src/Filters/FiltersExact.php (1)
69-73: Correcte variabelevolgorde inuse-statement
Het omdraaien van$propertyen$valuein deuse-clausule heeft geen functionele impact, maar verbetert de leesbaarheid door consistentie met de closure-parameters.tests/TestCase.php (3)
40-41: Goede toevoeging van testkolommen voor nieuwe filterfunctionaliteit.Deze nieuwe kolommen (
full_nameensalary) ondersteunen de nieuwe filtermogelijkheden die in deze PR worden geïntroduceerd. Het nullable maken van deze velden biedt flexibiliteit bij het testen.
66-66: Consistente kolom toevoeging voor gerelateerde modellen.De toevoeging van de
full_namekolom aan de gerelateerde modellen tabel is consistent met de wijzigingen in de hoofdtabel en ondersteunt het testen van relatie-gebaseerde filters.
97-97: Ray debugging provider uitgeschakeld.Het uitcommentariëren van de
RayServiceProvideris een logische aanpassing om onnodige dependencies tijdens het testen te verminderen.tests/TestClasses/Models/TestModel.php (1)
11-11: Goede implementatie van HasManyThrough relatie voor geneste filters.De toevoeging van de
nestedRelatedModelsrelatie is een goede uitbreiding die het testen van de nieuwe filterfunctionaliteit voor geneste relaties mogelijk maakt. De relatie is duidelijk gedefinieerd met expliciete foreign en local keys, wat een best practice is.Also applies to: 31-41
src/AllowedField.php (1)
6-6: Uitstekende toevoeging van snake_case conversie voor veldnamen.De wijziging voegt ondersteuning toe voor het optioneel converteren van veldnamen naar snake_case, wat goed aansluit bij de configuratieopties voor naamgevingsconventies die in deze PR worden geïntroduceerd. De implementatie is netjes en maakt gebruik van Laravel's
Str::snake()helper.De standaardwaarde
falsezorgt voor backward compatibility met bestaande code, wat een goede ontwikkelpraktijk is.Also applies to: 40-46
tests/RelationFilterTest.php (2)
4-4: Goede toevoeging voor de nieuwe operator filter functionaliteit.De import van
FilterOperatorenum is correct toegevoegd om de nieuwe operator filters in de test te kunnen gebruiken.
120-130: De test dekt de nieuwe operator filter functionaliteit goed af.De test volgt hetzelfde patroon als de bestaande tests voor exact en partial filtering, en test specifiek het gedrag van operator filtering wanneer
$addRelationConstraintop false staat. De SQL controle verzekert dat de query correct wordt gegenereerd.src/Filters/FiltersScope.php (2)
57-57: Verbeterde syntax met PHP 8 nullsafe operator.De wijziging van
optional()->isSubclassOf()naar nullsafe operator?->is een goede modernisering van de code die gebruik maakt van nieuwe PHP 8 features.
62-62: Onnodige nullsafe check verwijderd.De directe instantiëring van het model zonder nullsafe operator is correct omdat op dit punt in de code
$this->getClass($parameter)gegarandeerd niet null is (gezien de vorige check op regel 57).CHANGELOG.md (1)
5-115: Complete en gedetailleerde changelog updates.De changelog is bijgewerkt met alle nieuwe functionaliteiten en fixes voor versies 6.1.0 t/m 6.3.2, inclusief:
- Generics support voor betere type inference
- Laravel 12.x compatibiliteit
- Nieuwe "belongs to" filters
- Operator-gebaseerde filters
- Configuratieopties voor naamgevingsconventies
- Diverse bugfixes en documentatie-updates
Alle entries volgen het standaard formaat met datums, wijzigingen, contributors en links naar volledige changelogs.
🧰 Tools
🪛 LanguageTool
[grammar] ~105-~105: Did you mean the formatting language “Markdown” (= proper noun)?
Context: ...pull/961 * Update Documentation for php markdown by @chengkangzai in https://github.com/...(MARKDOWN_NNP)
src/QueryBuilder.php (2)
17-18: Goede toevoeging van generieke type-annotatiesDe template-annotatie
@template TModel of Modelen de bijbehorende@mixin EloquentBuilder<TModel>zorgen voor betere type-inferentie en ondersteuning in IDEs. Dit verbetert de statische analyse zonder runtime-gedrag te wijzigen.
61-64: Uitstekende verbetering van type-veiligheidHet toevoegen van de PHPDoc-annotatie en het gebruik van een variabele om de query builder vast te leggen zorgt ervoor dat de generieke type-informatie behouden blijft in de return waarde. Dit verbetert IDE-ondersteuning en statische analyse tools kunnen hiermee beter typen infereren.
.github/workflows/run-tests.yml (2)
3-4: Vereenvoudigde trigger-conditieDe configuratie is vereenvoudigd door op elke push de tests uit te voeren, in plaats van specifieke bestandspaden te filteren.
16-25: Goede uitbreiding van test matrixDe ondersteuning voor PHP 8.4 en Laravel 12.* is toegevoegd aan de testmatrix, samen met de bijbehorende testbench versies. Dit zorgt voor betere compatibiliteit en toekomstbestendigheid.
src/Includes/IncludedRelationship.php (3)
25-39: Goede implementatie van dynamische tabel naam resolutieDe code implementeert een configureerbare strategie voor het omzetten van relatienamen naar tabelnamen. De try-catch structuur zorgt ervoor dat fouten bij het ophalen van gerelateerde modellen geen problemen veroorzaken.
40-40: Verbeterde flexibiliteit bij het doorgeven van tabelnamenDe functie
getRequestedFieldsForRelatedTableis aangepast om een optionele tabelnaam parameter te accepteren, wat meer flexibiliteit biedt in naamconventies.
48-48: Beter kwalificeren van kolommenHet gebruik van
$query->qualifyColumns($fields)zorgt ervoor dat kolommen correct worden gekwalificeerd met tabelnamen, wat SQL-fouten voorkomt bij complexe queries met meerdere tabellen.docs/features/filtering.md (2)
89-121: Uitstekende documentatie voor operator filtersDe nieuwe sectie over operator filters is duidelijk en goed gestructureerd. De voorbeelden tonen zowel statische operator filters (zoals GREATER_THAN) als dynamische operator filters, wat gebruikers helpt bij het implementeren van deze functionaliteit.
137-184: Duidelijke documentatie voor BelongsTo filtersDe sectie over BelongsTo filters bevat goede voorbeelden, inclusief basisgebruik, aliassen en geneste relaties. De code-voorbeelden zijn helder en demonstreren goed hoe deze filters in verschillende scenario's gebruikt kunnen worden.
tests/FilterTest.php (1)
125-152: Driver-specifieke ESCAPE-assert kan fout-positieven opleverenDe test verwacht geen
ESCAPE '\'voor MySQL/MariaDB/pgsql:$query->not->toContain("ESCAPE '\\'")Maar sommige versies/instellingen (e.g. ANSI_QUOTES) kunnen tóch een escape toevoegen. Overweeg om de assert te beperken tot het voorkomen van dubbele escapes of gebruik van regex die tolerant is voor driver-variaties.
|
@coderabbitai resolve |
✅ Actions performedComments resolved and changes approved. |
Summary by CodeRabbit
Nieuwe functies
Bugfixes
Documentatie
Tests
Chores