Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions .changeset/nested-conditional-validation-refs.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
'@objectstack/lint': patch
---

Fix `validate-translation-references` reporting a nested `conditional` validation branch's legitimate `_validations` bundle entry as an orphan `translation-target-unknown`, with inverted advice.

The rule built its `_validations` universe with a flat walk of `objects[].validations[]`. A `conditional` rule's `then` / `otherwise` branch is itself a full rule carrying its own `name`, and that branch name — not the wrapper's — is the address `checkConditional` delegates to and `authoredRuleMessage` keys on at runtime (`packages/objectql/src/validation/rule-validator.ts`). The flat walk never saw a branch name, so a correct bundle entry for one was flagged as an orphan, and the finding's own text ("keeps its source locale in every refusal") was the opposite of the truth for that key — acting on the advice (deleting the entry) reintroduced the exact defect it fixed.

The walk now descends into `then` / `otherwise`, mirroring `evaluateRule`'s recursion (a branch may itself be a nested `conditional`, so depth is unbounded). The wrapper's own name stays in the universe, unchanged: its message is structurally unreachable at runtime, but a bundle entry for it is deliberately kept elsewhere so the bundle mirrors the declared rule set 1:1.
114 changes: 114 additions & 0 deletions packages/lint/src/validate-translation-references.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,120 @@ describe('validateTranslationReferences — orphan keys', () => {
});
});

describe('validateTranslationReferences — nested conditional validation branches (#14700)', () => {
// Mirrors the card's own fixture: `demo_account` with one `conditional`
// rule whose `then` / `otherwise` are each a full, named rule.
const conditionalStack = (validations: unknown[], objectNode: Record<string, unknown>) => ({
objects: [
{
name: 'demo_account',
label: 'Account',
fields: {
status: { type: 'select', label: 'Status' },
churn_reason: { type: 'text', label: 'Churn reason' },
},
validations,
},
],
translations: [{ 'zh-CN': { objects: { demo_account: objectNode } } }],
});

const churnConsistencyRule = {
type: 'conditional',
name: 'churn_reason_consistency',
message: 'Churn reason must match the account state.',
when: "record.status == 'churned'",
then: {
type: 'script',
name: 'churn_reason_present',
message: 'A churned account needs a churn reason.',
condition: 'record.churn_reason == null',
},
otherwise: {
type: 'script',
name: 'churn_reason_absent',
message: 'A non-churned account must not carry a churn reason.',
condition: 'record.churn_reason != null',
},
};

it('accepts bundle entries for both branch names and the wrapper name at once', () => {
// Before the fix, this reported `translation-target-unknown` on BOTH
// branch keys — the card's own measurement — because the flat walk over
// `obj.validations` never descended into `then` / `otherwise`, even
// though `checkConditional` / `authoredRuleMessage` address the branch by
// exactly this name at runtime.
const findings = validateTranslationReferences(
conditionalStack([churnConsistencyRule], {
_validations: {
// The wrapper's own message is never rendered by `checkConditional`
// (#14518 keeps a bundle entry for it anyway, deliberately, so the
// bundle mirrors the declared rule set 1:1).
churn_reason_consistency: { message: 'Wrapper message (never rendered)' },
churn_reason_present: { message: '流失账户需要填写流失原因。' },
churn_reason_absent: { message: '未流失账户不应填写流失原因。' },
},
}),
);
expect(findings).toEqual([]);
});

it('still flags a bundle entry naming no rule at any depth — real orphans stay caught', () => {
const findings = validateTranslationReferences(
conditionalStack([churnConsistencyRule], {
_validations: {
churn_reason_present: { message: '流失账户需要填写流失原因。' },
churn_reason_absent: { message: '未流失账户不应填写流失原因。' },
churn_reason_ghost: { message: 'Nothing declares this.' },
},
}),
);
expect(findings.map((f) => f.path)).toEqual([
'translations[0]["zh-CN"].objects.demo_account._validations.churn_reason_ghost',
]);
expect(findings[0].rule).toBe(TRANSLATION_TARGET_UNKNOWN);
});

it('descends through a branch that is itself a nested conditional', () => {
const outerGate = {
type: 'conditional',
name: 'outer_gate',
message: 'outer',
when: "record.status == 'churned'",
then: {
type: 'conditional',
name: 'inner_gate',
message: 'inner',
when: 'record.churn_reason != null',
then: {
type: 'script',
name: 'innermost_rule',
message: 'deepest branch of all',
condition: 'true',
},
},
};
const findings = validateTranslationReferences(
conditionalStack([outerGate], { _validations: { innermost_rule: { message: '最深层的分支。' } } }),
);
expect(findings).toEqual([]);
});

it('skips an unnamed branch, same as an unnamed top-level rule already was (#14253)', () => {
const gateWithUnnamedBranch = {
type: 'conditional',
name: 'gate',
message: 'gate',
when: "record.status == 'churned'",
then: { type: 'script', message: 'has no name', condition: 'true' },
};
const findings = validateTranslationReferences(
conditionalStack([gateWithUnnamedBranch], { _validations: { gate: { message: '门。' } } }),
);
expect(findings).toEqual([]);
});
});

