Skip to content

Commit 880ace3

Browse files
authored
Merge branch 'development' into fix/development-cannot-npm-install
2 parents 2f939a4 + 589ac7a commit 880ace3

4 files changed

Lines changed: 234 additions & 57 deletions

File tree

lib/Settings/register.d/bookkeeping-cost-centers-dimensions.json

Lines changed: 49 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -276,50 +276,64 @@
276276
},
277277
"x-openregister-aggregations": {
278278
"segmentPnl": {
279-
"description": "Segment P&L for this dimension (cost-center rows only): sum of GLLine amounts whose costCenterCode equals this record's code, including child cost-centers rolled up via parentCode per REQ-CC-005. Consumes GLLine.costCenterCode = @self.code with eliminationFlag = false. Scoped dimensionType = cost-center.",
280-
"join": {
281-
"schema": "GLLine",
282-
"condition": "GLLine.costCenterCode = @self.code AND GLLine.eliminationFlag = false"
279+
"description": "Segment P&L per cost-center (REQ-CC-005): debit/credit sums of non-eliminated GLLine amounts, grouped by costCenterCode + periodId + accountNumber. Returns EVERY cost-center; narrow to one with the extraFilter query params (?costCenterCode=...). It does not correlate on @self — no caller supplies a parent row. Parent roll-up over the parentCode tree is NOT included here; byCostCenterHierarchy does that through the join.",
280+
"from": "GLLine",
281+
"where": {
282+
"eliminationFlag": false
283283
},
284284
"groupBy": [
285+
"costCenterCode",
285286
"GLLine.fiscalYearId",
286-
"GLLine.accountNumber"
287+
"accountNumber"
287288
],
288-
"operations": {
289-
"totalDebit": {
290-
"field": "GLLine.amount",
291-
"operation": "sum",
292-
"condition": "GLLine.side = 'debit'"
289+
"metrics": [
290+
{
291+
"metric": "sum",
292+
"field": "amount",
293+
"condition": {
294+
"side": "debit"
295+
},
296+
"as": "totalDebit"
293297
},
294-
"totalCredit": {
295-
"field": "GLLine.amount",
296-
"operation": "sum",
297-
"condition": "GLLine.side = 'credit'"
298+
{
299+
"metric": "sum",
300+
"field": "amount",
301+
"condition": {
302+
"side": "credit"
303+
},
304+
"as": "totalCredit"
298305
}
299-
}
306+
]
300307
},
301308
"segmentPnlByCostObject": {
302-
"description": "Segment P&L for this dimension (cost-object rows only): sum of GLLine amounts whose kostenDragerCode equals this record's code per REQ-CC-005. Replaces the retired KostenDrager.segmentPnl. Scoped dimensionType = cost-object.",
303-
"join": {
304-
"schema": "GLLine",
305-
"condition": "GLLine.kostenDragerCode = @self.code AND GLLine.eliminationFlag = false"
309+
"description": "Segment P&L per cost-object (REQ-CC-005): debit/credit sums of non-eliminated GLLine amounts, grouped by costCarrierCode + periodId + accountNumber. Returns EVERY cost-object; narrow to one with the extraFilter query params (?costCarrierCode=...). Replaces the retired KostenDrager.segmentPnl; the field is costCarrierCode — kostenDragerCode is not declared on GLLine and matched nothing.",
310+
"from": "GLLine",
311+
"where": {
312+
"eliminationFlag": false
306313
},
307314
"groupBy": [
315+
"costCarrierCode",
308316
"GLLine.fiscalYearId",
309-
"GLLine.accountNumber"
317+
"accountNumber"
310318
],
311-
"operations": {
312-
"totalDebit": {
313-
"field": "GLLine.amount",
314-
"operation": "sum",
315-
"condition": "GLLine.side = 'debit'"
319+
"metrics": [
320+
{
321+
"metric": "sum",
322+
"field": "amount",
323+
"condition": {
324+
"side": "debit"
325+
},
326+
"as": "totalDebit"
316327
},
317-
"totalCredit": {
318-
"field": "GLLine.amount",
319-
"operation": "sum",
320-
"condition": "GLLine.side = 'credit'"
328+
{
329+
"metric": "sum",
330+
"field": "amount",
331+
"condition": {
332+
"side": "credit"
333+
},
334+
"as": "totalCredit"
321335
}
322-
}
336+
]
323337
},
324338
"spentToDate": {
325339
"description": "Sum of GL debit postings tagged to this cost-center dimension (recursive over descendant cost-centers via parentCode self-relation) per REQ-CPA-106. Excludes eliminated intercompany lines. Scoped dimensionType = cost-center.",
@@ -390,14 +404,17 @@
390404
},
391405
"byCostCenterHierarchy": {
392406
"description": "Hierarchical roll-up of GL amounts per AnalyticalDimension[dimensionType=cost-center].parentCode tree per REQ-CD-007. Child cost-center sums propagate to parents through the self-relation; drives drill-down navigation. Declarative (ADR-031). Re-targeted from CostCenter to AnalyticalDimension per REQ-ADIM-101.",
393-
"source": "GLLine",
394407
"groupBy": [
395408
"AnalyticalDimension.parentCode"
396409
],
397410
"filter": {},
411+
"metric": "sum",
412+
"field": "amount",
398413
"join": {
399414
"through": "AnalyticalDimension",
400-
"on": "AnalyticalDimension.code",
415+
"on": {
416+
"costCenterCode": "code"
417+
},
401418
"filter": {
402419
"dimensionType": "cost-center"
403420
},

lib/Settings/shillinq_register.json

Lines changed: 22 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -5509,27 +5509,34 @@
55095509
},
55105510
"x-openregister-aggregations": {
55115511
"segmentPnl": {
5512-
"description": "Segment P&L for this project: sum of GLLine amounts whose projectCode equals this record's code per REQ-CC-005. Shape is sufficient for WBSO time-per-project join per REQ-CC-007.",
5513-
"join": {
5514-
"schema": "GLLine",
5515-
"condition": "GLLine.projectCode = @self.code AND GLLine.eliminationFlag = false"
5512+
"description": "Segment P&L per project (REQ-CC-005): debit/credit sums of non-eliminated GLLine amounts, grouped by projectCode + periodId + accountNumber. Returns EVERY project; narrow to one with the extraFilter query params (?projectCode=...), which is what the REST endpoint passes through. It does not correlate on @self: no caller supplies a parent row, and an unanswerable @self reference now raises rather than filtering on null. Shape is sufficient for the WBSO time-per-project join per REQ-CC-007.",
5513+
"from": "GLLine",
5514+
"where": {
5515+
"eliminationFlag": false
55165516
},
55175517
"groupBy": [
5518+
"projectCode",
55185519
"GLLine.fiscalYearId",
5519-
"GLLine.accountNumber"
5520+
"accountNumber"
55205521
],
5521-
"operations": {
5522-
"totalDebit": {
5523-
"field": "GLLine.amount",
5524-
"operation": "sum",
5525-
"condition": "GLLine.side = 'debit'"
5522+
"metrics": [
5523+
{
5524+
"metric": "sum",
5525+
"field": "amount",
5526+
"condition": {
5527+
"side": "debit"
5528+
},
5529+
"as": "totalDebit"
55265530
},
5527-
"totalCredit": {
5528-
"field": "GLLine.amount",
5529-
"operation": "sum",
5530-
"condition": "GLLine.side = 'credit'"
5531+
{
5532+
"metric": "sum",
5533+
"field": "amount",
5534+
"condition": {
5535+
"side": "credit"
5536+
},
5537+
"as": "totalCredit"
55315538
}
5532-
}
5539+
]
55335540
},
55345541
"costsIncurredToDate": {
55355542
"description": "Sum of GL expense-account postings tagged to this project via GLLine.subLedgerRef (REQ-CPA-006).",

tests/Unit/Settings/CostCentersDimensionsFragmentTest.php

Lines changed: 33 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -142,14 +142,39 @@ public function testGlLineCarriesSegmentPnlAggregations(): void {
142142
self::assertArrayNotHasKey('sum', $aggs[$key], '`sum` is not an engine key; use metric+field');
143143
}
144144

145-
// byCostCenterHierarchy and byAnalyticalDimension are NOT translated and
146-
// still compute nothing. They need engine features rather than a
147-
// rewrite: the first groups by `AnalyticalDimension.parentCode`, a field
148-
// on the JOINED schema, and the second by the wildcard `dimensions.*`.
149-
// Tracked in #1261; pinned here so the gap stays visible.
150-
foreach (['byCostCenterHierarchy', 'byAnalyticalDimension'] as $key) {
151-
self::assertArrayNotHasKey('metric', $aggs[$key], 'still untranslated — see #1261');
152-
}
145+
// byCostCenterHierarchy groups by `AnalyticalDimension.parentCode` — a
146+
// field that exists only on the JOINED schema. applyJoin() runs AFTER
147+
// grouping, so this used to group on a column every row lacks, giving
148+
// one null bucket holding everything: a plausible total, not an error.
149+
// OpenRegister #2916 projects joined group fields onto the rows first,
150+
// which is what makes the declaration computable.
151+
self::assertSame('sum', $aggs['byCostCenterHierarchy']['metric']);
152+
self::assertSame('amount', $aggs['byCostCenterHierarchy']['field']);
153+
self::assertArrayNotHasKey(
154+
'source',
155+
$aggs['byCostCenterHierarchy'],
156+
'`source` is not an engine key'
157+
);
158+
159+
// The `on` SHORTHAND ("AnalyticalDimension.code") is refused by the
160+
// joined-field grouping path: it names the joined side only, leaving the
161+
// parent key to be inferred — and inferring the JOINED field produced a
162+
// single '' bucket rather than one per region. The explicit map states
163+
// both sides: parent GLLine.costCenterCode -> joined AnalyticalDimension.code.
164+
self::assertSame(
165+
['costCenterCode' => 'code'],
166+
$aggs['byCostCenterHierarchy']['join']['on'],
167+
'`on` MUST be an explicit parent-field => joined-field map'
168+
);
169+
170+
// byAnalyticalDimension is still untranslated: it groups by the wildcard
171+
// `dimensions.*`, which no engine key expresses. Tracked in #1261 and
172+
// pinned here so the gap stays visible.
173+
self::assertArrayNotHasKey(
174+
'metric',
175+
$aggs['byAnalyticalDimension'],
176+
'still untranslated — wildcard groupBy, see #1261'
177+
);
153178

154179
// After the ADIM merge (REQ-ADIM-101), byCostCenter joins through the unified
155180
// AnalyticalDimension schema (filtered by dimensionType=cost-center) rather than

tests/validate-registers.js

Lines changed: 130 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -590,7 +590,7 @@ const AGGREGATION_REF_BASELINE = new Map([
590590
// and NOT a `from`, which would have switched the runner into its cross-schema
591591
// path — plus `sum: ["amount"]`, which is not an engine key. Verified live
592592
// against the rows, not just for a non-empty response.
593-
const AGG_NO_METRIC_BASELINE = 216
593+
const AGG_NO_METRIC_BASELINE = 211
594594

595595
// A STRING `groupBy` is silently ignored, and the result is a WRONG NUMBER.
596596
//
@@ -714,7 +714,7 @@ const AGG_PLACEHOLDER_TENANT_KEYS = new Set(['administrationId', 'organisationId
714714
// Measured 2026-08-26 by this check, after removing 67 tenant placeholders
715715
// across 22 files. Counted BY THE GATE, not by a one-off script — an earlier
716716
// estimate of 73 came from a narrower hand-written predicate and was wrong.
717-
const AGG_PLACEHOLDER_BASELINE = 84
717+
const AGG_PLACEHOLDER_BASELINE = 81
718718

719719
function collectPlaceholders(node, path, out) {
720720
if (node === null || node === undefined) return
@@ -739,6 +739,22 @@ function checkAggregationPlaceholders(registry) {
739739

740740
for (const slug of Object.keys(registry).sort()) {
741741
for (const { aggName, agg, file } of registry[slug].aggregations) {
742+
// `where` IS scanned on a cross-schema spec too.
743+
//
744+
// This check briefly exempted it, on the reasoning that
745+
// runCrossSchema() substitutes `@self.<field>` against the parent
746+
// row and so the spelling works there. It resolves it, but nothing
747+
// SUPPLIES a parent row: AggregationController, ReportRenderService
748+
// and ThresholdEvaluationService all call AggregationRunner::run()
749+
// without one, so `@self.code` became null — and a null is applied
750+
// as a real filter VALUE, returning the target rows whose own field
751+
// is null. For a segment P&L that is the unassigned total, handed
752+
// back confidently for every parent record.
753+
//
754+
// OpenRegister now raises on an unresolvable `@self` rather than
755+
// filtering on null, which makes these declarations loudly broken
756+
// instead of quietly wrong. Either way they are defects, so the
757+
// exemption is gone.
742758
const found = []
743759
for (const key of ['filter', 'where', 'join', 'match']) {
744760
collectPlaceholders(agg[key], key, found)
@@ -801,6 +817,116 @@ function checkAggregationPlaceholders(registry) {
801817
return ok
802818
}
803819

820+
// The BARE half of the same subject.
821+
//
822+
// checkAggregationFieldRefs() validates only `Schema.field` refs — it opens
823+
// with `if (dot === -1) continue`, so a bare `fiscalYearId` was never looked
824+
// at. That is the larger half: a bare groupBy on a property the target schema
825+
// does not declare groups every row into ONE null bucket, and a bare filter
826+
// key that resolves to nothing narrows to zero rows. Both answer HTTP 200 with
827+
// a plausible figure, which is why none of them ever surfaced as a bug.
828+
//
829+
// The bare form was skipped because "the source schema would need resolving,
830+
// which source/sourceSchema spell inconsistently". The engine settles it:
831+
// AggregationRunner reads `from` and nothing else — `source` and
832+
// `sourceSchema` are inert keys it never consults. So the target is `from`
833+
// when present and the declaring schema otherwise, exactly as the runner
834+
// computes it, and the ambiguity that justified skipping this is gone.
835+
// 120 of the 451 bare references checked resolve to nothing today. They are
836+
// NOT waived — each returns a plausible figure (one null bucket, or zero rows)
837+
// under HTTP 200, which is why the class went unnoticed. The ratchet keeps the
838+
// number falling and refuses any new one. Classified in #1261; the bulk are
839+
// declarations carrying the inert `source` key that MEANT another schema and
840+
// therefore resolve their fields against the declaring schema instead.
841+
const AGG_BARE_REF_BASELINE = 120
842+
843+
function checkAggregationBareRefs(registry) {
844+
const offenders = []
845+
let checked = 0
846+
847+
for (const slug of Object.keys(registry).sort()) {
848+
for (const { aggName, agg, file } of registry[slug].aggregations) {
849+
// Resolve the target the way AggregationRunner does.
850+
const targetSlug =
851+
typeof agg.from === 'string' && agg.from !== '' ? agg.from : slug
852+
const target = registry[targetSlug]
853+
// An unresolvable target is a different defect class — and may
854+
// legitimately live in another app's register.
855+
if (!target) continue
856+
857+
const refs = []
858+
const gb = Array.isArray(agg.groupBy)
859+
? agg.groupBy
860+
: typeof agg.groupBy === 'string'
861+
? [agg.groupBy]
862+
: []
863+
for (const g of gb)
864+
if (typeof g === 'string')
865+
refs.push({ key: 'groupBy', ref: g.trim() })
866+
for (const src of ['filter', 'where']) {
867+
const v = agg[src]
868+
if (v && typeof v === 'object' && Array.isArray(v) === false)
869+
for (const k of Object.keys(v))
870+
refs.push({ key: src, ref: k.trim() })
871+
}
872+
if (typeof agg.field === 'string' && agg.field !== '')
873+
refs.push({ key: 'field', ref: agg.field.trim() })
874+
if (Array.isArray(agg.metrics))
875+
for (const m of agg.metrics)
876+
if (m && typeof m.field === 'string' && m.field !== '')
877+
refs.push({ key: 'metrics[].field', ref: m.field.trim() })
878+
879+
for (const { key, ref } of refs) {
880+
// Dotted refs are checkAggregationFieldRefs()' subject.
881+
if (ref.includes('.') === true) continue
882+
// `_`-prefixed keys are OpenRegister control params, not
883+
// properties; `@`-prefixed values are placeholders.
884+
if (ref.startsWith('_') === true || ref.startsWith('@') === true)
885+
continue
886+
if (IMPLICIT_OBJECT_FIELDS.has(ref) === true) continue
887+
checked++
888+
if (target.props.has(ref) === false)
889+
offenders.push(
890+
`${slug}.${aggName} ${key}="${ref}" — ${targetSlug} declares no such property`
891+
+ `\n declared in ${file}`,
892+
)
893+
}
894+
}
895+
}
896+
897+
console.log(
898+
`[validate-registers] aggregation BARE field references checked: ${checked}`,
899+
)
900+
// A check that examined nothing must not report success.
901+
if (checked === 0) {
902+
console.error(
903+
'[validate-registers] FAIL — the bare aggregation reference check resolved ZERO references. '
904+
+ 'That means it stopped seeing its own subject, not that the registers are clean.',
905+
)
906+
return false
907+
}
908+
console.log(
909+
`[validate-registers] aggregation bare refs that resolve to nothing: ${offenders.length} `
910+
+ `(baseline ${AGG_BARE_REF_BASELINE}) — see #1261`,
911+
)
912+
if (offenders.length > AGG_BARE_REF_BASELINE) {
913+
console.error(
914+
'[validate-registers] FAIL — bare aggregation references that cannot resolve: '
915+
+ `${offenders.length}, above the baseline of ${AGG_BARE_REF_BASELINE}.`,
916+
)
917+
for (const o of offenders) console.error(` - ${o}`)
918+
return false
919+
}
920+
if (offenders.length < AGG_BARE_REF_BASELINE) {
921+
console.error(
922+
`[validate-registers] ${AGG_BARE_REF_BASELINE - offenders.length} better than baseline — `
923+
+ `please lower AGG_BARE_REF_BASELINE to ${offenders.length}.`,
924+
)
925+
return false
926+
}
927+
return true
928+
}
929+
804930
function checkAggregationFieldRefs(registry) {
805931
const problems = []
806932
const baselined = []
@@ -916,6 +1042,7 @@ function main() {
9161042
const slugsOk = checkSlugCaseCollisions(registry)
9171043
const sameSlugFullDefinitionOk = checkSameSlugFullDefinitionCollisions(registry)
9181044
const aggregationRefsOk = checkAggregationFieldRefs(registry)
1045+
const aggregationBareRefsOk = checkAggregationBareRefs(registry)
9191046
const aggregationPlaceholdersOk = checkAggregationPlaceholders(registry)
9201047
const aggregationMetricsOk = checkAggregationMetrics(registry)
9211048
const aggregationGroupByOk = checkAggregationGroupByShape(registry)
@@ -925,6 +1052,7 @@ function main() {
9251052
slugsOk === false
9261053
|| sameSlugFullDefinitionOk === false
9271054
|| aggregationRefsOk === false
1055+
|| aggregationBareRefsOk === false
9281056
|| aggregationPlaceholdersOk === false
9291057
|| aggregationMetricsOk === false
9301058
|| aggregationGroupByOk === false

0 commit comments

Comments
 (0)