Skip to content

[feature]: Include cell reference details in error summary#406

Open
deeonwuli wants to merge 2 commits into
developmentfrom
feat/import-error-cell-location
Open

[feature]: Include cell reference details in error summary#406
deeonwuli wants to merge 2 commits into
developmentfrom
feat/import-error-cell-location

Conversation

@deeonwuli

Copy link
Copy Markdown

📌 References

📝 Implementation

  • Added ImportRowLookup, a class that maps every metadata/org unit/TEI/attribute id present in an imported Excel template to its sheet and cell location ({ sheet, row, column }) before the import is sent to DHIS2
  • Column letter is extracted from cell.ref in all three Excel reading paths: readByRow (event programs), readTeiEvents (tracker program events), and readTeiRows (tracker program TEI attributes)
  • Import errors now show "Found in sheet X, cell B6 of the Excel file" in the Details column instead of just row number; row-level ids (org unit, TEI, program stage) that have no column association continue to show "row N"
  • When both a cell-level and row-only location exist for the same row, the cell ref is preferred
  • Tracker import errors that DHIS2 reports at multiple hierarchy levels (e.g. both ENROLLMENT and TRACKED_ENTITY) are deduplicated by message — the copy with location details is kept, the rest dropped

🔥 Notes for the reviewer

📹 Screenshots/Screen capture

image Screenshot 2026-06-30 at 13 21 21

📑 Others

#869dg8vmf

@bundlemon

bundlemon Bot commented Jun 30, 2026

Copy link
Copy Markdown

BundleMon

No change in files bundle size

Groups updated (1)
Status Path Size Limits
Build Folder
./**/*
1.74MB (+1.18KB +0.07%) +20%

Final result: ✅

View report in BundleMon website ➡️


Current branch size history | Target branch size history

@gqcorneby gqcorneby left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Thanks @deeonwuli! Nice work 🎉 the tests are thorough. A few in-line and general comments

  1. Translations - Can we check and remove fuzzies in translation files?
  2. Cell based data source - As stated in the description, only rows, teis, and events are mapped back to the template. Cell based data sources (usually datasets), are not supported. @MiquelAdell Do we want to include this in this PR or as a separate backlog?

Heads-up: Need to double check but I think we can't just set group = buildRowNumber(cell.ref) in readByCell. Group is the grouping key at ExcelReader.ts:89, so distinct row numbers would re-split the aggregated payload. It probably needs sheet/row/column carried on the data value itself, with the lookup indexing by cell.

file you can use to test:
NHWA Module 1_v1 (1).xlsx

locationsById.set(id, [...(locationsById.get(id) ?? []), location]);
};

dataPackage.dataEntries.forEach(entry => {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This works for row based data sources but follows an imperative pattern. The same output can be built as id → location pairs via flatMap, then a single groupBy. What do you think?

Comment thread src/data/Dhis2Import.ts

type Item = { id: string; name: string };

function deduplicateErrors(errors: ErrorMessage[]): ErrorMessage[] {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Grouping by message alone seems correct for the intended case, maybe we can add it as a comment to add some context?

There's a theoretical hole: two distinct errors sharing an identical generic message where only one resolved a location, the un-located would get dropped as if it were a duplicate. I think it's not a real scenario but opening it up for discussion. Cheap fix would be to update the grouping to .groupBy(e => ${e.message}@@${e.details}`

@MiquelAdell

MiquelAdell commented Jul 6, 2026

Copy link
Copy Markdown

Cell based data source - As stated in the description, only rows, teis, and events are mapped back to the template. Cell based data sources (usually datasets), are not supported. @MiquelAdell Do we want to include this in this PR or as a separate backlog?

No, since this is paid by the client and the client only uses tracker programs let's not do that in this one.
Thanks!

@deeonwuli you can address the rest of the comments whenever you have a moment.
Thanks!

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.

3 participants