describe('validateTranslationReferences — option keys', () => {
it('flags an option key that is a near-miss of the stored value', () => {
// The HotCRM instance: `direct-mail` for the value `direct_mail`.
Expand Down
38 changes: 33 additions & 5 deletions packages/lint/src/validate-translation-references.ts
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,21 @@ function strName(v: unknown): string | undefined {
return typeof v === 'string' && v.length > 0 ? v : undefined;
}

/**
* Add a validation rule's own `name` to the universe, then recurse into a
* `conditional` rule's `then` / `otherwise` branch — each branch is itself a
* full rule (with its own `name`, and possibly its own nested branches), and
* mirrors the recursion `evaluateRule` performs at runtime (#14700). See the
* call site in {@link buildUniverse} for why the wrapper's own name is kept
* too, even though `then` / `otherwise` are what a caller actually reads.
*/
function collectValidationRuleNames(rule: AnyRec, validations: Set<string>): void {
const ruleName = strName(rule.name);
if (ruleName) validations.add(ruleName);
if (isRec(rule.then)) collectValidationRuleNames(rule.then, validations);
if (isRec(rule.otherwise)) collectValidationRuleNames(rule.otherwise, validations);
}

/**
* "Did you mean?" over the known names — a namespace pass the shared helper
* cannot see, falling back to `suggestName`'s containment/edit-distance
Expand Down Expand Up @@ -209,9 +224,12 @@ interface ObjectFacts {
sections: Set<string>;
/**
* `_validations` names — the custom validation rules this object declares
* (`objects[].validations[].name`). `objects.<obj>._validations.<rule>.message`
* (#14253) is keyed by that name, so a ghost here is a rule message that
* renders in the source locale inside an otherwise translated refusal.
* (`objects[].validations[].name`, including a nested `conditional` rule's
* `then` / `otherwise` branch — the branch is itself a full rule and is the
* address `checkConditional` / `authoredRuleMessage` actually key on;
* #14700). `objects.<obj>._validations.<rule>.message` (#14253) is keyed by
* that name, so a ghost here is a rule message that renders in the source
* locale inside an otherwise translated refusal.
*/
validations: Set<string>;
/**
Expand Down Expand Up @@ -605,9 +623,19 @@ function buildUniverse(stack: AnyRec): Universe {
// #14253: `_validations.<rule>` is keyed by the rule's own `name`. A rule
// without a name has no key and is not registered — the resolver cannot
// address it either, so nothing is lost by skipping it here.
// #14700: a `conditional` rule's `then` / `otherwise` branch is itself a
// full rule, and its `name` — not the wrapper's — is the address
// `checkConditional` delegates to and `authoredRuleMessage` keys on (see
// `packages/objectql/src/validation/rule-validator.ts`). A flat walk over
// `obj.validations` never sees a branch name, so a legitimate bundle
// entry for one was reported as an orphan. Descend into `then` /
// `otherwise`, mirroring `evaluateRule`'s recursion (a branch may itself
// be a nested `conditional`, so depth is unbounded); the wrapper's own
// name stays in the universe too — its message is structurally
// unreachable at runtime, but #14518 keeps a bundle entry for it
// deliberately so the bundle mirrors the declared rule set 1:1.
for (const rule of asArray(obj.validations)) {
const ruleName = strName(rule.name);
if (ruleName) facts.validations.add(ruleName);
collectValidationRuleNames(rule, facts.validations);
}
}

Expand Down
Loading