Skip to content

Conversation

@dilankavishka
Copy link

@dilankavishka dilankavishka commented Jan 13, 2026

Requirements

  • This PR has a title that briefly describes the work done including the ticket number. If there is a ticket, make sure your PR title includes a conventional commit label. See existing PR titles for inspiration.
  • My work is based on designs, which are linked or shown either in the Jira ticket or the description below. (See also: Styleguide)
  • My work includes tests or is validated by existing tests.

Summary

Created the Growth Chart App

Screenshots

Screenshot 2026-01-13 at 19 27 57

Related Issue

https://openmrs.atlassian.net/browse/O3-5332

Other

@dilankavishka
Copy link
Author

@anjula-sack @denniskigen Could you review this please?

@@ -0,0 +1,117 @@
# @openmrs/esm-patient-growth-chart-app
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you update this with more details like a small description of growth chart, scope, implementation plan and current status? @dilankavishka

}
}
]
} No newline at end of file
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing end of line here

"@openmrs/esm-form-engine-lib": "next",
"lodash-es": "^4.17.21",
"react-error-boundary": "^4.0.13"
"lodash.findlast": "^4.6.0",
Copy link
Contributor

Choose a reason for hiding this comment

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

Why form engine's package.json is updated? @dilankavishka

@NethmiRodrigo
Copy link
Collaborator

NethmiRodrigo commented Jan 20, 2026

Thank you kicking this off @dilankavishka! I hope this doesn’t sound harsh, but a couple of things (that applies to your other PRs as well):

  1. I see that you’ve broken down the tickets, which is good, but they are slightly a bit too broken down. This isn’t sufficient enough of work done in one PR to be merged in (especially since this is the community repo). It should be a working functionality that is hooked up to the API. For example, this PR would be a combination of your current changes and the work in O3-5333 : Fetch Patient Data #2951.
  2. It is a requirement to have unit tests and E2E tests in the same PR as the one you introduce the functionality.
  3. In the case of O3-5334: Add WHO growth standards JSON data #2954, we don’t have usually have data loaded in the frontend modules in the form of JSONs. We typically utilise the initializer module to load metadata and configure them through content packages. I’m not quite sure how that would work here since there is no backend module that would take care of actually saving that metadata. I suggest either asking this on talk or on your ticket and tagging folks from the platform team. Making it so that the metadata is from content packages, also makes sure that its configurable, ie, any implementer would be able to just change it to use their own standard data.
  4. I see that you’ve made a table version, but I see that we need a chart, but no requirement for a table in the wiki.
  5. Ideally, we should build this under a feature flag, until we are sure that its ready (especially since this is in the patient chart itself)

@dilankavishka
Copy link
Author

Thank you so much for the feedback @NethmiRodrigo !

I understand the concerns about the PR. I’ll adjust my approach and combine the related changes into a single PR going forward. would it be okay if I close other two PRs and combine their changes into this PR, and then continue all future work on this branch to deliver the complete functionality in a single PR?

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