Refactor webapp Enketo service into shared-libs

[jkuester]

What feature do you want to improve? Support cht-core@^1.14, in https://github.com/medic/cht-conf-test-harness requires updating the enketo-core version used by the test harness and the surrounding harness code that is used to bootstrap the Enketo forms. Much of the logic needed (in the test harness form-host) for bootstrapping enketo matches what we are already doing in cht-core currently in the webapp enketo.service.

Going forwards, as we continue to update the version of enketo-core used by the CHT, it will require extra effort to ensure the logic for bootstrapping Enketo remains consistent before cht-core and cht-conf-test-harness.

Describe the improvement you'd like Much of this ongoing maintenance work could be removed or at least simplified by moving as much of the core logic as possible from the enketo.service into a new shared-libs folder that could also be consumed by the cht-conf-test-harness.

Describe alternatives you've considered Definitely open to other approaches that might be better for addressing this issue, but this feels like the most straightforward direction that is also inline with patterns we have followed in the past.

Additional context A proof-of-concept refactoring can be seen in this commit where I tried to just decouple the logic from the Angular dependencies. Ideally it would probably make sense to have this refactoring be more sophisticated and make it easier to consume.

[mrjones-plip]

@jkuester - is this dependent on the uplift in #6345 such that it should be 4.0.0?

[jkuester]

Yes, good call! I have updated the milestone.

[jkuester]

For the record, after discussing with @garethbowen we have decided not to put the reusable code into shared-libs since the code is not actually going to be reused within cht-core components (it is still only used by webapp).

The current approach in https://github.com/medic/cht-core/pull/7567 is to just refactor the code into webapp/src/js/enketo. This is sufficient to allow for reusing this code, but the big downside of this approach is that it remains less clear what the "interfaces" are that we expect the test harness to use and increases the risk that something changes in the cht-core code and inadvertently breaks the test harness (or just makes unnecessary work when it comes to update the cht-core version in the harness).

Ideally it would be great to have an entirely separate NPM dependency that could be consumed by both cht-core and cht-conf-test-harness that would contain all the resources needed by the test harness (so that it would not need to depend on cht-core at all). However, given the large amount of various files that the test harness is currently using from cht-core, creating such a library is going to be more complex than we want to get into at this time.

[garethbowen]

As discussed, not a blocker for 4.0.0.

[jkuester]

I think there are two different possibilities for addressing this issue:

• Angular Element
  • Pros: You get a js file at the end of the process that can be easily pulled into the test harness build without needing to refactor the harness too much (in theory)
  • Cons: Does not seem like Elements are really designed to be distributed as stand-alone npm packages. This seems odd to me and makes me wonder if I am missing something fundamental about Elements. :thinking:
• Angular Library
  • Pros: Clearly designed for re-usability as a stand-alone package.
  • Cons: Advertised as only for consumption by other Angular applications. May require refactoring the test harness to include Angular. Though, it is notable that ng-packagr states that the resulting "npm package can be consumed by . . . Webpack"

More investigation is necessary here, but my current opinion is that the Angular Library may be the best choice for long-term maintainability (though, of course, at a higher up-front cost). It would seem to involve the least amount of "hacking" and ideally the modular Angular framework could hopefully provide a clean way for the test harness to stub in mock implementations of the required services....

[jkuester]

Okay, so after a day of investigation and tinkering, I think that the Angular Element approach is promising!

Any solution to this issue is going to have to deal with the fact that all the code around the EnketoService is a mess of business logic interleaved with Angular framework code and tightly coupled to a ton of components/services. There is no clean way to split this code out and doing so would be a massive refactoring effort with dubious benefits. (Other than the test harness, there is not a clear need for the Enketo code to be shareable...)

In https://github.com/medic/cht-core/tree/7462_forms_module I have started working on creating a cht-form Angular Element that can be built into a Web Component. This Web Component is essentially JS code + HTML + CSS that can be pulled into any other web page/app. This Angular Element is basically a separate sub-application within webapp with its own tsconfig.app.json configuration.

