feat(#566): implement auto-inclusion of task and target and contact s…

[inromualdo]

Description

Ability to compile and upload tasks and contact summary from reusable componnent. This PR auto include any file end by .tasks.js, .targets.js and .contact-summary.js

#566

Code review items

• Readable: Concise, well named, follows the style guide, documented if necessary.
• Documented: Configuration and user documentation on cht-docs
• Tested: Unit and/or integration tests where appropriate
• Backwards compatible: Works with existing data and configuration. Any breaking changes documented in the release notes.

License

The software is provided under AGPL-3.0. Contributions to this project are accepted under the same license.

[jkuester]

@inromualdo sorry for the delay! Very interested in the feature you have added here!

Before I get too far into reviewing the code, can you help me understand a bit better how this is expected to work? Here is how I think things go and maybe you can correct me if I get anything wrong:

• Currently tasks.js and targets.js each export an array. The PR would basically allow any file ending in .tasks.js or .targets.js to export an array and then entries from all the arrays will be concated together when webpacking tasks/targets for the app_settings.json.
• Similar functionality is true for the contact-summary where the exported cards and fields arrays are concated and the exported context objects are merged using Object.assign.
• Basically all this allows for dynamically extending the configuration by adding additional files without having to edit the base files.

There are some tradeoffs here between clarity and convenience. (There is more complexity when we are merging files behind the scenes and it adds another level of abstraction between what is seen in the config files and what is actually uploaded to the server. On the other hand, making configuration more reusable has clear benefits as well.)

I think I see how this works for just dropping new files into the project directory. How will this work for config coming from re-usable NPM libs like https://github.com/medic/cht-stock-monitoring-workflow? Would I still have to manually copy files from that repo into my local config?

[inromualdo]

You get everything right here @jkuester

re-usable NPM libs are supposed to copy their files tasks, targets etc... directly in the project folder. we have a v2 for stock monitoring that dors that.

[jkuester]

Okay, thanks for your patience @inromualdo! I have been doing a ton more thinking about this and digging through the existing code along with your changes here. I think this is a really exciting opportunity to step into a new era of CHT configuration. What you have here does a good job being passive and minimal in terms of the changes it envisions. Instead, I would like to propose we go all-in on this and expand our approach to just set a new standard! To summarize, I am proposing:

1. We remove all remaining support for "non-declarative" configuration for tasks/targets/contact-summary.
2. Add support for dynamically reading configuration from files in the ./contact-summary, ./tasks, and ./targets directories (using directories to group the files instead of your current approach with the .contact-summary/tasks/targets.js prefix).
   • Deprecate the contact-summary.templated.js, tasks.js, and targets.js files, but retain support for reading config from them for backwards compatibility.
3. Add support for not including contact-summary/tasks/targets configuration in a project (just default to empty config instead).

With this approach, a typical basic configuration structure would look something like:

• app_settings/
  • base_settings.json
• contact-summary/
  • base.js
• targets/
  • base.js
• tasks/
  • base.js

You can expand the contact-summary/tasks/targets configuration by adding additional files to the associated directory. Running the compile-app-settings action will pull everything together into the ./app_settings.json file.

What do you think? I am trying to actually reduce complexity by just standardizing along the intended go-forwards code paths. Do you think this will be useful/maintainable? Do you have any ideas on how to improve this (or achieve the same objective more cleanly)?

---

I don't think implementing what I am going for here would take a huge amount of additional effort. You have already done the most difficult design work of figuring out the details for merging the config. After poking around the code a bit, here is my thoughts on the implementation details, but we might be able to come up with a better approach....

• contact-summary:
  • Remove support for "freeform" contact summary configuration (aka the contact-summary.js file). Only keep support for "structured" (declarative) contact-summary config.
    • Going forwards, we will only support reading the declarative contact summary config from contact-summary.templated.js file and contact-summary/*.js files.
  • Move the contact-summary merge functionality out of contact-summary/lib.js into contact-summary-emitter.js so that it takes an array of contactSummaries instead of the current contactSummary parameter.
  • Remove the contact-summary/lib.js file and replace it with a file generated at compile-time in the compile-contact-summary.js logic (similar to cht-cards-extensions-shim.js).
    • The generated file should only:
      • Require the contact-summary.templated.js file and all the various contact-summary/*.js files,
      • Use the emitter to emit the contact summary config.
    • The logic should just treat the contact-summary.templated.js like the other contact-summary/*.js files, but it should be the "most preferred".
      • Also can log a message that contact-summary.templated.js is deprecated and should switch to contact-summary/base.js
  • Add support for not having any contact-summary.templated.js file and contact-summary/*.js files (and just adding empty config into the app_settings)
• tasks/targets:
  • Remove support for rules.nools.js config (nools). Only keep support for declarative config.
    • Going forwards we will only support reading the declarative config from tasks.js, targets.js, tasks/*.js, and targets/*.js files.
  • Remove the nools/lib.js and replace it with a file generated at compile-time in the compile-tasks-and-targets.js logic (similar to how the current *extensions-shim.js files are being generated).
    • The generated file should only:
      • Require the tasks.js, targets.js, tasks/*.js, and targets/*.js files.
      • Use the emitters to emit the tasks and targets.
    • The logic should treat the tasks.js and targets.js like the other tasks/*.js, and targets/*.js files, but the tasks.js and targets.js should be "most preferred".
      • Also can log a message that tasks.js and targets.js are deprecated and should switch to tasks/base.js, and targets/base.js
  • Add support for not having any tasks.js, targets.js, tasks/*.js, and targets/*.js files (and just adding empty config into the app_settings)