-
Notifications
You must be signed in to change notification settings - Fork 10
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: update the summary table to display a message #2002
base: master
Are you sure you want to change the base?
Conversation
🚀 Deployed on https://pr-2002--dhis2-import-export.netlify.app |
1c53038
to
d4a9ee7
Compare
afc1b83
to
e0df447
Compare
2941960
to
4434f17
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
one comment + please fix failing tests
const Summary = ({ summary, importType }) => { | ||
// gml import type object return | ||
const Summary = ({ summary, importType, isDryRun }) => { | ||
if (isDryRun) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this logic looks like it would apply to all import dry runs, but we want to only apply it to tracked entities
and events
.. right?
21bca40
to
82f372c
Compare
…f a table when isdryrun is true feat: update the summary to display noticebox for pages based on import type
82f372c
to
09c2b19
Compare
const Summary = ({ summary, importType }) => { | ||
// gml import type object return | ||
const Summary = ({ summary, importType, isDryRun }) => { | ||
if (isDryRun && importType === 'TRACKER_IMPORT_JOB') { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would just put this in a variable to make the intention clearer
if (isDryRun && importType === 'TRACKER_IMPORT_JOB') { | |
const isTrackerDryRun = isDryRun && importType === 'TRACKER_IMPORT_JOB | |
if (isTrackerDryRun) { |
and maybe add a comment for posterity:
// the new tracker endpoints don't return a full summary, so we're just showing the ignored/total
Implements DHIS2-17164
Description
In the case of a dry run, this commit updates the table summary (screenshots below), and instead show a Notification that says "{ignoredCount} entities will be ignored out of {totalCount} entities"
Known issues
Checklist
All points above should be relevant for feature PRs. For bugfixes, some points might not be relevant. In that case, just check them anyway to signal the work is done.
Screenshots