Skip to content

Commit 833504f

Browse files
asheshvclaude
authored andcommitted
fix(schema-diff): aggregate child counts at parent group rows (pgadmin-org#10021)
In Tools > Schema Diff, the top-level group rows ("Schema Objects", "Database Objects") rendered blank difference counts even when their child object-type groups (Functions, Tables, Sequences, ...) listed Source-Only / Target-Only / Different objects, giving no signal at the parent level that anything differed. The results tree has three levels: top-level object-group rows, mid-level object-type rows, and leaf object rows carrying a status. setRecordCount() only handled leaf children, so mid-level rows counted correctly but top-level rows (whose children are mid-level rows with no status) stayed at zero. The render gate 'identicalCount' in row also skipped count rendering for top-level rows because generateGridData() never seeded those fields on them. Fix: setRecordCount() now recognises counted-group children (recursing to refresh their counts so filter changes propagate, then rolling the totals up); leaf-only behaviour is unchanged. generateGridData() seeds the four count fields on every top-level group row so the render gate fires. Adds Jest coverage driving the now-exported setRecordCount directly, including the reporter's exact tree and a filter-refresh case. Fixes pgadmin-org#9892. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 94a7687 commit 833504f

4 files changed

Lines changed: 183 additions & 2 deletions

File tree

docs/en_US/release_notes_9_16.rst

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,3 +27,5 @@ Housekeeping
2727
2828
Bug fixes
2929
*********
30+
31+
| `Issue #9892 <https://github.com/pgadmin-org/pgadmin4/issues/9892>`_ - Fix blank difference counts on the top-level group rows in Schema Diff.

web/pgadmin/tools/schema_diff/static/js/components/ResultGridComponent.jsx

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -159,14 +159,38 @@ function useFocusRef(isSelected) {
159159
};
160160
}
161161