There are a couple of options for structuring the webapp code here. One way would be to move all of the Enketo-related code into the cht-form sub-project and then have the main webapp code depend on cht-form. The problem with this is that, as was previously mentioned, the EnketoService has a ton of dependencies and cannot be cleanly extracted without taking a bunch of stuff with it.

Another option (and the one I am currently trying in 7462_forms_module) is to instead leave all the existing code in place in the main webapp and simply add a new component into cht-form (a cht-form component, if you will). This component will be specifically for external use (at least for now) and will not be consumed by other webapp code. Instead, it will simply link over to the necessary webapp code like EnketoService to include when packaging the "Web Component" for external use. One of the benefits of this approach is that certain dependencies of EnketoService can, in theory, be "extracted" (my current approach has been to overwrite things the compilerOptions.path in the tsconfig.app.json to point to "stub" implementations of a particular service, but perhaps more sophisticated injection is possible....). A pure-js "Web Component" that allows for injecting implementations of various dependencies, while still retaining all of the Enketo-related business logic is pretty much exactly what we need for the test harness!

Objectively, it feels like the first option is better, but the reality of our current situation might make the second proposal (leaving all the code in webapp) better just from a practical perspective. Then we can gradually work towards de-coupling the Enketo logic so that it would be feasible for the webapp code to actually consume the cht-form (and we could centralized the enketo-related logic....)

[jkuester]

Just an additional note on next steps here: I do not think we have enough information yet to put together a proper tech design for solving this. However, I believe I have been able to demonstrate the viability of the Angular Element approach and that it has basic principles necessary to solve the problem! To proceed, I want to confirm stakeholder buy-in for an Angular Element/Web Component based approach. Will try to schedule a brief meeting with @kennsippell and @garethbowen to discuss.

[jkuester]

As I look at carving out code into a reusable component, one thing I have to determine is what code to include in the component and what to stub out. We need to supply all the dependencies of the EnketoService either by pulling in the proper service code, or by stubbing the service (to allow for shimming/mocking). Until we do a proper refactor that would allow webapp to depend on the component, this dependency tree will be a bit of a moving target and may change with future updates to the code.

To help determine which code should be stubbed and which services can actually be depended on by the cht-form component, I have put together this helpful graph:

• Nodes with rounded edges represent the services I intend to stub
• Any transitive dependencies only referenced via dotted lines are irrelevant to the cht-form component since they will not actually be dependencies

flowchart LR

subgraph "Direct EnketoService Depenencies"
	attachment
    cht-script-api([cht-script-api])
    contact-summary([contact-summary])
    db([db])
    enekto-prepopulation-data
    enketo-translation
    extract-lineage
    feedback([feedback])
    file-reader
    get-report-content
    language([language])
    lineage-model-generator
    search([search])
    submit-form-by-sms([submit-form-by-sms])
    training-cards([training-cards])
    transitions([transitions])
    translate
    translate-from
    user-contact
    xml-forms([xml-forms])
    z-score([z-score])
    
    get-report-content --> db & file-reader
    lineage-model-generator --> db
end

subgraph "Transitive Dependencies"
auth
changes
contact-types
debug
feedback
form2sms
format-date
get-data-records
location
pipes
route-snapshot
session
settings
telemetry
uhc-settings
uhc-stats
user-settings([user-settings])
validation
version
xml-forms-context-utils
end

db -.-> session & location
enekto-prepopulation-data --> language & enketo-translation & user-settings
language -.-> settings & format-date & telemetry
search -.-> db & session & get-data-records & telemetry
submit-form-by-sms -.-> form2sms & settings
user-contact --> user-settings & lineage-model-generator & db
xml-forms -.-> auth & changes & contact-types & db & file-reader & user-contact & xml-forms-context-utils & feedback
z-score -.-> changes & db
contact-summary -.-> settings & pipes & feedback & uhc-settings & uhc-stats & cht-script-api
transitions -.-> settings & validation
feedback -.-> db & session & version & debug & language & telemetry
cht-script-api -.-> settings & changes & session
training-cards -.-> xml-forms & db & session & route-snapshot & feedback

