...
UI Extension Patterns Follow Up
Previous strategy was to think we should focus on big-sky, extend anything.
Newer strategic thinking is to be more specific about where extension should occur. This is a learned piece from the amount of code that had to be forked and maintained in Malawi.
Current strategy: Add extension points where Malawi made forks
- Pub/Sub application wide events
- want to break apart large event logic (e.g. app wide login)
- if it happens everywhere, fire an event
- Decorate service/factory endpoints
- if it has logic, wrap it in a factory - avoid use in controllers and directives
- openlmis-download
- ui-component creates base functionality
- auth-ui adds authentication token
- Generated documentation
- facility-service-cache
- program-service
- Add new functionality to a service
- Avoid business logic in Directives or Controllers
- Noticed that using a decorating pattern in directives / controllers turns into a mess
- Move calculations / business logic to factories or services
- openlmis-download in repo openlmis-ui-components has an example of a factory holding business logic (the logic is to open the given URL in a new window)
- Looking at the download factory, decorated in the auth-ui: https://github.com/OpenLMIS/openlmis-auth-ui/blob/85a0eecd80774ddce9e0a252148333124fc22b21/src/openlmis-download-auth/openlmis-download.factory.decorator.js
- Since this is auth, this takes that generic download factory, and adds the OAuth2 authorization token to OpenLMIS links
- Could it be moved into the directive?
- Perhaps not, as the directive is the piece that works directly with the DOM.
- Have had a need to extend business logic, have not yet seen the need to extend a directive.
- How do we document where extensions are?
- add "Decorator" to the name, so it's findable, eg. http://build.openlmis.org/job/OpenLMIS-reference-ui/lastSuccessfulBuild/artifact/build/docs/index.html#/api/openlmis-download-auth.openlmisDownloadFactory
- might try to add an "implementations" section like javadocs has
Code Review
Code review documents
Recently some reviews have been:
- too large. Lets reject reviews that are over a few files or a couple hundred lines
- too many people, not clarity who's responsible. Lets assign 1 person (others can view, but one is assigned).
This issue has been creeping up for a while
Some reviews are very old
We have not modified our code review process for 9 months, this might be a good time to update that review process.
- 1 person should be assigned and responsible for the ticket in review
- Use the review description field to focus the reviewer about what they are reading
UI code reviews are done for most things, but some small tweaks were missed