AMP-31022 Indicators module changes #4430

brianbrix · 2025-08-21T19:42:34Z

No description provided.

Copilot

Pull Request Overview

This PR implements comprehensive enhancements to the AMP Indicators module, adding support for new indicator fields, outcome/output management, and improved UI. The changes introduce additional categorization capabilities and a management interface for outcomes and outputs.

Key changes include:

Addition of new indicator fields (outcome, output, type, disaggregation, unit of measure, etc.)
Implementation of outcome and output management with CRUD operations
Enhanced UI with improved filtering, search, and visual organization
Database schema updates with new entities and Hibernate mappings

Reviewed Changes

Copilot reviewed 41 out of 41 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
Java entity classes	Added AmpOutcome and AmpOutput entities with bidirectional relationships
Hibernate mappings	New mapping files for outcome/output entities and updated indicator mappings
Service classes	New service for outcome/output CRUD operations and updated indicator service
Database patches	SQL script to create indicator category values
Frontend components	New React pages for outcome/output management with improved UI
API endpoints	New REST endpoints for outcome/output operations

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-09-01T14:43:44Z

.../org/digijava/kernel/ampapi/endpoints/indicator/manager/service/AmpOutcomeOutputService.java

+
+    public List<AmpOutputDTO> getOutputsByOutcomeId(Long outcomeId) {
+        Session session = PersistenceManager.getSession();
+        AmpOutcome outcome = (AmpOutcome) session.get(AmpOutcome.class, outcomeId);


Unnecessary explicit casting. The session.get() method with a Class parameter already returns the correct type. Remove the explicit cast: AmpOutcome outcome = session.get(AmpOutcome.class, outcomeId);

Suggested change

AmpOutcome outcome = (AmpOutcome) session.get(AmpOutcome.class, outcomeId);

AmpOutcome outcome = session.get(AmpOutcome.class, outcomeId);

Copilot · 2025-09-01T14:43:45Z

.../org/digijava/kernel/ampapi/endpoints/indicator/manager/service/AmpOutcomeOutputService.java

+
+    public void deleteOutcome(Long outcomeId) {
+        Session session = PersistenceManager.getSession();
+        AmpOutcome outcome = (AmpOutcome) session.get(AmpOutcome.class, outcomeId);


Same unnecessary explicit casting as above. Remove the cast: AmpOutcome outcome = session.get(AmpOutcome.class, outcomeId);

Suggested change

AmpOutcome outcome = (AmpOutcome) session.get(AmpOutcome.class, outcomeId);

AmpOutcome outcome = session.get(AmpOutcome.class, outcomeId);

Copilot · 2025-09-01T14:43:46Z

...TEMPLATE/reampv2/packages/reampv2-app/src/modules/admin/indicator_manager/utils/constants.ts

@@ -1,2 +1,2 @@
-export const TRN_PREFIX = 'amp.inidcatormanager.dashboard:';
-export const TRN_PREFIX_DASHBOARD = 'amp.inidcatormanager.dashboard:';
+export const TRN_PREFIX = 'amp.indicatormanager.dashboard:';


[nitpick] Fixed typo in constant name from 'inidcator' to 'indicator', but consider if TRN_PREFIX and TRN_PREFIX_DASHBOARD should have different values since they're now identical.

Suggested change

export const TRN_PREFIX = 'amp.indicatormanager.dashboard:';

export const TRN_PREFIX = 'amp.indicatormanager:';

Copilot · 2025-09-01T14:43:46Z

...packages/reampv2-app/src/modules/admin/indicator_manager/components/table/IndicatorTable.tsx

+        },
+        csvFormatter: (_cell: any, row: any) => {
+          if (outcomesReducer.loading) return '';
+          const foundOutcome = !outcomesReducer.loading && outcomesReducer.programs.find((outcome: any) => outcome.id === row.outcomeId);


Incorrect property access. Should be outcomesReducer.outcomes instead of outcomesReducer.programs to find outcomes correctly.

Suggested change

const foundOutcome = !outcomesReducer.loading && outcomesReducer.programs.find((outcome: any) => outcome.id === row.outcomeId);

const foundOutcome = !outcomesReducer.loading && outcomesReducer.outcomes.find((outcome: any) => outcome.id === row.outcomeId);

Copilot · 2025-09-01T14:43:46Z

...ges/reampv2-app/src/modules/admin/indicator_manager/components/modals/EditIndicatorModal.tsx

+            calculationMethod: values.calculationMethod,
+            responsibleOrganizations: values.responsibleOrganizations,
+            frequency: values.frequency,
+            programId: values.programId ? values.programId: null,


[nitpick] Missing space after colon. Should be values.programId ? values.programId : null for consistent formatting.

Suggested change

programId: values.programId ? values.programId: null,

programId: values.programId ? values.programId : null,

Copilot · 2025-09-01T14:43:46Z

...ain/java/org/digijava/kernel/ampapi/endpoints/indicator/manager/IndicatorManagerService.java

+                .collect(Collectors.toSet());
+            indicator.setResponsibleOrganizations(orgs);
+        } else {
+            indicator.setResponsibleOrganizations(new java.util.HashSet<>());


[nitpick] Use the already imported HashSet class instead of the fully qualified name: new HashSet<>()

Suggested change

indicator.setResponsibleOrganizations(new java.util.HashSet<>());

indicator.setResponsibleOrganizations(new HashSet<>());

Copilot · 2025-09-01T14:43:47Z

...ain/java/org/digijava/kernel/ampapi/endpoints/indicator/manager/IndicatorManagerService.java

+                    .collect(Collectors.toSet());
+                indicator.setResponsibleOrganizations(orgs);
+            } else {
+                indicator.setResponsibleOrganizations(new java.util.HashSet<>());


[nitpick] Same issue as above - use the imported HashSet class: new HashSet<>()

Suggested change

indicator.setResponsibleOrganizations(new java.util.HashSet<>());

indicator.setResponsibleOrganizations(new HashSet<>());

AMP-31027: List/View Existing Outcomes and Outputs AMP-31028:Add New Outcome AMP-31029:Add New Output

AMP-31044: break down target and original value by dissagregation

…heme.

…t similar logic for NPO/Theme/Program x indicator.

.../main/java/org/dgfoundation/amp/nireports/amp/indicators/IndicatorTextualTokenBehaviour.java

brianbrix changed the title ~~AMP-31026: Output / outcome manager AMP-31027: List/View Existing Outcomes and Outputs AMP-31028:Add New Outcome AMP-31029:Add New Output~~ AMP-31022 Indicators module changes Aug 21, 2025

brianbrix requested a review from jdeanquin-dg August 28, 2025 17:36

jdeanquin-dg requested a review from Copilot September 1, 2025 14:41

Copilot AI reviewed Sep 1, 2025

View reviewed changes

brianbrix added 5 commits September 15, 2025 13:04

AMP-31026: Output / outcome manager

d1007bd

AMP-31027: List/View Existing Outcomes and Outputs AMP-31028:Add New Outcome AMP-31029:Add New Output

AMP-31025: ADD / Edit indicator

24df9de

AMP-31022:Indicators module changes

2948b4f

AMP-31022:Indicators module changes

7d10cab

AMP-31044: break down target and original value by dissagregation

AMP-31044: break down target and original value by dissagregation.

8c3fb35

jdeanquin-dg force-pushed the task/AMP-31022/Indicators-module-changes branch from a6db9b2 to 8c3fb35 Compare September 15, 2025 16:05

This comment was marked as outdated.

Sign in to view

ginchauspe added 10 commits September 15, 2025 15:50

AMP-30988: Add log entry for sorting.

b5320b6

AMP-31050: Fix displaying the indicator value 2 times in the column.

f263dd4

AMP-31050: Modify the indicator per sector view to track Pillar/NPO/T…

6be8da1

…heme.

AMP-31050: Rollback changes over the sectors x indicator and implemen…

66fa163

…t similar logic for NPO/Theme/Program x indicator.

AMP-31050: Add behavior.

f61d28a

AMP-31050: Testing multi level hierarchy reports.

ab7bd2f

AMP-31050: Replace strings with constants.

b497cb2

AMP-31050: Improve speed.

a4ccb82

AMP-31050: Update 2 more views.

f9cb32e

AMP-31050: Checking some options for the Donor Group hierarchies.

6bf9807

cursor bot reviewed Sep 15, 2025

View reviewed changes

.../main/java/org/dgfoundation/amp/nireports/amp/indicators/IndicatorTextualTokenBehaviour.java Show resolved Hide resolved

AMP-31022: Force recreate views.

c242c3d

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

AMP-31022 Indicators module changes #4430

AMP-31022 Indicators module changes #4430

Uh oh!

brianbrix commented Aug 21, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Sep 1, 2025

Uh oh!

Copilot AI Sep 1, 2025

Uh oh!

Copilot AI Sep 1, 2025

Uh oh!

Copilot AI Sep 1, 2025

Uh oh!

Copilot AI Sep 1, 2025

Uh oh!

Copilot AI Sep 1, 2025

Uh oh!

Copilot AI Sep 1, 2025

Uh oh!

This comment was marked as outdated.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	AmpOutcome outcome = (AmpOutcome) session.get(AmpOutcome.class, outcomeId);
	AmpOutcome outcome = session.get(AmpOutcome.class, outcomeId);

	export const TRN_PREFIX = 'amp.indicatormanager.dashboard:';
	export const TRN_PREFIX = 'amp.indicatormanager:';

	const foundOutcome = !outcomesReducer.loading && outcomesReducer.programs.find((outcome: any) => outcome.id === row.outcomeId);
	const foundOutcome = !outcomesReducer.loading && outcomesReducer.outcomes.find((outcome: any) => outcome.id === row.outcomeId);

	programId: values.programId ? values.programId: null,
	programId: values.programId ? values.programId : null,

	indicator.setResponsibleOrganizations(new java.util.HashSet<>());
	indicator.setResponsibleOrganizations(new HashSet<>());

AMP-31022 Indicators module changes #4430

Are you sure you want to change the base?

AMP-31022 Indicators module changes #4430

Uh oh!

Conversation

brianbrix commented Aug 21, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Sep 1, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 1, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 1, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 1, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 1, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 1, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 1, 2025

Choose a reason for hiding this comment

Uh oh!

This comment was marked as outdated.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants