AMP-31226: Add third dimension for disaggregation by brianbrix · Pull Request #4522 · devgateway/amp

brianbrix · 2026-10-29T07:33:29Z

No description provided.

# Conflicts: # amp/src/main/java/org/digijava/module/aim/helper/GlobalSettingsConstants.java # amp/src/main/resources/xmlpatches/4.0/AMP-31057-Add-Currecies-for-dashboard.xml # amp/src/main/resources/xmlpatches/4.0/AMP-31057-Donor-Funding-Admin-Levels-v1.xml # amp/src/main/resources/xmlpatches/general/views/v_nationalobjectives_level_0.xml # amp/src/main/resources/xmlpatches/general/views/v_nationalobjectives_level_1.xml # amp/src/main/resources/xmlpatches/general/views/v_nationalobjectives_level_2.xml # amp/src/main/resources/xmlpatches/general/views/v_nationalobjectives_level_3.xml # amp/src/main/resources/xmlpatches/general/views/v_nationalobjectives_level_4.xml # amp/src/main/resources/xmlpatches/general/views/v_nationalobjectives_level_5.xml # amp/src/main/resources/xmlpatches/general/views/v_nationalobjectives_level_6.xml # amp/src/main/resources/xmlpatches/general/views/v_nationalobjectives_level_7.xml # amp/src/main/resources/xmlpatches/general/views/v_nationalobjectives_level_8.xml # amp/src/main/resources/xmlpatches/general/views/v_pledges_tertiary_programs_level_2.xml # amp/src/main/resources/xmlpatches/general/views/v_pledges_tertiary_programs_level_3.xml # amp/src/main/webapp/WEB-INF/jsp/esrigis/view/mainmapPopup.jsp

Missing gazeteer

fix/GGW/GGW-issues

# Conflicts: # .github/workflows/deploy.yml # amp/pom.xml # amp/src/main/java/org/dgfoundation/amp/onepager/util/ActivityUtil.java # amp/src/main/java/org/digijava/module/message/jobs/AmpDonorFundingJob.java # amp/src/main/resources/org/digijava/module/aim/dbentity/AmpStructure.hbm.xml # amp/src/main/webapp/WEB-INF/jsp/esrigis/view/mainmapPopup.jsp

Copilot

🔵 Needs a closer look

A critical active finding remains in AmpStructure.clone(): its shallow-copied image set is mutated, corrupting the original structure during merges.

3 open issues

_{Review effort: Lite}

Copilot

🟡 Changes recommended

Approval is blocked by critical active issues: the new report type is missing from backend mappings, causing previews to fail, and its fetch path includes non-disaggregated rows instead of only option rows.

6 open issues

_{Review effort: Lite}

+    case 'funding-indicator-disaggregation':
+      return 'ID';


            case ArConstants.INDICATOR_TYPE:
+            case ArConstants.INDICATOR_DISAGGREGATION_TYPE:
                return indicatorColumn;


+    @JsonProperty("ID")
+    INDICATOR_DISAGGREGATION((long) INDICATOR_DISAGGREGATION_TYPE, "ID");


Copilot

🟡 Changes recommended

A critical view defect produces synthetic criterion and option rows with invalid IDs and empty labels, while the locationless-actuals branch cannot return rows because the write path never persists the required links.

7 open issues

_{Review effort: Lite}

+                CROSS JOIN LATERAL (VALUES
+                    (0, disagg_value.parent_category_id),
+                    (1, disagg_value.child_category_id),
+                    (2, disagg_value.third_category_id)
+                ) AS disagg_slot(criterion_order, option_id)


Copilot

🟡 Changes recommended

Two unresolved critical issues remain: criterion validation accepts category values outside the required class, and prepareMerge can mutate the source structure or dereference a null image set.

8 open issues

_{Review effort: Lite}

+            if (session.get(AmpCategoryValue.class, criterion) == null) {
+                throw new ApiRuntimeException(BAD_REQUEST, ApiError.toError(
+                        "Disaggregation criterion with id " + criterion + " not found."));
+            }


Copilot

🟡 Changes recommended

Active moderate and critical findings remain around unsupported report-option serialization, incorrect NULL disaggregation handling that duplicates amounts, and locationless actuals being discarded on activity saves.

11 open issues

_{Review effort: Lite}

+				/* Locationless actuals remain visible for each linked activity. */
+				SELECT con.activity_id AS amp_activity_id,
+				aigv.original_value AS value,
+				aigv.type AS value_type,
+				aigv.original_value_date AS value_date,
+				ind.indicator_id AS me_indicator_id,
+				t.amp_theme_id AS program_id,
+				aidv.parent_category_id AS category_value_id,
+				aidv.child_category_id AS child_category_value_id,
+				aidv.third_category_id AS third_category_value_id,


+export const INDICATOR_DISAGGREGATION_REPORT = 'funding-indicator-disaggregation';
+export const FUNDING_GROUPING_RADIO_OPTIONS = [FUNDING_DONOR, REGIONAL_REPORT, COMPONENT_REPORT, PLEDGES_REPORT,
+  INDICATOR_REPORT, INDICATOR_DISAGGREGATION_REPORT];


Copilot

🟡 Changes recommended

Two unresolved issues remain: a newly added funding-grouping option is not wired through the API and type conversion, and null image collections can still cause a critical exception in AmpStructure.prepareMerge.

11 open issues

1 resolved since last review

Missing amount coordinates duplicate values across combinations

1 file not reviewed

amp/TEMPLATE/reampv2/package-lock.json: Generated file

_{Review effort: Lite}

+            aux.getImages().clear();
+            aux.getImages().addAll(new HashSet<>());


Copilot

🔵 Needs a closer look

