Skip to content

Seed CDIP indicator data - #4516

Closed
brianbrix wants to merge 3 commits into
developfrom
cdip-seed-data
Closed

brianbrix wants to merge 3 commits into
developfrom
cdip-seed-data

Conversation

@brianbrix

Copy link
Copy Markdown
Contributor

No description provided.

Copilot AI balanced review requested due to automatic review settings September 29, 2026 05:01

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Activity selection is not Senegal-specific, and retry logic can omit missing indicator values.

Review effort: Balanced
Findings: 1 High severity · 1 Medium severity

Open (2)
What changed in this PR

Seeds CDIP core indicator data for Senegal and registers its funding export job.

Changes:

  • Adds indicator categories, indicators, connections, and demo values.
  • Registers AmpCoreIndicatorFundingJob.
File Description
CDIP-AMP-core-indicator-funding-job.xml Seeds CDIP indicator data and registers the Quartz job.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

SELECT 1
FROM amp_indicator_values existing
WHERE existing.ind_connect_id = connection.id
AND existing.value_type = value_type
Copilot AI review requested due to automatic review settings September 29, 2026 05:09

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

Anchor selection can use non-Senegal locations, and the value-type existence check is incorrectly scoped.

Review effort: Balanced
Findings: 1 High severity · 1 Medium severity

Open (2)

Copilot AI review requested due to automatic review settings September 29, 2026 06:32

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

SQL scoping and overly broad migration updates can leave values incomplete or overwrite unrelated indicator connections.

Review effort: Balanced
Findings: 1 High severity · 2 Medium severity

Open (3)
Resolved since last review (1)

Comment on lines +68 to +70
UPDATE amp_indicator_connection existing
SET activity_id = anchor.amp_activity_id,
activity_location = anchor.amp_activity_location_id
SELECT 1
FROM amp_indicator_values existing
WHERE existing.ind_connect_id = connection.id
AND existing.value_type = value_type
@brianbrix brianbrix closed this Oct 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants