search

dhis2 / aggregate-data-entry-app

Data entry app for DHIS 2
https://dhis2-data-entry.netlify.app
BSD 3-Clause "New" or "Revised" License
3 stars 3 forks source link

feat(DHIS2-17002): display custom text before and after a section #373

Closed kabaros closed 7 months ago

kabaros commented 7 months ago

This PR adds the ability to configure custom HTML text before and after a section. This implements DHIS2-17002

Implementation notes

text_before_after_section.webm

Context

This is part of an epic to provide native configurations options to data entry forms to reduce the necessity for custom forms. Users are currently building custom forms as a workaround for shortcomings of the configuration options (ability to transpose, or customise a cell design) or implementation (such to avoid issues with RTL issues). This is an RFC that describes the approach and the priorities for form configuration options. This is based on a thorough investigation by the functional design team for custom form use cases in real-life implementations. Based on that investigation, the ability to transpose forms and adding free-text descriptions were identified as the top functionalities to support.

dhis2-bot commented 7 months ago

🚀 Deployed on https://pr-373--dhis2-data-entry.netlify.app

kabaros commented 7 months ago

@michael-markevich this PR might have security implications. We're allowing users to add html content to display before and after a data entry form. I've used a library here that sanitises the HTML and only allows certain HTML tags (including links but it takes care of xss through href).

This sanitisation happens when displaying the form - I am wondering whether we should be actually sanitising the content when configuring it in the maintenance app, or is it enough to sanitise when displaying it? I am just wary of implicitly changing something people enter in the maintenance app.

michael-markevich commented 7 months ago

@kabaros As a general rule, it would be best to sanitize the input before saving it to ensure that the database doesn't contain untrusted data. Then, I would run the sanitizer again before displaying the data.

KaiVandivier commented 7 months ago

Also have a look at the display options types: https://github.com/dhis2/aggregate-data-entry-app/blob/72c96bf4e3c144e0d05908868a43a1c7f773e547/src/data-workspace/category-combo-table-body-pivoted/category-combo-table-body-pivoted.js#L149-L154

An object seems expected but a string is used, and the names of the before and after text seems different

(PS why is displayOptions a string instead of an object? Just curious) https://github.com/dhis2/aggregate-data-entry-app/blob/72c96bf4e3c144e0d05908868a43a1c7f773e547/src/data-workspace/section-form/section.js#L69-L70

kabaros commented 7 months ago

(PS why is displayOptions a string instead of an object? Just curious)

thanks for the review @KaiVandivier - displayOptions is a string because of a limitation of the backend hibernate mapping, where we'd need to either have it as a string, or map it into Java objects - discussion is here.

dhis2-bot commented 7 months ago

:tada: This PR is included in version 100.5.0 :tada:

The release is available on:

Your semantic-release bot :package::rocket: