Skip to content

feat(model): allow select() and friends to start a query-builder chain (#3346) - #3368

Merged
bpamiri merged 2 commits into
developfrom
fix/bot-3346-select-chain-entry
Aug 5, 2026
Merged

feat(model): allow select() and friends to start a query-builder chain (#3346)#3368
bpamiri merged 2 commits into
developfrom
fix/bot-3346-select-chain-entry

Conversation

@bpamiri

@bpamiri bpamiri commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator

What / Why

model("Person").select("id,firstName").where(...).get() threw Wheels.MethodNotFound because the model's chain-entry dispatch list in onMissingMethod only covered where/orderBy-style methods. QueryBuilder.cfc already implements select(), include(), group(), distinct(), and forUpdate() — they just couldn't START a chain, only continue one. This closes that API-consistency gap.

Approach

  • vendor/wheels/model/onmissingmethod.cfc: extended the chain-entry ListFindNoCase list with select,include,group,distinct,forUpdate, plus a comment noting user scopes / real model methods keep precedence and that the list must stay in sync with ScopeChain.
  • vendor/wheels/model/query/ScopeChain.cfc: added forUpdate to the scope-to-builder transition list for parity (it was the lone QueryBuilder method missing there), plus a comment pinning that user-defined scopes are checked BEFORE this list.
  • Specs (queryBuilderSpec.cfc): chain-start select() asserting the returned columnList, the issue's exact select().where().get() example, include/group/distinct/forUpdate chain-start smoke tests, and the scope().forUpdate() transition. All 7 written first and confirmed red (Error) before the implementation.
  • Docs: builder-methods table + entry-position example in web/sites/guides/.../query-builder-and-scopes.mdx; corrected the repo-root CLAUDE.md query-builder method list (previously omitted select).
  • Changelog fragment: changelog.d/query-builder-select-entry.added.md.

Behavior-change surface is minimal: every affected name previously threw Wheels.MethodNotFound from entry position, so nothing working today changes. Pure string-list edit — no closures, struct literals, or tag wrappers (no cross-engine anti-pattern surface).

Test evidence

  • TDD red: tools/test-local.sh model → 960 passed, 0 failed, 7 errors (the 7 new specs).
  • TDD green: tools/test-local.sh model → 967 passed, 0 failed, 0 errors.
  • Full core suite (Lucee 7 + SQLite, port 8091): 4664 pass / 1 fail / 8 errors — the 9 non-passing are all Playwright browser specs (BrowserLauncherSpec etc., driver not installed in this environment). Proven pre-existing: detached origin/develop baseline on the same machine → 4657 pass / 1 fail / 8 errors, identical failure set. Delta is exactly the +7 new specs.

Fixes #3346

🤖 Generated with Claude Code

#3346)

Extends the chain-entry dispatch list in model onMissingMethod with
select, include, group, distinct, and forUpdate so every QueryBuilder
method can start a chain directly on the model class, matching where()
and orderBy(). Adds forUpdate to the ScopeChain builder-transition list
for parity. User-defined scopes keep precedence in both dispatchers.

Specs pin the issue's exact example (select().where().get()), the
returned columnList, include/group/distinct smoke coverage, and the
scope-to-forUpdate transition. Guide and CLAUDE.md quick reference now
document the entry-position builder methods.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Peter Amiri <petera@pai.com>
wheels-bot[bot]
wheels-bot Bot previously approved these changes Aug 4, 2026

@wheels-bot wheels-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Wheels Bot — Reviewer

TL;DR — This PR makes select(), include(), group(), distinct(), and forUpdate() work as chain-entry points on the model (they previously threw Wheels.MethodNotFound from entry position despite already existing on QueryBuilder.cfc), and adds forUpdate to the scope-to-builder transition list for parity. Implementation, tests, docs, changelog, and CLAUDE.md are all in order. Verdict: approve.

Correctness

Verified the change does what the title/body claim:

  • vendor/wheels/model/query/QueryBuilder.cfc implements every newly-listed name — select (:195), include (:205), group (:215), distinct (:223), forUpdate (:237) — so all five can now legitimately begin a chain.
  • Precedence is preserved: in onmissingmethod.cfc named scopes (:12) and enum is* checkers (:30) are still evaluated before the builder entry block (:59), so a user scope named e.g. select keeps precedence. None of the five new names collide with dynamic-finder prefixes (findOneBy/findAllBy, :119) or association setters (:196), so their position relative to those is inert.
  • The two dispatch lists are now genuinely identical (both end ...distinct,forUpdate); before this PR ScopeChain.cfc:239 was the lone list missing forUpdate, so the "keep in sync" comments describe a state the PR actually establishes.

Tests

Well covered and TDD-first (7 specs, confirmed red-to-green in the PR body):

  • Test models exist and back the assertions — model("authorScoped") defines withLastNameDjurner (vendor/wheels/tests/_assets/models/AuthorScoped.cfc), and Author declares hasMany("posts") so include("posts") resolves.
  • The select().where().get() spec (queryBuilderSpec.cfc:247) mirrors the issue's exact example and asserts both recordcount and the restricted columnList, so it exercises the fix rather than just dispatch.
  • The scope-to-builder transition is pinned separately (model("authorScoped").withLastNameDjurner().forUpdate().count()).

Conventions

Minor, non-blocking:

  • onmissingmethod.cfc:56 — the new comment says "dynamic finders, association setters, and enum checkers above take precedence," but dynamic finders (:119) and association setters (:196) are actually handled below the builder block at :59. The claim is harmless because no builder name overlaps those handlers, but the word "above" is imprecise — only scopes and enum checkers sit above. Consider rewording to note that dynamic finders / association setters live below and simply never collide because no builder name matches their patterns.

Docs

  • CLAUDE.md builder method list corrected (it previously omitted both offset — already dispatchable — and the five names added here). Good catch.
  • web/sites/guides/.../query-builder-and-scopes.mdx table + entry-position example added.
  • Changelog fragment changelog.d/query-builder-select-entry.added.md uses the fragment system correctly (.added.md, full bullet), not a direct CHANGELOG.md edit.

Commits

Single commit feat(model): allow select() and friends to start a query-builder chain (#3346) — valid conventional-commit type/scope, header within 100 chars.

Cross-engine

No surface: the change is a pure ListFindNoCase string edit plus Invoke() delegation — no closures, struct literals, .map(), reserved-scope params, or tag wrappers. Nothing here behaves differently across Lucee/Adobe/BoxLang.

…ch list

The comment added in the #3346 change claimed dynamic finders and
association setters take precedence over the chain-entry list, but both
branches dispatch AFTER it in onMissingMethod — only user-defined scopes
and enum checkers run first. Comment-only change; no behavior change.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Peter Amiri <petera@pai.com>

@wheels-bot wheels-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Wheels Bot — Reviewer

TL;DR — This PR lets select(), include(), group(), distinct(), and forUpdate() begin a query-builder chain directly on the model (they already existed on QueryBuilder.cfc but previously threw Wheels.MethodNotFound from entry position), and adds forUpdate to the scope-to-builder transition list for parity. Head commit ebeb415f also corrects the precedence comment flagged on the prior commit. Implementation, TDD specs, guide docs, changelog fragment, and CLAUDE.md are all in order. Verdict: approve.

Correctness

Verified the change does what the title/body claim:

  • vendor/wheels/model/query/QueryBuilder.cfc implements every newly-listed name — select (:195), include (:205), group (:215), distinct (:223), forUpdate (:237) — so all five can now legitimately begin a chain via the Invoke() delegation in onmissingmethod.cfc:62.
  • Precedence holds: named scopes (onmissingmethod.cfc:12) and enum is* checkers (:30) are evaluated before the builder block (:61), so a user scope or enum named e.g. select keeps precedence. Dynamic finders (:120) and association methods (:198) run after the builder block, so the new comment stating an association named e.g. select resolves to the builder instead is accurate.
  • Both dispatch lists are now identical (each ends …distinct,forUpdate); ScopeChain.cfc:239 was the lone list missing forUpdate before this PR, so the "keep in sync" comments now describe a state the PR actually establishes.

The head-commit comment fix (ebeb415f) resolves the one nit from the prior review on b026ae35 — the earlier text said dynamic finders / association setters were "above," but they are below the builder block. The reworded comment now correctly states they run after the list.

Tests

Well covered and TDD-first (7 specs, confirmed red-to-green in the PR body):

  • Test models back the assertions — AuthorScoped.cfc defines the withLastNameDjurner scope, and Author.cfc:5 declares hasMany("posts") so include("posts") resolves.
  • select() specs (queryBuilderSpec.cfc:236, :247) assert the restricted columnList (not just dispatch), so they exercise the actual SELECT-narrowing, and the second mirrors the exact select().where().get() example from the issue.
  • The forUpdate() scope-to-builder transition is pinned separately (model("authorScoped").withLastNameDjurner().forUpdate().count()), and the inline comment correctly notes FOR UPDATE is a no-op on SQLite/MSSQL so the spec pins dispatch rather than locking.

Docs

  • CLAUDE.md builder-method list corrected (previously omitted select/offset and the four other names) with an explicit "any of these can START the chain" note.
  • web/sites/guides/.../query-builder-and-scopes.mdx adds the five methods to the table plus an entry-position select() example.
  • Changelog fragment changelog.d/query-builder-select-entry.added.md uses the fragment system correctly (.added.md, full bullet) — not a direct CHANGELOG.md edit.

Commits

Both commits are valid conventional commits within the 100-char header limit: feat(model): allow select() and friends to start a query-builder chain (#3346) and fix(model): correct precedence note on the builder chain-entry dispatch list.

Cross-engine

No surface: the change is a pure ListFindNoCase string-list edit plus Invoke() delegation — no closures, struct literals, .map(), reserved-scope params, or tag wrappers. ListSort(result.columnList, "textnocase") in the specs is portable across Lucee/Adobe/BoxLang.

@bpamiri
bpamiri merged commit f344e7d into develop Aug 5, 2026
18 checks passed
@bpamiri
bpamiri deleted the fix/bot-3346-select-chain-entry branch August 5, 2026 02:52
bpamiri added a commit that referenced this pull request Aug 5, 2026
)

The two forUpdate() chain-entry specs added by PR #3368 used .count() as
the terminal. Postgres and CockroachDB reject aggregate functions combined
with FOR UPDATE, so every postgres/cockroachdb leg in the compat matrix
failed with 'FOR UPDATE is not allowed with aggregate functions'. The specs
pin chain-entry dispatch, not locking semantics, so a non-aggregate .get()
terminal asserts the same behavior on every engine.

Verified locally on lucee7 + cockroachdb (previously failing leg):
4775 pass / 0 fail / 0 error; lucee7 + sqlite: 4763 pass / 0 fail / 0 error.

Signed-off-by: Peter Amiri <petera@pai.com>
Co-authored-by: Peter Amiri <petera@pai.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Chainable Query Builder with select

1 participant