Skip to content

Conversation

@gqcorneby
Copy link
Contributor

📌 References

📝 Implementation

  • generate cartesianProduct of selected orgUnit, categoryOption, and period
  • create issue usecase. Currently, no check for duplicates. Probably unlikely that user will create manual issues with the same exact description.
  • make issue period optional

📹 Screenshots/Screen capture

2025-08-05.13-14-36.mp4
2025-08-05.13-17-25.mp4

🔥 Notes to the tester

@bundlemon
Copy link

bundlemon bot commented Aug 5, 2025

BundleMon

No change in files bundle size

Groups added (1)
Status Path Size Limits
Build Folder
./**/*
+1.24MB +20%

Final result: ✅

View report in BundleMon website ➡️


Current branch size history

Copy link
Contributor

@eperedo eperedo left a comment

Choose a reason for hiding this comment

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

[code-review] Thanks for the code @gqcorneby, looks good, just minor things.

execute(options: CreateIssueUseCaseOptions): FutureData<Maybe<QualityAnalysis>> {
const { qualityAnalysisId, sectionId, issues } = options;

if (issues.length <= 0) return Future.success(undefined);
Copy link
Contributor

Choose a reason for hiding this comment

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

Not need to change, just a mention we have a Future.void() method which is equivalent to Futures.success(undefined)

api={api}
onChange={updateForm("orgUnitPaths")}
selected={addIssueForm.orgUnitPaths}
levels={[1, 2, 3]}
Copy link
Contributor

Choose a reason for hiding this comment

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

can we extract the levels and selectableLevels values into a constant and reuse it. Right now we have 3 different components where we're using these values (CountrySelector.tsx and ConfigurationForm.tsx)

}

function cartesianProduct<T>(arrays: T[][]): T[][] {
return arrays.reduce<T[][]>((acc, curr) => acc.flatMap(a => curr.map(c => [...a, c])), [[]]);
Copy link
Contributor

Choose a reason for hiding this comment

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

We have an implementation for cartesian in Collection.ts, if we're using this because a particular reason or limitation could you maybe add some comment with the reason?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh! I didn't see it there. Thanks, will use it.

period,
categoryOptionComboId: categoryOption,
countryId: orgUnit,
description: description || "",
Copy link
Contributor

Choose a reason for hiding this comment

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

No need to change, but I think description could be just string since we're adding an empty string as a fallback?

type AddIssueForm = {
    // ...
    description: string;
};

@gqcorneby gqcorneby requested a review from eperedo August 13, 2025 02:55
Copy link
Contributor

@Ramon-Jimenez Ramon-Jimenez left a comment

Choose a reason for hiding this comment

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

Thanks @gqcorneby , it looks really great. Just an adjustments I think we should do:

  • If you select a region (e.g. AFR), it should allow you to select and create Manual issues for any country within such region, or for the region itself. Right now, it only allows you to create the issue for the region (or empty), but not for the countries included in such region.

@gqcorneby
Copy link
Contributor Author

Hi @Ramon-Jimenez! I had to update d2-ui-components to 2.10.1 so I can manually set the children as selectable.

I just realized that the step summary in the home page will always either be gray or red for Manual Issues. Just wanted to confirm if there's a specific trigger where this step is set to "success".

image

Copy link
Contributor

@Ramon-Jimenez Ramon-Jimenez left a comment

Choose a reason for hiding this comment

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

Thanks @gqcorneby ! It looks and works great overall.

@Ramon-Jimenez Ramon-Jimenez merged commit e106732 into development Aug 19, 2025
6 checks passed
@Ramon-Jimenez Ramon-Jimenez deleted the feature/manual-issue branch August 19, 2025 12:40
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.

4 participants