(fix) O3-2627: Improvements to the visit notes image upload functionality

[jwnasambu]

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 conforms to the OpenMRS 3.0 Styleguide and design documentation.
• My work includes tests or is validated by existing tests.

Summary

I modified showImageCaptureModal function before setting the value of the image field to checks if the file type is 'image'. If it's not an image, to displays a Snackbar error message indicating that only image files are allowed.
It rejects the promise with an error if the file type is not an image, preventing the modal from closing and the value from being set and if the file type is an image, it proceeds to set the image field value and close the modal as before.

Screenshots

Related Issue

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

Other

[vasharma05]

Why are we even allowing all types of documents when we only need images?

[denniskigen]

I've tested the existing functionality on the refapp, and it looks like the validation is working as intended courtesy of work done in #1620, #1685, and #1584.

supported-file-extensions.mp4

So, this change should not be required. What could change, however, is:

• Modifying the allowedImageTypes array to prefix the file extensions with a leading dot so that you have an array like ['.jpeg', '.jpg', '.png', '.webp'].

• Modifying the isFileExtensionAllowed function to handle filenames that include dots. So it correctly extracts the file extension from a file name like "file....01.latest.png"

• Improving the error message shown when an upload doesn't match the supported file types from:

  

  to something like:

  

  which is a lot clearer.

• Improving the error message shown when an upload exceeds the file limit to include the file name:

  

[jwnasambu]

@denniskigen You are correct! I believe this PR should be closed unless there is something that needs fixing. Yesterday's internet was so poor that I couldn't see the Snackbar error message. Please forgive me.

[hadijahkyampeire]

@jwnasambu @vasharma05 I think in this case we want inline error notifications, not a snackbar, right? From what @denniskigen mentioned above, this is working as expected.

[mogoodrich]

Why are we even allowing all types of documents when we only need images?

I'm not familiar entirely with this functionality, but a generic attachments functionality is useful, to be able to upload things like reports in PDF format, etc.

[denniskigen]

@mogoodrich @vasharma05 it definitely makes sense to open this up to different file formats. This current implementation is guided by the design, specifically the Add Image button, which suggests that file uploads in the visit note form should be limited to images. This limitation is not enforced in the attachments app with the caveat that attachments recorded there aren’t linked to a visit note.

[ojwanganto]

@denniskigen it would be nice to understand the design requirements. It would be nice to open this to some common file formats unless there are clear reasons for not opening it up. I see a lot of use cases beyond just images though

[denniskigen]

Yeah, I'll seek Ciaran and Paul's feedback on this tomorrow @ojwanganto.

Update: Here's the slack thread https://openmrs.slack.com/archives/CKS32D55G/p1711010187179539.

[denniskigen]

I'm merging this as it meets the requirements in the ticket. I'll file a new ticket with the new requirement of uploading different file formats via the visit note form.

[mogoodrich]

Yeah, sorry, didn't mean to derail this ticket, just wanted to note that there is a significant use case for other kinds of uploads like PDFs.

[icrc-loliveira]

Hello @jwnasambu,
This PR makes the error messages lock a lot nicer, which I approve :)
However since you changed the configuration to require the dots, it broke our configuration.

• Old: 'jpeg', 'jpg', 'png', 'webp'
• New: '.jpeg', '.jpg', '.png', '.webp'

This configuration comes from the backend which for security reasons also validates the allowed extensions. And now if we set the global property attachments.allowedFileExtensions you will not be able to upload an attachment because of that difference.
PR: openmrs/openmrs-module-attachments#61

Do you have any reason to keep this change or can I revert it to the old one?

cc: @mogoodrich, @denniskigen, @ojwanganto, @hadijahkyampeire, @ibacher

[denniskigen]

Sorry about that, @icrc-loliveira. What does the request for the attachment.allowedFileExtensions GP return on your end?

[icrc-loliveira]

This is an example of the values that is expecting:

<globalProperty>
  <property>attachments.allowedFileExtensions</property>
  <value>txt,pdf,jpeg,jpg,png,gif</value>
</globalProperty>

[denniskigen]

@ibacher does the allowedFileExtensions GP allow arbitrary values? Can one specify file extensions prefixed with a .?

[ibacher]

@denniskigen All GPs are basically just string values. However, changing the value there would require updates to the attachments module code, since we're currently lopping the extension off separately.

@icrc-loliveira Would we want to allow all of those values on the visit notes image upload? That probably requires reworking the page a bit (since txt and pdf are definitely not images).

[denniskigen]

Please feel free to revert to the old convention, @icrc-loliveira. Sorry for the inconvenience.