162-
function setRecordCount(row, filterParams) {
162+
/*
163+
* Recompute the four status counts for ``row`` based on its descendants.
164+
*
165+
* Three row shapes can appear in the schema-diff results tree:
166+
* - Leaf row: has a ``status`` (Identical/Different/Source Only/Target
167+
* Only); contributes 1 to the matching count when it's
168+
* visible under ``filterParams``.
169+
* - Mid-level row (object-type group, e.g. "Functions"): children are
170+
* leaves. Counts are the sum of those leaves.
171+
* - Top-level row (object-group, e.g. "Schema Objects"): children are
172+
* mid-level rows. Counts are the sum of the mid-level rows' counts.
173+
*
174+
* Originally this function only handled the leaf-children case, which
175+
* left top-level group rows showing blank counts even when their
176+
* descendants had differences. Issue #9892.
177+
*/
178+
export function setRecordCount(row, filterParams) {
163179
row['identicalCount'] = 0;
164180
row['differentCount'] = 0;
165181
row['sourceOnlyCount'] = 0;
166182
row['targetOnlyCount'] = 0;
167183

168184
row.children.map((ch) => {
169-
if (filterParams.includes(ch.status)) {
185+
if ('identicalCount' in ch) {
186+
// Mid-level child: refresh its counts (in case filterParams
187+
// changed since the last render) and roll them up.
188+
setRecordCount(ch, filterParams);
189+
row['identicalCount'] += ch['identicalCount'];
190+
row['differentCount'] += ch['differentCount'];
191+
row['sourceOnlyCount'] += ch['sourceOnlyCount'];
192+
row['targetOnlyCount'] += ch['targetOnlyCount'];
193+
} else if (filterParams.includes(ch.status)) {
170194
if (ch.status == FILTER_NAME.IDENTICAL) {
171195
row['identicalCount'] = row['identicalCount'] + 1;
172196
} else if (ch.status == FILTER_NAME.DIFFERENT) {

web/pgadmin/tools/schema_diff/static/js/components/SchemaDiffCompare.jsx

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -495,6 +495,13 @@ export function SchemaDiffCompare({ params }) {
495495
'groupType': record.group_name,
496496
'isExpanded': false,
497497
'selected': false,
498+
// Seed status counts so the cell expander renders them at the
499+
// top-level group too — setRecordCount() will aggregate from
500+
// the mid-level children at render time. Issue #9892.
501+
'identicalCount': 0,
502+
'differentCount': 0,
503+
'sourceOnlyCount': 0,
504+
'targetOnlyCount': 0,
498505
'children': {}
499506
};
500507
let ch_id = record.id;
Lines changed: 148 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,148 @@
1+
/////////////////////////////////////////////////////////////
2+
//
3+
// pgAdmin 4 - PostgreSQL Tools
4+
//
5+
// Copyright (C) 2013 - 2026, The pgAdmin Development Team
6+
// This software is released under the PostgreSQL Licence
7+
//
8+
//////////////////////////////////////////////////////////////
9+
10+
import { setRecordCount } from '../../../pgadmin/tools/schema_diff/static/js/components/ResultGridComponent';
11+
import { FILTER_NAME } from '../../../pgadmin/tools/schema_diff/static/js/SchemaDiffConstants';
12+
13+
const ALL_FILTERS = [
14+
FILTER_NAME.IDENTICAL,
15+
FILTER_NAME.DIFFERENT,
16+
FILTER_NAME.SOURCE_ONLY,
17+
FILTER_NAME.TARGET_ONLY,
18+
];
19+
20+
function leaf(status) {
21+
return { status };
22+
}
23+
24+
function midGroup(label, children) {
25+
return {
26+
label,
27+
identicalCount: 0,
28+
differentCount: 0,
29+
sourceOnlyCount: 0,
30+
targetOnlyCount: 0,
31+
children,
32+
};
33+
}
34+
35+
function topGroup(label, children) {
36+
return {
37+
label,
38+
identicalCount: 0,
39+
differentCount: 0,
40+
sourceOnlyCount: 0,
41+
targetOnlyCount: 0,
42+
children,
43+
};
44+
}
45+
46+
describe('setRecordCount', () => {
47+
it('counts leaf children by status', () => {
48+
const row = midGroup('Functions', [
49+
leaf(FILTER_NAME.IDENTICAL),
50+
leaf(FILTER_NAME.DIFFERENT),
51+
leaf(FILTER_NAME.DIFFERENT),
52+
leaf(FILTER_NAME.SOURCE_ONLY),
53+
leaf(FILTER_NAME.SOURCE_ONLY),
54+
leaf(FILTER_NAME.SOURCE_ONLY),
55+
leaf(FILTER_NAME.TARGET_ONLY),
56+
]);
57+
58+
setRecordCount(row, ALL_FILTERS);
59+
60+
expect(row.identicalCount).toBe(1);
61+
expect(row.differentCount).toBe(2);
62+
expect(row.sourceOnlyCount).toBe(3);
63+
expect(row.targetOnlyCount).toBe(1);
64+
});
65+
66+
it('skips leaf children whose status is filtered out', () => {
67+
const row = midGroup('Functions', [
68+
leaf(FILTER_NAME.IDENTICAL),
69+
leaf(FILTER_NAME.DIFFERENT),
70+
leaf(FILTER_NAME.SOURCE_ONLY),
71+
leaf(FILTER_NAME.TARGET_ONLY),
72+
]);
73+
74+
// Only count DIFFERENT and SOURCE_ONLY.
75+
setRecordCount(row, [FILTER_NAME.DIFFERENT, FILTER_NAME.SOURCE_ONLY]);
76+
77+
expect(row.identicalCount).toBe(0);
78+
expect(row.differentCount).toBe(1);
79+
expect(row.sourceOnlyCount).toBe(1);
80+
expect(row.targetOnlyCount).toBe(0);
81+
});
82+
83+
it('aggregates mid-level child counts into the top-level group (issue #9892)', () => {
84+
// Tree mirroring the bug reporter's case: a top-level group whose
85+
// children are object-type groups (Functions, Tables, ...) that
86+
// each have leaf children with statuses. Before the fix the top
87+
// row stayed at 0/0/0/0.
88+
const top = topGroup('Schema Objects', [
89+
midGroup('Functions', [
90+
leaf(FILTER_NAME.SOURCE_ONLY),
91+
leaf(FILTER_NAME.SOURCE_ONLY),
92+
leaf(FILTER_NAME.SOURCE_ONLY),
93+
leaf(FILTER_NAME.SOURCE_ONLY),
94+
leaf(FILTER_NAME.SOURCE_ONLY),
95+
leaf(FILTER_NAME.SOURCE_ONLY),
96+
leaf(FILTER_NAME.SOURCE_ONLY),
97+
leaf(FILTER_NAME.TARGET_ONLY),
98+
]),
99+
midGroup('Tables', Array(6).fill(leaf(FILTER_NAME.SOURCE_ONLY))),
100+
midGroup('Procedures', Array(3).fill(leaf(FILTER_NAME.SOURCE_ONLY))),
101+
midGroup('Sequences', [
102+
leaf(FILTER_NAME.SOURCE_ONLY),
103+
leaf(FILTER_NAME.DIFFERENT),
104+
]),
105+
midGroup('Types', [leaf(FILTER_NAME.IDENTICAL)]),
106+
]);
107+
108+
setRecordCount(top, ALL_FILTERS);
109+
110+
// 7 (Functions src) + 6 (Tables) + 3 (Procedures) + 1 (Sequences) = 17
111+
expect(top.sourceOnlyCount).toBe(17);
112+
expect(top.targetOnlyCount).toBe(1);
113+
expect(top.differentCount).toBe(1);
114+
expect(top.identicalCount).toBe(1);
115+
});
116+
117+
it('refreshes mid-level child counts on every call (filter change)', () => {
118+
const mid = midGroup('Functions', [
119+
leaf(FILTER_NAME.IDENTICAL),
120+
leaf(FILTER_NAME.DIFFERENT),
121+
]);
122+
const top = topGroup('Schema Objects', [mid]);
123+
124+
// First pass: include both statuses → mid counts both.
125+
setRecordCount(top, ALL_FILTERS);
126+
expect(mid.identicalCount).toBe(1);
127+
expect(mid.differentCount).toBe(1);
128+
expect(top.identicalCount).toBe(1);
129+
expect(top.differentCount).toBe(1);
130+
131+
// Filter changes to DIFFERENT only → top must reflect mid's refreshed
132+
// counts, not the stale numbers from the first pass.
133+
setRecordCount(top, [FILTER_NAME.DIFFERENT]);
134+
expect(mid.identicalCount).toBe(0);
135+
expect(mid.differentCount).toBe(1);
136+
expect(top.identicalCount).toBe(0);
137+
expect(top.differentCount).toBe(1);
138+
});
139+
140+
it('handles an empty children list without throwing', () => {
141+
const row = midGroup('Empty', []);
142+
setRecordCount(row, ALL_FILTERS);
143+
expect(row.identicalCount).toBe(0);
144+
expect(row.differentCount).toBe(0);
145+
expect(row.sourceOnlyCount).toBe(0);
146+
expect(row.targetOnlyCount).toBe(0);
147+
});
148+
});

0 commit comments

Comments
 (0)