refactor(case): retire customer_rating, customer_feedback and the CSAT flow that fed them - #1699
Merged
Merged
Conversation
…T flow Both fields were writable, translated in four locales, and reachable from no screen: no form, no detail section, no list column, no filter. Nothing wrote them outside seed data, and neither was named in any profile's `fields` map — on this platform field permissions are built only from named fields, so an unnamed field is default-open. A satisfaction score typed by the person being scored is not the same fact as one the customer gave, and a CRM that cannot tell them apart reports a number it cannot defend. Retired under ADR-0049 enforce-or-remove per the maintainer ruling on #1428 (decision batch #21). `case_csat_followup` is retired rather than rewritten. Its only purpose was to notify the case owner, a day after close, to log a rating against a record page that had nowhere to take one. With the fields gone it feeds nothing, and no other purpose was measured to want it. Also removed: the four-locale label/help entries, the flow's row and the flow numeral in the built-in automation table (three doc locales), the CSAT knob and the satisfaction section of the packaged admin/service guides, and the flow's frontmatter source refs. The analytics pages now explain "CSAT by Agent" as having no satisfaction data at all rather than a field no measure aggregates. `crm_case` re-measured: `case_metrics` never declared a dimension or measure over either field, so `src/datasets/` was outside this change. The #1505 pin is reconciled: it asserts the columns are GONE rather than nulled. Measured while doing so — this engine refuses an undeclared write (`Unknown field 'customer_rating' on object 'crm_case'`, before hooks, sharing and security), so the plant path closes by construction. Co-authored-by: Claude
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
os-steve
marked this pull request as ready for review
September 6, 2026 11:29
This was referenced Sep 6, 2026
This was referenced Sep 6, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1428
Maintainer ruling, decision batch #21 (2026-09-03): Option A — retire
crm_case.customer_ratingandcustomer_feedbackunder ADR-0049enforce-or-remove, and retire or rewrite
case_csat_followupin the samechange. This is that change.
What this closes is a declared-≠-enforced hole, not two spare columns
Both fields were writable, translated in four locales, and reachable from no
screen in the product — no form, no detail section, no list column, no filter.
Nothing in
src/wrote them outside seed data. And neither field was named inany profile's
fieldsmap, which on this platform is not a neutral fact:field permissions are built only from the fields a permission set names, so an
unnamed field is default-open.
internal_notessits beside them and isnamed per profile —
service_agenteditable,sales_managerread-only,sales_repnot readable — which is what the two survey fields looked like theyhad and did not.
A satisfaction score typed by the person being scored is not the same fact as
one the customer gave, and a CRM that cannot tell the two apart reports a number
it cannot defend. There was no column that could record which kind a value was,
and no surface through which a customer could give one. Retiring is the only
move that makes the declared shape equal the enforced shape; the
customer-answered survey remains the sound long-term design and is not
precluded — it is a feature, and it would arrive as its own card.
The flow: retired, not rewritten
case_csat_followupfired a day after a case closed and notified the case ownerto "Reach out to the contact and log their satisfaction rating", with
actionUrl: '/crm_case/{record.id}'— a record page with no input for a rating.A shipped feedback loop with no landing point.
Rewriting it would have meant inventing a purpose. Its only data destination was
the two retired fields; nothing else in the app was measured to want a
post-close timer, and the maintainer ruled the survey out for now, so there was
no second job waiting to be given to it. A flow kept alive for its name is worse
than one removed. Measurement is on record and so is its cost: this was the
app's only flow with a
waitnode, so retiring it leaves the platform'stimer capability with no worked example in-tree. That is not a reason to keep a
flow that feeds nothing, and it is filed separately as #1697 rather than left to
be rediscovered.
Two claims conflicted; the measurement decides
The ruling says
case_metricsreporting reads these fields. The analytics prosethat PR #1683 just rewrote says
case_metricsdeclares no measure overcustomer_rating. The prose is right.src/datasets/case.dataset.tsdeclares six dimensions (status, priority, origin, type, created_date,
resolved_article) and eight measures, and not one of them names either field.
src/datasets/was therefore outside this change — the ruling's parentheticalwas carrying a claim from an earlier comment, not a reading of the tree.
Those six analytics pages were re-read from the freshest
origin/main, not fromany description. The sentence that concerned this card —
— would have become false the moment the field went. It now says the stronger
true thing: there is no satisfaction data at all, and still no agent dimension.
Same treatment for the matching bullet on the cubes page, in all three locales.
Reconciled with #1505, and a sharper mechanism than expected
#1505 closed the anonymous-intake hole by having the guest branch null both
fields. With the fields retired, that path closes by construction — but
"closed by construction" and "nulled" are different facts, and
toBeNull()reads green against both.
test/guest-submission-sanitisation.test.tsnowasserts the columns are gone, in both halves: absent from
crm_case'sdeclared field map (read off the compiled stack) and absent from the stored row,
with an anti-vacuum guard so an empty field map cannot make it pass by
describing nothing.
Measured while making that change, and it is why the two keys left the planted
payload rather than staying as a stronger plant: this engine refuses an
undeclared write outright. Re-running the case with them still in the document
failed the whole insert with
before the hooks, before sharing, before security. A submitter naming a retired
column no longer gets a case with the value dropped — they get no case. Keeping
the plant would have made that case assert the refusal instead of the
sanitisation it exists for, and taken its four other assertions down with it.
No change to #1505's own card scope was needed; it is closed and its pin lives
here.
The two epic-#1579 files — exactly what changed, and why each is entailed
The PM ruled that the epic fence does not block a ruled retirement: an assertion
disappearing because the thing it asserted was deleted is a different act
from the epic's (deleting an assertion because a platform rule covers it). Both
edits are one line, and both are the deleted flow's own name:
test/smoke.test.ts'case_csat_followup'from the record-change flow list on line 46test/flow-condition-totality.test.ts'case_csat_followup'from theboot([...])listboot()loads the named flows to drive their conditions; a name with no flow behind it loads nothingNothing else in either file was touched — no assertion weakened, no case
removed, no count changed.
git diffon the two files is two lines out, twolines in.
In-place boundary, declared
content/docs/service/cases.{mdx,zh-Hans,zh-Hant}name the two fields in threeplaces each. This PR subtracts only: the two names come out, and the
arithmetic that moved because of the subtraction moves with it (the object's
field total 27 to 25, the "eleven not on the Details tab" to nine, the form
table's own row sum). It does not re-derive the other numbers on that page,
because they are stale for reasons this card did not cause — PR #1427 narrowed
the form to one section of nine fields and PR #1506 added
internal_notestothe Details tab, and neither updated the page. Correcting that is a different
defect class and a whole table replaced in three locales, so it is filed as
#1696 with the measurement rather than ridden in here. The page is left
exactly as consistent, and exactly as stale, as it was found.
Untouched on purpose
CHANGELOG.mdand the four.changeset/*.mdfiles that mention the fields orthe flow — they record what a past change did ([finding] The 17.3.0 upgrade left 19 comments asserting 17.2.0 is "the pinned" version — third recurrence of #1460 / #1467, and #1669 names only 1 of the 13 files #1676's boundary).
docs/architecture/module-split-inventory.json— a generated snapshot stampedto commit
a6be39a3, whose own header says "Regenerate from the artifact;never hand-edit", and no regenerator exists in the repo. Filed as
module-split-inventory.jsonsays "regenerate, never hand-edit" but no generator is in the repo — it now lists a deleted flow file #1698.content/docs/ai-copilot/service-copilot.mdx— says the Customer 360 skillcarries no satisfaction roll-up, which names no field and is still true.
The surface, re-derived on
origin/main@d6c930e9git grep -lnfor the two fields found 21 files (17 source and docs, plus 4changesets); for the flow, 17 (15 plus
CHANGELOG.mdand 2 changesets) —two more than the dispatch's reading, which had not named
docs/feature-inventory.mdordocs/architecture/module-split-inventory.json.src/data/service.seed.tsis in neither list, confirmed. Beyond the identifiergreps, a label-level grep (
Customer Satisfaction/Customer Feedbackandtheir four locale spellings) found three more readers the identifier grep could
not see — the three
content/docs/service/casesfaces.docs/feature-inventory.mdkeeps itsSVC-011row and marks it removed, whichis that file's own documented rule for a retired capability rather than a choice
made here.
Verification
pnpm verifyfully green ond66dd468, the commit this PR carries, every heavyrun through the container's shared verify lock with the exit code captured
before any pipe:
pnpm changeset status --since=origin/mainexit 0 (hotcrm patch). No controlcharacters in any changed file (
grep -naPover the staged set, clean).Reverse verification of the reconciled pin, run from the committed state,
direction predicted before it was run: re-declare
customer_ratingoncrm_caseand the pin should go red naming the re-declaration, and thedocs/STATUS.mdfield-count rule should go red beside it. Both moved —Test Files 2 failed (2)— the pin withThe mutation was proved on disk before the run (injected-text count 0 then 1,
git diff --statnaming the file) and restored under anEXIT INT TERMtrapwith absolute paths via
git checkout HEAD -- path: restored blob8b47c7aa0f9b89ec76203f2db7fa9dfe88fb86fbidentical to the HEAD blob, andgit diff HEADempty. No build or dist step is involved — these suites import../srcand../objectstack.configdirectly, so no dist preflight applies.Upgrade note
Stored
customer_rating/customer_feedbackvalues stop being read or writtenby the app, and the engine now refuses writes naming either column. Nothing here
migrates them; the changeset says so for the release-notes reader.
Generated by Claude Code