Approval is blocked by an unresolved client/server report-option mismatch and a critical shallow-clone bug that can clear image associations from the source structure.

11 open issues

1 file not reviewed

amp/TEMPLATE/reampv2/package-lock.json: Generated file

_{Review effort: Lite}

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

🟡 Changes recommended

Critical integration gaps prevent the new report option and locationless disaggregated actuals from functioning end to end, while a missing third-category deletion check can cause database errors.

12 open issues

1 file not reviewed

amp/TEMPLATE/reampv2/package-lock.json: Generated file

_{Review effort: Lite}

+    @Interchangeable(fieldTitle = "Third Category", importable = true, pickIdOnly = true)
+    private AmpCategoryValue thirdCategory;


brianbrix and others added 30 commits April 26, 2026 21:33

Add funding columns to this old GGW codebase

30c6a72

Add workflow to old codebase

5d998ec

Add workflow to old codebase

6dbd505

Add workflow to old codebase

01722e9

Resolve failing deployment

d487737

Add funding columns to the old GGW codebase DonorFundingJob

f6b5be9

Add Missing global settings

d27116f

Add Missing global settings

efc1653

Failing xml patches

d3f6db2

Failing xml patches

8ac723a

Failing xml patches

6e8d5ec

Failing xml patches

8489239

Failing xml patches

4737503

Revert donor job columns

2d506e6

Revert donor job columns

eaf47c3

Resolve map popup issue

7824ac6

Resolve map popup issue

ccda24b

Report engine failing

d4c708f

Report engine failing

0ed93d1

Missing gazeteer

Internal error when saving activity

fd2781c

Internal error when saving activity

d7e3293

Internal error when saving activity

20a1394

Project list folded on load

04fbc87

AMP-31125: Issue viewing/editing some activities from tab

2011c00

AMP-31125: Issue viewing/editing some activities from tab

aaf11ce

AMP-31125: Issue some activities missing from GIS

3c6957d

AMP-31125: Issue some activities missing from GIS

045d5f0

Merge pull request #4475 from devgateway/fix/GGW/GGW-issues

37c81be

fix/GGW/GGW-issues

Copilot AI lite review requested due to automatic review settings November 2, 2026 05:27

Copilot started reviewing on behalf of brianbrix November 2, 2026 05:28

View session

Copilot AI reviewed Nov 2, 2026

View reviewed changes

Copilot AI lite review requested due to automatic review settings November 2, 2026 07:30

Copilot started reviewing on behalf of brianbrix November 2, 2026 07:30

View session

Copilot AI reviewed Nov 2, 2026

View reviewed changes

Copilot AI lite review requested due to automatic review settings November 2, 2026 11:47

Copilot started reviewing on behalf of brianbrix November 2, 2026 11:47

View session

Copilot AI reviewed Nov 2, 2026

View reviewed changes

brianbrix force-pushed the feature/AMP-31226/New-Disaggregation-Dimension branch from c7cb252 to ed678a1 Compare November 2, 2026 13:16

Copilot AI lite review requested due to automatic review settings November 2, 2026 13:16

Copilot started reviewing on behalf of brianbrix November 2, 2026 13:16

View session

Copilot AI reviewed Nov 2, 2026

View reviewed changes

AMP-31226: Extend indicator reports to include disaggregation again

1b36349

Copilot AI lite review requested due to automatic review settings November 3, 2026 00:38

AMP-31226: Extend indicator reports to include disaggregation again

71a1a90

Copilot started reviewing on behalf of brianbrix November 3, 2026 00:38

View session

Copilot AI reviewed Nov 3, 2026

View reviewed changes

AMP-31226: Add filter for indicator reports

5babb05

Copilot AI lite review requested due to automatic review settings November 3, 2026 08:32

Copilot started reviewing on behalf of brianbrix November 3, 2026 08:33

View session

Copilot AI reviewed Nov 3, 2026

View reviewed changes

Comment thread amp/src/main/java/org/digijava/module/aim/dbentity/AmpStructure.java

Comment on lines +201 to +202

aux.getImages().clear();

aux.getImages().addAll(new HashSet<>());

AMP-31226: Filter widget failing due to date picker

483ea62

Copilot AI lite review requested due to automatic review settings November 3, 2026 09:16

Copilot started reviewing on behalf of brianbrix November 3, 2026 09:17

View session

Copilot AI reviewed Nov 3, 2026

View reviewed changes

AMP-31226: Pin report filter widget and add regressions

8653573

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI lite review requested due to automatic review settings November 3, 2026 10:14

Copilot started reviewing on behalf of brianbrix November 3, 2026 10:15

View session

Copilot AI reviewed Nov 3, 2026

View reviewed changes

Comment thread amp/src/main/java/org/digijava/module/aim/dbentity/AmpIndicatorDisaggregationValue.java

Comment on lines +23 to +24

@Interchangeable(fieldTitle = "Third Category", importable = true, pickIdOnly = true)

private AmpCategoryValue thirdCategory;

Navigation Menu

Repository navigation

AMP-31226: Add third dimension for disaggregation - #4522

AMP-31226: Add third dimension for disaggregation#4522
brianbrix wants to merge 54 commits into
developfrom
feature/AMP-31226/New-Disaggregation-Dimension

brianbrix commented Oct 29, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		@JsonProperty("ID")
		INDICATOR_DISAGGREGATION((long) INDICATOR_DISAGGREGATION_TYPE, "ID");

		aux.getImages().clear();
		aux.getImages().addAll(new HashSet<>());

		@Interchangeable(fieldTitle = "Third Category", importable = true, pickIdOnly = true)
		private AmpCategoryValue thirdCategory;

Conversation

brianbrix commented Oct 29, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants