(feat) Dynamic Offline Data for Forms | Offline Support for Concept Labels #710
Conversation
| const expectedUrls = await getCacheableFormUrls(identifier); | ||
| const absoluteExpectedUrls = expectedUrls.map((url) => window.origin + makeUrl(url)); | ||
| const cache = await caches.open('omrs-spa-cache-v1'); | ||
| const keys = (await cache.keys()).map((key) => key.url); | ||
| return absoluteExpectedUrls.every((url) => keys.includes(url)); |
There was a problem hiding this comment.
I am honestly torn about whether we should extract this logic here into a utility function in core. It is more or less the same code in every instance where it occurs (at the moment - it doesn't necessarily have to stay that way). I think it is duplicated around 5 times right now.
The reason for not doing it at the moment is that not moving it into core means that we don't have to commit to a new API that needs to be supported in the long run. Maybe it makes sense to wait and see how this develops and then, at some point, we can move it to core? Any thoughts?
There was a problem hiding this comment.
So my usually threshold for this is that if I've copied and pasted a snippet (or written something close enough) 3 times, its time to move it into a proper function. We can even do this without necessarily committing to an API by marking the function as @internal.
In any case, at some point, either the scheme contained in these lines is the de facto standard and having it implemented as a function means that its easier to make sure that others implement it correctly or we discover a need to change the scheme and its much easier if we only need to update it in one place.
There was a problem hiding this comment.
[...] 3 times, its time to move it into a proper function [...]
That sounds like a good approach. I think in this specific case, I'd like to delay it a little bit to move things forward, but I'll keep it on my todo list (and perhaps even think about a proper way to do this - functions for cache interaction would generally be a nice addition to core, maybe even without an @internal modifier).
… service function.
… into feature/offline-forms-dynamic-data
… into feature/offline-forms-dynamic-data
packages/esm-form-entry-app/src/app/form-schema/form-schema.service.ts
Outdated
Show resolved
Hide resolved
Requirements
Summary
This builds upon the changes in this PR. It migrates the patient chart towards the new dynamic data API. Notably, these areas have been updated:
In addition, since it builds upon the new dynamic data API, I integrated offline support for the Ampath form concept labels which have been introduced in this commit a while ago. This should allow forms using concept labels to properly display offline.
Related Issue
https://issues.openmrs.org/browse/O3-1110
https://issues.openmrs.org/browse/O3-1293