From 6e9403ef19ea0212e757bfd3381475006b0f0b5e Mon Sep 17 00:00:00 2001 From: Ashesh Vashi Date: Sun, 7 Jun 2026 22:07:32 +0530 Subject: [PATCH] fix(schema-diff): aggregate child counts at parent group rows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Schema Diff results tree shows status counts (Identical / Different / Source Only / Target Only) on each mid-level row (object-type group such as "Functions", "Tables"). The top-level group rows ("Schema Objects", "Database Objects") were rendered without those counts, so a user could see "Schema Objects" sit at blank/zero while its child groups had non-trivial mismatches hidden below it. Issue #9892. Two changes: - ResultGridComponent.jsx: ``setRecordCount`` now handles a third child shape — a child that is itself a counted group (has ``identicalCount`` and friends) rather than a leaf with a ``status``. For those children it recurses to refresh the child's counts (in case ``filterParams`` changed since the last render) and rolls the four numbers up. Leaf-only behavior is unchanged. Exported the function so the regression test can drive it directly. - SchemaDiffCompare.jsx: seed ``identicalCount`` / ``differentCount`` / ``sourceOnlyCount`` / ``targetOnlyCount`` to 0 on every top-level group row when building the tree, so the existing ``'identicalCount' in row`` render gate in ``CellExpanderFormatter`` passes and the counts are drawn. Adds regression/javascript/schema_diff/result_grid_count_spec.js with five scenarios: - Leaf children counted by status (sanity, pre-fix behavior). - Leaf children outside ``filterParams`` skipped (sanity). - Mid-level children's counts aggregated into the top-level group — would have shown 0 across the board before the fix and now correctly sums to the reporter's example numbers. - Re-running with a narrower ``filterParams`` refreshes the mid-level counts (no stale numbers leak from a previous render). - Empty children list is a no-op. Verified: - yarn run eslint clean on changed files. - yarn run jest -t "setRecordCount": 5/5 new tests pass. - yarn run test:js-once: 829/829 pass (was 824/824). Fixes #9892 --- .../js/components/ResultGridComponent.jsx | 28 +++- .../js/components/SchemaDiffCompare.jsx | 7 + .../schema_diff/result_grid_count_spec.js | 148 ++++++++++++++++++ 3 files changed, 181 insertions(+), 2 deletions(-) create mode 100644 web/regression/javascript/schema_diff/result_grid_count_spec.js diff --git a/web/pgadmin/tools/schema_diff/static/js/components/ResultGridComponent.jsx b/web/pgadmin/tools/schema_diff/static/js/components/ResultGridComponent.jsx index 906bd05cc57..33f1569288e 100644 --- a/web/pgadmin/tools/schema_diff/static/js/components/ResultGridComponent.jsx +++ b/web/pgadmin/tools/schema_diff/static/js/components/ResultGridComponent.jsx @@ -159,14 +159,38 @@ function useFocusRef(isSelected) { }; } -function setRecordCount(row, filterParams) { +/* + * Recompute the four status counts for ``row`` based on its descendants. + * + * Three row shapes can appear in the schema-diff results tree: + * - Leaf row: has a ``status`` (Identical/Different/Source Only/Target + * Only); contributes 1 to the matching count when it's + * visible under ``filterParams``. + * - Mid-level row (object-type group, e.g. "Functions"): children are + * leaves. Counts are the sum of those leaves. + * - Top-level row (object-group, e.g. "Schema Objects"): children are + * mid-level rows. Counts are the sum of the mid-level rows' counts. + * + * Originally this function only handled the leaf-children case, which + * left top-level group rows showing blank counts even when their + * descendants had differences. Issue #9892. + */ +export function setRecordCount(row, filterParams) { row['identicalCount'] = 0; row['differentCount'] = 0; row['sourceOnlyCount'] = 0; row['targetOnlyCount'] = 0; row.children.map((ch) => { - if (filterParams.includes(ch.status)) { + if ('identicalCount' in ch) { + // Mid-level child: refresh its counts (in case filterParams + // changed since the last render) and roll them up. + setRecordCount(ch, filterParams); + row['identicalCount'] += ch['identicalCount']; + row['differentCount'] += ch['differentCount']; + row['sourceOnlyCount'] += ch['sourceOnlyCount']; + row['targetOnlyCount'] += ch['targetOnlyCount']; + } else if (filterParams.includes(ch.status)) { if (ch.status == FILTER_NAME.IDENTICAL) { row['identicalCount'] = row['identicalCount'] + 1; } else if (ch.status == FILTER_NAME.DIFFERENT) { diff --git a/web/pgadmin/tools/schema_diff/static/js/components/SchemaDiffCompare.jsx b/web/pgadmin/tools/schema_diff/static/js/components/SchemaDiffCompare.jsx index c6c0388936c..15551a92eda 100644 --- a/web/pgadmin/tools/schema_diff/static/js/components/SchemaDiffCompare.jsx +++ b/web/pgadmin/tools/schema_diff/static/js/components/SchemaDiffCompare.jsx @@ -495,6 +495,13 @@ export function SchemaDiffCompare({ params }) { 'groupType': record.group_name, 'isExpanded': false, 'selected': false, + // Seed status counts so the cell expander renders them at the + // top-level group too — setRecordCount() will aggregate from + // the mid-level children at render time. Issue #9892. + 'identicalCount': 0, + 'differentCount': 0, + 'sourceOnlyCount': 0, + 'targetOnlyCount': 0, 'children': {} }; let ch_id = record.id; diff --git a/web/regression/javascript/schema_diff/result_grid_count_spec.js b/web/regression/javascript/schema_diff/result_grid_count_spec.js new file mode 100644 index 00000000000..d171eadb12d --- /dev/null +++ b/web/regression/javascript/schema_diff/result_grid_count_spec.js @@ -0,0 +1,148 @@ +///////////////////////////////////////////////////////////// +// +// pgAdmin 4 - PostgreSQL Tools +// +// Copyright (C) 2013 - 2026, The pgAdmin Development Team +// This software is released under the PostgreSQL Licence +// +////////////////////////////////////////////////////////////// + +import { setRecordCount } from '../../../pgadmin/tools/schema_diff/static/js/components/ResultGridComponent'; +import { FILTER_NAME } from '../../../pgadmin/tools/schema_diff/static/js/SchemaDiffConstants'; + +const ALL_FILTERS = [ + FILTER_NAME.IDENTICAL, + FILTER_NAME.DIFFERENT, + FILTER_NAME.SOURCE_ONLY, + FILTER_NAME.TARGET_ONLY, +]; + +function leaf(status) { + return { status }; +} + +function midGroup(label, children) { + return { + label, + identicalCount: 0, + differentCount: 0, + sourceOnlyCount: 0, + targetOnlyCount: 0, + children, + }; +} + +function topGroup(label, children) { + return { + label, + identicalCount: 0, + differentCount: 0, + sourceOnlyCount: 0, + targetOnlyCount: 0, + children, + }; +} + +describe('setRecordCount', () => { + it('counts leaf children by status', () => { + const row = midGroup('Functions', [ + leaf(FILTER_NAME.IDENTICAL), + leaf(FILTER_NAME.DIFFERENT), + leaf(FILTER_NAME.DIFFERENT), + leaf(FILTER_NAME.SOURCE_ONLY), + leaf(FILTER_NAME.SOURCE_ONLY), + leaf(FILTER_NAME.SOURCE_ONLY), + leaf(FILTER_NAME.TARGET_ONLY), + ]); + + setRecordCount(row, ALL_FILTERS); + + expect(row.identicalCount).toBe(1); + expect(row.differentCount).toBe(2); + expect(row.sourceOnlyCount).toBe(3); + expect(row.targetOnlyCount).toBe(1); + }); + + it('skips leaf children whose status is filtered out', () => { + const row = midGroup('Functions', [ + leaf(FILTER_NAME.IDENTICAL), + leaf(FILTER_NAME.DIFFERENT), + leaf(FILTER_NAME.SOURCE_ONLY), + leaf(FILTER_NAME.TARGET_ONLY), + ]); + + // Only count DIFFERENT and SOURCE_ONLY. + setRecordCount(row, [FILTER_NAME.DIFFERENT, FILTER_NAME.SOURCE_ONLY]); + + expect(row.identicalCount).toBe(0); + expect(row.differentCount).toBe(1); + expect(row.sourceOnlyCount).toBe(1); + expect(row.targetOnlyCount).toBe(0); + }); + + it('aggregates mid-level child counts into the top-level group (issue #9892)', () => { + // Tree mirroring the bug reporter's case: a top-level group whose + // children are object-type groups (Functions, Tables, ...) that + // each have leaf children with statuses. Before the fix the top + // row stayed at 0/0/0/0. + const top = topGroup('Schema Objects', [ + midGroup('Functions', [ + leaf(FILTER_NAME.SOURCE_ONLY), + leaf(FILTER_NAME.SOURCE_ONLY), + leaf(FILTER_NAME.SOURCE_ONLY), + leaf(FILTER_NAME.SOURCE_ONLY), + leaf(FILTER_NAME.SOURCE_ONLY), + leaf(FILTER_NAME.SOURCE_ONLY), + leaf(FILTER_NAME.SOURCE_ONLY), + leaf(FILTER_NAME.TARGET_ONLY), + ]), + midGroup('Tables', Array(6).fill(leaf(FILTER_NAME.SOURCE_ONLY))), + midGroup('Procedures', Array(3).fill(leaf(FILTER_NAME.SOURCE_ONLY))), + midGroup('Sequences', [ + leaf(FILTER_NAME.SOURCE_ONLY), + leaf(FILTER_NAME.DIFFERENT), + ]), + midGroup('Types', [leaf(FILTER_NAME.IDENTICAL)]), + ]); + + setRecordCount(top, ALL_FILTERS); + + // 7 (Functions src) + 6 (Tables) + 3 (Procedures) + 1 (Sequences) = 17 + expect(top.sourceOnlyCount).toBe(17); + expect(top.targetOnlyCount).toBe(1); + expect(top.differentCount).toBe(1); + expect(top.identicalCount).toBe(1); + }); + + it('refreshes mid-level child counts on every call (filter change)', () => { + const mid = midGroup('Functions', [ + leaf(FILTER_NAME.IDENTICAL), + leaf(FILTER_NAME.DIFFERENT), + ]); + const top = topGroup('Schema Objects', [mid]); + + // First pass: include both statuses → mid counts both. + setRecordCount(top, ALL_FILTERS); + expect(mid.identicalCount).toBe(1); + expect(mid.differentCount).toBe(1); + expect(top.identicalCount).toBe(1); + expect(top.differentCount).toBe(1); + + // Filter changes to DIFFERENT only → top must reflect mid's refreshed + // counts, not the stale numbers from the first pass. + setRecordCount(top, [FILTER_NAME.DIFFERENT]); + expect(mid.identicalCount).toBe(0); + expect(mid.differentCount).toBe(1); + expect(top.identicalCount).toBe(0); + expect(top.differentCount).toBe(1); + }); + + it('handles an empty children list without throwing', () => { + const row = midGroup('Empty', []); + setRecordCount(row, ALL_FILTERS); + expect(row.identicalCount).toBe(0); + expect(row.differentCount).toBe(0); + expect(row.sourceOnlyCount).toBe(0); + expect(row.targetOnlyCount).toBe(0); + }); +});