-
Notifications
You must be signed in to change notification settings - Fork 22
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: [DHIS2-14775][DHIS2-14729] Render data entry plugins #3222
feat: [DHIS2-14775][DHIS2-14729] Render data entry plugins #3222
Conversation
…uildFormMetadataOnDemand
…uildFormMetadataOnDemand # Conflicts: # src/core_modules/capture-core/components/DataEntries/EnrollmentRegistrationEntry/EnrollmentRegistrationEntry.types.js # src/core_modules/capture-core/components/Pages/New/RegistrationDataEntry/RegistrationDataEntry.component.js
…ataOnDemand' into eh/feat/DHIS2-14729_RenderDataEntryPlugins # Conflicts: # package.json # src/core_modules/capture-core/components/DataEntries/common/TEIAndEnrollment/useMetadataForRegistrationForm/buildFunctions/buildEnrollmentForm.js # src/core_modules/capture-core/components/DataEntries/common/TEIAndEnrollment/useMetadataForRegistrationForm/buildFunctions/buildTrackedEntityTypeCollection.js # src/core_modules/capture-core/components/DataEntries/common/TEIAndEnrollment/useMetadataForRegistrationForm/hooks/useEnrollmentFormFoundation.js # src/core_modules/capture-core/components/DataEntries/common/TEIAndEnrollment/useMetadataForRegistrationForm/hooks/useTrackedEntityTypeCollection.js # yarn.lock
…_RenderDataEntryPlugins # Conflicts: # package.json
Lockfile seems to not respond to the resolutions we set because we're running an alpha version of the app-platform |
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.
Great job on this feature @eirikhaugstulen! I added a few small comments that I hope you can have a look at. Thanks!
const { data: enrollment, isLoading, error } = useIndexedDBQuery( | ||
['enrollmentForm', program?.id], | ||
() => buildEnrollmentForm({ | ||
// $FlowFixMe - Flow does not understand that the values are not null here |
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.
Hey, there are quite a few // $FlowFixMe
in this file. Can anything be done to improve this?
@@ -129,8 +134,24 @@ export class EnrollmentFactory { | |||
) { | |||
// $FlowFixMe | |||
await cachedProgramTrackedEntityAttributes.asyncForEach(async (trackedEntityAttribute) => { | |||
const element = await this.dataElementFactory.build(trackedEntityAttribute); | |||
element && section.addElement(element); | |||
if (trackedEntityAttribute?.id === 'plugin') { |
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.
The string 'plugin'
is used a couple of times across the PR. Does it make sense to move to a const in DataEntryPlugin.const.js ?
if (trackedEntityAttribute?.id === 'plugin') { | ||
const element = new DataEntryPluginConfig((o) => { | ||
o.id = trackedEntityAttribute.id; | ||
o.name = trackedEntityAttribute.name; | ||
o.pluginSource = trackedEntityAttribute.pluginSource; | ||
o.fields = new Map(); | ||
}); | ||
|
||
await trackedEntityAttribute.fieldMap.asyncForEach(async (field) => { | ||
const dataElement = await this.dataElementFactory.build(field); | ||
dataElement && element.addField(field.IdFromPlugin, dataElement); | ||
}); | ||
|
||
element && section.addElement(element); | ||
} else { | ||
const element = await this.dataElementFactory.build(trackedEntityAttribute); | ||
element && section.addElement(element); | ||
} |
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 is the same logic as in EnrollmentFactory.js
. Can it be extracted to a commonplace and reused?
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.
Looks really good @eirikhaugstulen 👍 Let's have a chat tomorrow.
src/core_modules/capture-core/components/D2Form/D2SectionFields.component.js
Outdated
Show resolved
Hide resolved
src/core_modules/capture-core/components/D2Form/D2SectionFields.component.js
Outdated
Show resolved
Hide resolved
src/core_modules/capture-ui/FormBuilder/FormBuilder.component.js
Outdated
Show resolved
Hide resolved
src/core_modules/capture-core/components/D2Form/DataEntryPlugin/DataEntryPlugin.container.js
Outdated
Show resolved
Hide resolved
src/core_modules/capture-core/components/D2Form/DataEntryPlugin/DataEntryPlugin.types.js
Outdated
Show resolved
Hide resolved
@@ -500,6 +531,31 @@ export class FormBuilder extends React.Component<Props> { | |||
}, []); | |||
} | |||
|
|||
renderPlugin = ( | |||
field: Object, |
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 believe the type can be improved to field: FieldConfig
if field.props
is checked for null or undefined below. WDYT? Thanks!
const { id: fieldId } = fieldMetadata; | ||
const { querySingleResource } = this.props; | ||
const validators = getValidators(fieldMetadata, querySingleResource); | ||
console.log('options', options); |
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.
Forgotten console
?
} | ||
|
||
return Object.entries(metadataByPluginId) | ||
.reduce((acc, metadata) => { |
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.
Hey,
This function has high complexity. Does it make sense to split it into smaller pieces of code?
Thank you!
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.
Just added a small comment on a detail, did not make a proper review.
package.json
Outdated
"@dhis2/app-runtime": "^3.10.0-alpha.2", | ||
"@dhis2/ui": "8.4.5", | ||
"@dhis2/d2-i18n": "^1.1.0", | ||
"@dhis2/cli-app-scripts": "^10.4.0-alpha.1", | ||
"@babel/preset-react": "7.16.7", | ||
"react-error-overlay": "6.0.9", | ||
"react-scripts": "4.0.3", | ||
"core-js": "2.5.7" |
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.
Just noticed that these entries should be sorted by alphabet
…_RenderDataEntryPlugins # Conflicts: # i18n/en.pot # package.json
We're reverting back to displaying an error message for now until the linked issues are merged into app-platform. Thanks for the reviews @simonadomnisoru & @JoakimSM. Should be fixed now. Will change back to plugin component in https://dhis2.atlassian.net/browse/DHIS2-15475. |
…_RenderDataEntryPlugins # Conflicts: # i18n/en.pot # packages/rules-engine/src/rulesEngine.types.js # src/core_modules/capture-core/components/DataEntries/TeiRegistrationEntry/TeiRegistrationEntry.component.js
🚀 Deployed on https://deploy-preview-3222--dhis2-capture.netlify.app |
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.
Tested successfully on 2.41 ,2.40.1,2.39.4,2.38.5 versions
…_RenderDataEntryPlugins # Conflicts: # package.json # src/core_modules/capture-core/components/DataEntries/common/TEIAndEnrollment/useMetadataForRegistrationForm/buildFunctions/buildEnrollmentForm.js # src/core_modules/capture-core/components/DataEntries/common/TEIAndEnrollment/useMetadataForRegistrationForm/buildFunctions/buildTrackedEntityTypeCollection.js # src/core_modules/capture-core/components/DataEntries/common/TEIAndEnrollment/useMetadataForRegistrationForm/hooks/useEnrollmentFormFoundation.js # src/core_modules/capture-core/components/DataEntries/common/TEIAndEnrollment/useMetadataForRegistrationForm/hooks/useTrackedEntityTypeCollection.js # src/core_modules/capture-core/components/DataEntries/common/TEIAndEnrollment/useMetadataForRegistrationForm/index.js # src/core_modules/capture-core/components/DataEntries/common/TEIAndEnrollment/useMetadataForRegistrationForm/useMetadataForRegistrationForm.js
🎉 This PR is included in version 100.35.0 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
No description provided.