Skip to content

Where Query documentation improvements#15599

Open
gsartori wants to merge 13 commits intoapache:7.0.xfrom
gsartori:7.0.x
Open

Where Query documentation improvements#15599
gsartori wants to merge 13 commits intoapache:7.0.xfrom
gsartori:7.0.x

Conversation

@gsartori
Copy link
Copy Markdown
Contributor

No description provided.

@bito-code-review
Copy link
Copy Markdown

This question isn’t related to the pull request. I can only help with questions about the PR’s code or comments.

@gsartori gsartori changed the title Simplified sorting chapter in where queries docs + added multiple field sorting Where Query documentation improvements Apr 24, 2026
@jdaugherty
Copy link
Copy Markdown
Contributor

@gsartori is this ready to review again?

@gsartori
Copy link
Copy Markdown
Contributor Author

gsartori commented Apr 26, 2026

@gsartori is this ready to review again?

Nope, ill complete it tomorrow in consideration of your review, ill comment here once done

@gsartori
Copy link
Copy Markdown
Contributor Author

@jdaugherty I've reintroduced the paragraph about the auto-generated aliases. The PR can be reviewd 👍

@jdaugherty jdaugherty requested a review from jamesfredley April 28, 2026 00:10
Copy link
Copy Markdown
Contributor

@jamesfredley jamesfredley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the documentation changes against the codebase. The technical claims are accurate (delete() failOnError omission, Map-based multi-field sort, nested criteria block, properties = [...] binding, list() pagination params - all verified against DetachedCriteria.groovy, DynamicFinder.java, GormInstanceApi.groovy, DetachedCriteriaTransformer.java, and WebDataBinding). Leaving line-level suggestions for editorial / consistency / flow polish. The Map-sort feature was previously undocumented, so this PR genuinely fills a gap.

Non-blocking but worth noting:

  • The PR body is empty - CONTRIBUTING.md expects a description and (where applicable) issue references.
  • Branch 7.0.x -> 7.0.x skips the docs/ prefix convention from AGENTS.md, so auto-labelers won't tag this as documentation.
  • The newly-documented Map-sort path (DynamicFinder.applySortForMap) has no test coverage in DetachedCriteriaSpec. Per AGENTS.md rule #10 ("every code change must include new or enhanced tests"), consider adding a small spec to lock in the contract being documented.
  • grails-data-graphql/.../DeleteEntityDataFetcher.groovy incorrectly passes failOnError: true to delete() - that parameter is silently ignored. Worth filing separately.

Comment thread grails-doc/src/en/guide/GORM/quickStartGuide/basicCRUD.adoc
Comment thread grails-doc/src/en/guide/GORM/quickStartGuide/basicCRUD.adoc
Comment thread grails-doc/src/en/guide/GORM/quickStartGuide/basicCRUD.adoc
Comment thread grails-doc/src/en/guide/GORM/quickStartGuide/basicCRUD.adoc Outdated
Comment thread grails-doc/src/en/guide/GORM/quickStartGuide/basicCRUD.adoc
Comment thread grails-doc/src/en/guide/GORM/quickStartGuide/basicCRUD.adoc
Comment thread grails-doc/src/en/guide/GORM/quickStartGuide/basicCRUD.adoc Outdated
Comment thread grails-data-hibernate5/docs/src/docs/asciidoc/querying/whereQueries.adoc Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

4 participants