Skip to content

Feature: Add missing dhis2 periods#385

Open
nshandra wants to merge 8 commits intodevelopmentfrom
feat/add-missing-dhis2-periods
Open

Feature: Add missing dhis2 periods#385
nshandra wants to merge 8 commits intodevelopmentfrom
feat/add-missing-dhis2-periods

Conversation

@nshandra
Copy link

@nshandra nshandra commented Dec 23, 2025

📌 References

📝 Implementation

Added the following dataSet periods:

  • WeeklyWednesday
  • WeeklyThursday
  • WeeklySaturday
  • WeeklySunday
  • BiWeekly
  • BiMonthly
  • QuarterlyNov
  • SixMonthly
  • SixMonthlyApril
  • SixMonthlyNov

Added tests for all periods.

🔥 Notes for the reviewer

📹 Screenshots/Screen capture

📑 Others

#869b9v6pp

@nshandra nshandra self-assigned this Dec 23, 2025
@ifoche
Copy link
Member

ifoche commented Dec 23, 2025

@bundlemon
Copy link

bundlemon bot commented Dec 23, 2025

BundleMon

No change in files bundle size

Groups updated (1)
Status Path Size Limits
Build Folder
./**/*
1.72MB (-165.77KB -8.62%) +20%

Final result: ✅

View report in BundleMon website ➡️


Current branch size history | Target branch size history

@nshandra nshandra marked this pull request as ready for review January 13, 2026 19:30
@nshandra nshandra requested a review from adrianq January 13, 2026 19:32
@adrianq adrianq requested a review from xurxodev March 19, 2026 12:15
Copy link

@xurxodev xurxodev left a comment

Choose a reason for hiding this comment

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

thanks @nshandra

This PR expands the supported DHIS2 dataset period types and adds a new TypeScript implementation of buildAllPossiblePeriods, along with Jest coverage for all newly added variants.

It's great.

I've found some issues to review.

1. Should fix

  • src/domain/entities/DataForm.ts declares periodType?: DataFormPeriod, but src/webapp/utils/periods.ts can throw for missing/unsupported periodType.
    buildAllPossiblePeriods should not throw when periodType is missing; either accept periodType?: DataFormPeriod and return [], or add an early guard in the function.
  • Improve type safety/readability in src/webapp/utils/periods.ts and push it towards a more functional/immutable style without introducing mutable let state. It's safe this change because now we have unit test. Examples:
    • eliminate let current; in generateSixMonthlyNovPeriods by computing current as a const expression (ternary) and, where feasible, use clone().add(...) to avoid mutating Moment instances;
    • type the accumulator/output arrays explicitly (e.g. const dates: string[] = []) so the contract is clear at a glance;
    • make intermediate values explicit/typed in helpers (so the types of start/end/current and formatting pieces are obvious during review).

2. Recommendations non blocking

  • Tests status: src/test/utils/periods.spec.ts covers the newly added period variants and passes (yarn test --runInBand src/test/utils/periods.spec.ts). Consider adding a couple more edge cases (e.g. startDate > endDate) to validate robustness.

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