[jkuester]

Technical Design

Terminology note

For the purposes of discussing this functionality, I believe the terms "Angular Element", "web component", and "custom element" are roughly equivalent. However:

• A "custom element" is technically a subset of the "Web Components" standard. Since too many "element"s gets confusing, I have tried to use "web component" when referring specifically to the js/html code built from the Angular code/config.
• I try to use "Angular Element" when referring to the specific custom Angular code/config needed for building the web component.

General Approach

• Use the Angular Element functionality to build a custom web component from a subset of our existing webapp code.
  • The web component should encapsulate the functionality provided by the EneketoService (and some of its direct dependencies) to provide consumers the ability to display ODK forms in an environment that is basically identical to opening the form in cht-core.
• Should not involve any major refactoring of our current webapp code. Instead the Angular Element can be designed to reference the EnektoService and other necessary code in-place.
  • The EnektoService is, of course, dependent on some services that cannot be included in the web component (e.g. the DbService since that would introduce a dependency on PouchDB). These services will be replaced in the Angular Element config with stub implementations.
• The web component will only be responsible for displaying a form given the inputs of the form's transformed HTML and model XML configuration. It will not (at least at this time) include functionality for transforming an xform configuration into the HTML/model. Currently that logic resides in the cht-core api code and other options need to be explored for making that reusable.
  • Outside of this caveat, the js/css code provided in the web component should be sufficient for displaying and interacting with an ODK form in a browser without the consumer needing to reference any other code from cht-core
• The primary consumer of this web component is the cht-conf-test-harness, but the component should be generic enough to be useful in a number of other cases (e.g. the project-explorer could use it for displaying forms)
• Sufficient automated tests should be included for the web component to validate it functions as expected and to alert developers if changes to webapp code have broken the web component
• We should be able to publish the built web component as an NPM library for ease of distribution.

Details

• Add an additional project entry to webapp/angular.jsonfor building a new Angular Element in webapp/web-components/cht-form
• cht-form:
  • Has its own tsconfig that extends webapp/tsconfig.base.json
    • This tsconfig should override compilerOptions.paths to inject the necessary stub services:
  • The following services will need stubbed implementation in cht-form:
    • cht-script-api
    • contact-summary
    • db
    • feedback
    • language
    • search
    • submit-form-by-sms
    • training-cards
    • transitions
    • user-settings
    • xml-forms
    • z-score
  • For these stubbed services, the goal is to include the minimum necessary code required for form interactions. Can look at existing behavior in the cht-conf-test-harness as an example of reasonable mock behavior for many of these services (at least that is a good place to start, but possibly should be generalized from there....)
  • The styles.less file imports any necessary styles from webapp (e.g. src/css/enketo/medic.lss)
  • The main.ts will bootstrap the module and perform other necessary initialization steps:
    • Setup stub for window.CHTCore
  • The app.module.ts will create the custom cht-form element
  • The app.component.html and app.component.ts are going to be similar to webapp/src/ts/components/enketo
    • Additionally, the app.component.ts should:
      • Accept as @Input fields any necessary input from the consumer of the web component as HTML attributes (e.g. the form html/model).
        • The HarnessInputs from the cht-conf-test-harness probably represent a good base-level set of inputs to accept here as well
      • Trigger the EnektoService to render the form in the ngAfterViewInit function

[jkuester]

@garethbowen @dianabarsan @kennsippell @m5r I have been successful in implementing a rough hard-coded prototype of this Angular Element functionality in https://github.com/medic/cht-core/tree/7462_forms_module (demonstrating the feasibility of encapsulating the Enekto logic in a web component). Based on this effort, I have compiled an initial Technical Design in the comment above. I would love your thoughts/feedback on this design! (Hoping to have any major blockers called out before making it to code review!)

cc @mrjones-plip

[mrjones-plip]

Thanks for taking the time to write all this up and to solicit feedback! :pray:

