fix(model): tableName("x") sets the table instead of silently no-opping - #3104
fix(model): tableName("x") sets the table instead of silently no-opping#3104wheels-bot[bot] wants to merge 2 commits into
Conversation
The documented setter form tableName("my_table") in config() was a silent
no-op: tableName() was a zero-argument getter, so the extra positional
argument was ignored and the model kept its convention table (first finder
throws Wheels.TableNotFound). tableName() now accepts an optional name and
delegates to table() when given, returning the resolved name; the
zero-argument getter behavior is unchanged.
Refs #3079
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
Wheels Bot — Docs updatedAdded a doc commit to this PR:
The six MDX guide pages and |
There was a problem hiding this comment.
Wheels Bot — Reviewer
TL;DR: This PR makes tableName(name) act as a setter (delegating to table()) instead of silently ignoring the argument, fixing the widely-documented-but-broken tableName("my_table") form from #3079. The implementation is minimal and correct, the new specs fail pre-fix and pass post-fix, and the changelog fragment and commit message follow repo conventions. Verdict: comment — no blocking findings, one minor test-hygiene nit.
Correctness
Verified, no issues:
table()atvendor/wheels/model/miscellaneous.cfc:35assignsvariables.wheels.class.tableNamedirectly, so the delegation intableName()(vendor/wheels/model/miscellaneous.cfc:182-185) genuinely takes effect, and the fall-through to the existinglowerCaseTableNamesbranch means the setter call returns the resolved name as the docblock claims.- The
any name(non-required) signature matchestable(required any name), sotableName(false)composes with the documentedtable(false)tableless-model escape hatch rather than fighting it. - No existing framework caller passes an argument to
tableName()—vendor/wheels/migrator/AutoMigrator.cfc:21,197,vendor/wheels/model/sql.cfc:1368, and$quotedTableName()(miscellaneous.cfc:198) are all zero-arg — so the overload cannot change behavior at any existing call site.
Cross-engine
No issues. The change is a plain optional argument plus StructKeyExists(arguments, "name"), which is portable across Lucee 5/6/7, Adobe 2018–2025, and BoxLang. The spec additions use the file's existing arrow-closure + unscoped-variable style and contain none of the known engine traps (no inline closures as constructor args, no local writes in catch/finally, no bare tag-in-script statements).
Tests
The three new specs in vendor/wheels/tests/specs/model/miscellaneousSpec.cfc:162-187 cover setter-takes-effect, setter-return-value, and getter-unchanged, and the PR body documents the pre-fix failure (Expected [tbl_authors_override] but received [c_o_r_e_authors]), confirming they exercise the bug.
One minor nit (non-blocking): the afterEach at miscellaneousSpec.cfc:165-168 restores the shared class state with a hardcoded literal:
g.model("author").table("c_o_r_e_authors")The sibling primaryKeys describe block in the same file (lines 51-62) instead captures the original value in beforeEach and restores that, which can't drift if the Author fixture's mapping ever changes. The risk here is small — the third spec asserts the convention value and would surface drift — but a beforeEach capture (originalTable = g.model("author").tableName()) would match the established restore pattern and is cheap to adopt in a follow-up.
Docs
Handled honestly: the function docblock is updated in place, the changelog fragment changelog.d/3079-tablename-setter.fixed.md follows the fragment convention (no direct CHANGELOG.md edit), and the PR body explicitly defers the six doc sites + CLAUDE.md:261 to bot-update-docs.yml with unchecked checklist boxes rather than claiming completion.
Commits
fix(model): tableName("x") sets the table instead of silently no-opping — valid type, suggested scope, subject under 100 chars, body explains the why, DCO Signed-off-by present. Conforms to commitlint.config.js.
There was a problem hiding this comment.
Wheels Bot — Reviewer
TL;DR: Re-review at head eb0eb5f. The delta since the previous review round (9767d77) is a single docs commit aligning .ai/wheels/snippets/model-snippets.md to the now-working tableName("custom_table_name") setter form. The framework change itself (optional name argument on tableName() delegating to table(), fixing the silent no-op from #3079) re-verifies clean: the delegation writes the same class data the getter reads, no existing call site passes an argument, and the three new specs cover setter-takes-effect, setter-return-value, and getter-unchanged. Verdict: comment — no blocking findings; one minor test-hygiene nit carried over from the prior round, plus a trivial checklist-staleness note.
Correctness
Verified, no issues:
table()atvendor/wheels/model/miscellaneous.cfc:35-37assignsvariables.wheels.class.tableNamedirectly, so the delegation atvendor/wheels/model/miscellaneous.cfc:183-185genuinely takes effect, and falling through to the existinglowerCaseTableNamesbranch makes the setter call return the resolved name exactly as the docblock claims.- The optional
any namesignature is compatible withtable(required any name), sotableName(false)composes with the documentedtable(false)tableless-model escape hatch rather than conflicting with it; the zero-argument getter path is byte-for-byte unchanged. - A repo-wide grep for
tableName(with an argument finds no existing caller of the public function with args — only the new specs and the unrelated$tableName(list, action)adapter helper (vendor/wheels/databaseAdapters/Base.cfc:304). The overload cannot change behavior at any existing call site.
Conventions
The snippet change in .ai/wheels/snippets/model-snippets.md:11 (table(...) → tableName(...)) is the right call: it aligns the AI reference with the form CLAUDE.md's Model Quick Reference teaches (tableName("tbl_users")), which is exactly the form this PR makes functional. The file has no other table( occurrences, so the doc is internally consistent.
Tests
The three specs at vendor/wheels/tests/specs/model/miscellaneousSpec.cfc:163-188 exercise the fix, and the PR body documents the pre-fix failure (Expected [tbl_authors_override] but received [c_o_r_e_authors]), confirming they fail without the change.
One minor nit, carried over unaddressed from the previous review round (still non-blocking): the afterEach at miscellaneousSpec.cfc:165-168 restores shared class state with a hardcoded literal:
g.model("author").table("c_o_r_e_authors")The sibling primaryKeys block in the same file (lines 51-62) captures the original value in beforeEach and restores that, which can't drift if the Author fixture's mapping ever changes. The third spec asserting the convention value keeps the risk small, so this stays a nit — fine to take in a follow-up.
Docs
The changelog fragment changelog.d/3079-tablename-setter.fixed.md follows the fragment convention (no direct CHANGELOG.md edit), and the function docblock is updated in place. One trivial staleness note: the PR body's checklist still marks AI Reference Docs as "deferred to bot-update-docs.yml", but head commit eb0eb5f now updates .ai/wheels/snippets/model-snippets.md itself — worth ticking/annotating that box so the checklist reflects what shipped.
Commits
Both commits conform to commitlint.config.js:
fix(model): tableName("x") sets the table instead of silently no-opping— valid type, suggested scope, subject under 100 chars, body explains the why, DCO sign-off present.docs: align model-snippets reference to use tableName() setter form— valid type, no scope (allowed), subject under 100 chars, DCO sign-off present and matching the committer.
|
Closing per maintainer design decision on issue #3079: the fail-loud guard ships instead of the setter overload — see #3134. Rationale: verified against the v2.5.1 source that tableName() has been a zero-arg getter (and table() the setter) unchanged since 2.5, so tableName("x") never worked in any era — the guard breaks nothing that ever functioned, keeps table() as the single canonical setter, and converts the always-silent mistake into a located development-mode error. This PR's two setter specs were verified non-tautological and their inverted shape is reused in #3134 with credit. The review's three criticals (docs re-canonicalization contradicting #3091, the CLAUDE.md truth-maintenance, docstring canonicality) are all mooted or handled by the guard approach. |
Summary
tableName()on models was a zero-argument getter while the table setter istable(name). CFML silently accepts the extra positional argument, so the widely-documentedtableName("my_table")form inconfig()did nothing — the model kept its convention table and the first finder threwWheels.TableNotFound(or silently hit the wrong table). This makestableName(name)act as a setter: when anameargument is passed it delegates totable()and returns the resolved name; the zero-argument getter behavior is unchanged. The alias follows the #2781 / #2803 precedent of accepting the form humans and agents keep reaching for.This PR ships the framework (model + spec) half of the issue. The six docs sites and
CLAUDE.md:261that teach the broken setter form are handled separately bybot-update-docs.yml, so this lands asRefsrather thanFixes.Refs #3079
Related Issue
Refs #3079
Type of Change
Feature Completeness Checklist
Signed-off-by:(committed withgit commit -s)vendor/wheels/tests/specs/model/miscellaneousSpec.cfcadds a setter-overload suite (failing → passing)bot-update-docs.ymlbot-update-docs.ymlbot-update-docs.yml(the Model Quick ReferencetableName("X")atCLAUDE.md:261is the contamination source)changelog.d/3079-tablename-setter.fixed.mdbash tools/test-local.sh model: before the fix894 passed, 2 failed; after the fix896 passed (0 failed, 0 errors)on Lucee 7 + SQLiteTest Plan
bash tools/test-local.sh modelmiscellaneousSpec.cfc("Tests that tableName acts as a setter when given a name - issue 3079") assert:tableName("tbl_authors_override")thentableName()returns the override (delegates totable())tableName()still returns the current nameExpected [tbl_authors_override] but received [c_o_r_e_authors], confirming the silent no-op; post-fix all pass with no regressions across the model layer.