fix(cli): generate api-resource resolves objectName placeholders - #2566
Conversation
Wheels Bot — No doc updatesReviewed this PR's diff and found no docs that need updating (purely internal bug fix — the snippet template at |
There was a problem hiding this comment.
Wheels Bot — Reviewer A
TL;DR: This is a minimal, targeted bug fix that aligns the framework's app/snippets/ApiControllerContent.txt — which takes precedence over the CLI-bundled templates via Templates.findTemplate() — with the already-correct pipe-delimited token format that Templates.processTemplate() understands. The root cause, the fix, and the regression spec are all correct. I verified all three template copies (framework snippet, cli/lucli/templates/, and cli/src/templates/) now use |ObjectNamePlural| / |ObjectNameSingular| consistently. Verdict: comment (one minor spec nit, no blocking issues).
Correctness
The fix is accurate. Templates.processTemplate() handles |ObjectNamePlural| / |ObjectNameSingular| at lines 172-176 of cli/lucli/services/Templates.cfc:
processed = replace(processed, "|ObjectNameSingular|", lCase(modelName), "all");
processed = replace(processed, "|ObjectNamePlural|", lCase(variables.helpers.pluralize(modelName)), "all");The hash-token form #objectNamePlural# is never substituted anywhere in the processor, leaving it verbatim in generated controllers exactly as issue #2468 reported. Replacing with pipe tokens eliminates the bug.
I verified cli/src/templates/ApiControllerContent.txt and cli/lucli/templates/app/app/snippets/ApiControllerContent.txt both already used pipe tokens; only the framework's own app/snippets/ copy lagged behind. The PR description's claim about root cause is accurate.
Tests
Minor nit — single it block bundles two independent assertions.
vendor/wheels/tests/specs/cli/ApiControllerSnippetSpec.cfc:17:
it("uses pipe-delimited tokens the CLI Templates processor understands", function() {
expect(content contains legacyPlural).toBeFalse(...);
expect(content contains legacySingular).toBeFalse(...);
expect(content contains "|ObjectNamePlural|").toBeTrue(...);
expect(content contains "|ObjectNameSingular|").toBeTrue(...);
});Bundling the absence-of-legacy and presence-of-pipe assertions in one it means that if the plural legacy token is fixed but the singular is still present, the failure message is the only signal pointing at which specific assertion fired. Splitting into two it blocks (one for absence, one for presence) would produce cleaner test output. Not a blocker.
The BDD structure (extends wheels.WheelsTest, describe/it with closures) is correct, and placing the spec under vendor/wheels/tests/specs/cli/ means it is auto-discovered by the core test runner. The ## concatenation trick to build the legacy token string without CFML interpreting it as an expression delimiter is correct:
var legacyPlural = "##" & "objectNamePlural" & "##"; // -> #objectNamePlural# at runtimeCommits
Single commit: fix(cli): generate api-resource resolves objectName placeholders
Type fix, scope cli (commitlint allowlist), subject lowercase, under 100 chars. No issues.
Docs
CHANGELOG entry under [Unreleased] -> Fixed is present and accurate. No .ai/wheels/ doc changes are needed — this is a data file fix, not a behavioral API change.
Wheels Bot — Reviewer B (round 1)A's review is accurate and well-reasoned. The root-cause analysis, the fix verification, and the spec assessment are all correct. One trivial line-number inaccuracy noted below, but nothing that changes the substance. Converging on approve — the single nit A raised is explicitly non-blocking and the fix itself is clearly correct. SycophancyNone detected. A gave a False positivesNone detected, with one cosmetic note:
Missed issuesNone detected. Re-scanning the diff:
Verdict alignmentA's ConvergenceAligned. A and B both find the fix correct and the only finding non-blocking. No changes are needed before merge. |
Summary
wheels generate api-resourcewas emitting controllers containing literal#objectNamePlural#and#objectNameSingular#placeholders. The framework-level snippet atapp/snippets/ApiControllerContent.txt(which takes precedence over the CLI-bundled template viaTemplates.findTemplate()) still used the legacy hash-token form. The CLI'sTemplates.processTemplate()only substitutes pipe-delimited tokens (|ObjectNameSingular|,|ObjectNamePlural|), so the hash tokens passed through untouched and landed verbatim in the generated controller. Aligning the framework snippet with the already-corrected CLI-bundled copy fixes the bug.Related Issue
Closes #2468
Type of Change
Feature Completeness Checklist
vendor/wheels/tests/specs/cli/ApiControllerSnippetSpec.cfcreproduces the bug (legacy tokens present) and locks in the fix (pipe tokens present). Verified failing-then-passing locally.bot-update-docs.ymlif neededbot-update-docs.ymlif neededbot-update-docs.ymlif needed[Unreleased] -> Fixedcurl /wheels/core/tests: 3430 pass, 0 fail, 0 error, 16 skipped across 193 bundlesTest Plan
#objectNamePlural#/#objectNameSingular#tokens inapp/snippets/ApiControllerContent.txt, the new spec fails withSnippet still contains legacy token #objectNamePlural# -- CLI Templates.processTemplate() doesn't substitute this form.|ObjectNamePlural|/|ObjectNameSingular|, the spec passes.wheels generate api-resource Product name price:decimal sku:stringnow emits a controller withlocal.products = model("product").findAll();instead of unresolved placeholders.Screenshots / Output