[garethbowen]

Very impressive, thanks @jkuester

My main concern with the design is the amount of stubbing required. This makes the API very complicated and increases the likelihood of accidentally breaking the compatibility with the harness and other users. If, for example, the CHT changed the API of the feedback service, then all the consumers of this Angular Element would have to update their stubs to match. The list of services required should be reduced as much as possible. This increases the up front effort, but I think will make it much easier to support in the future.

There are a bunch of services that are only needed on setup (eg: Language service, script api, zscore, contact summary, search) - if the CHT called these and then passed the data in as params that would reduce a bunch of service level dependencies.

The feedback service shouldn't be necessary if we just throw the errors from the component and catch and call feedback from the CHT.

I think the DB service can be removed in the same way - do some work to check permissions beforehand, then just return the doc to save/update.

Feel free to defer or push back if this is too much work for now, but I think it'll help improve the maintainability of this element as well as the readabilty of the CHT code.

[jkuester]

Your feedback is well received @garethbowen! One clarification I want to make is that, regarding the stubbed services, my thought was that the implementation of those stubbed services would be handled in the Angular Element itself. That implementation would, of course, need to depend on input from the consumer via some formal input parameters in the web component, but the overall structure of the stubbed services would be completely abstracted away from the consumers of the web component. This way, when we inevitably make changes to the services (e.g. add a new service that the EnketoServices depends on) this will cause the build to fail for the Angular Element in cht-core (as soon as the code changes are made in the webapp). In many cases, when this build fails for the Angular Element, we should be able to fix the element code without requiring any changes to the interface of the web component, itself (and so without being noticeable to the consumers of the web component). I think doing it this way will result in the most maintainable solution by keeping the required changes between webapp and the Angular Element tightly coupled.

That being said, maintaining the stubs in the Angular Element will be the responsibility of cht-core developers and the larger that shared surface area is, the more work it is going to be to keep everything synced up (and the harder it will be to refactor anything). So, I absolutely agree that minimizing the services we need to stub is going to have significant long-term benefits! Before going further with this Angular Element, I will start by trying to reduce the number of services we need to stub by finding any low-hanging fruit that we can easily refactor out of the EnketoService.

[mrjones-plip]

@jkuester - I haven't full kept up with this ticket's details so my question might be moot, but with #8455 merged, can we close this ticket?

[jkuester]

@mrjones-plip sorry for the late response! No, we don't actually want to close this yet since this issue is mainly for the development of the actual Web Component itself. Those changes will be coming in a different PR soon!

[mrjones-plip]

Gotcha - thanks!

[jkuester]

@tatilepizs here are the primary test cases that I think we want to make sure we get covered in the web component tests so we are properly validating the behavior of the component. (A lot of these are probably already covered in the forms you are already testing, but I have just added them all here for completeness.)

• Setting contactSummary parameter to provide instance('contact-summary')/context data in the form.
  • E.g. the delivery form is reading the pregnancy_uuid value from the contact summary and using it to populate data/meta/__pregnancy_uuid
• Setting the content.contact parameter to provide inputs/contact data to the form.
  • E.g. the delivery form is reading inputs/contact/name to populate patient_name.
• Test the form "edit" workflow by filling out a form once. Then, take the fields data from the resulting doc and provide it as the content parameter for a new form. Now the form should be pre-populated with data from the first time. Edit the data and submit the form again.
• Setting the user.language parameter should change the language/translation used for the form
• Setting the user parameter to provide inputs/user data to the form
• Verify that when the "Cancel" button is pressed, the "Form Canceled" header is displayed as expected

In the test code, these attribute values should be set after loading the url in the browser, but before starting to fill out the form:

    await browser.execute(() => {
      const myForm = document.getElementById('myform');
      myForm.content = { contact: { name: "My Test Patient" } };
    });

[garethbowen]

@jkuester Would you mind also adding the new npm run commands to the documentation?

[jkuester]

@garethbowen Done! https://github.com/medic/cht-docs/pull/1232