Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(refactor) Deprecate automatic equals filters #1454

Closed
sfount opened this issue Mar 30, 2017 · 1 comment
Closed

(refactor) Deprecate automatic equals filters #1454

sfount opened this issue Mar 30, 2017 · 1 comment
Labels

Comments

@sfount
Copy link
Contributor

sfount commented Mar 30, 2017

The FilterParser service on the server currently will use ALL options that are passed from the client, if no rule or exception is provided it will try to run the equals method on the key and the value (table.[options.key] = options.value).

This behaviour was convenient when we had a small number of filters and it was easy to see that this was happening. Now that we have many different modules performing complex filters I propose that we remove this default functionality and require the programmer to specify each equals.

Benefits:

  • No 'magic' behaviours - if a filter is supported by a server side API it will have to be specified in the code
  • Simpler to pass additional options that are not necessarily required by filters, the client will not have to worry about these being matched automatically by the server

APIs that have to be updated:

  • Patient invoices
  • Cash payments
  • Vouchers
  • Patients
  • ...
jniles added a commit to jniles/bhima that referenced this issue Jun 2, 2017
This commit migrates the server-side code to use FilterParser without
autoParseStatements turned on.  It removes a test invalidated by this
change, as well as cleans up a few whitespace issues in other modules.

Partially addresses IMA-WorldHealth#1454.
jniles added a commit to jniles/bhima that referenced this issue Jun 5, 2017
This commit migrates the server-side code to use FilterParser without
autoParseStatements turned on.  It removes a test invalidated by this
change, as well as cleans up a few whitespace issues in other modules.

Partially addresses IMA-WorldHealth#1454.
jniles added a commit to jniles/bhima that referenced this issue Jun 5, 2017
This commit migrates the server-side code to use FilterParser without
autoParseStatements turned on.  It removes a test invalidated by this
change, as well as cleans up a few whitespace issues in other modules.

Partially addresses IMA-WorldHealth#1454.
bors bot added a commit that referenced this issue Jun 5, 2017
1760: refactor: update invoice registry to latest standards r=sfount
This PR updates the Invoice Registry to the latest standard.  It should help developers of other registries figure out how to update their registries as well.

~~**Note**: The 5K+ line change is because I've committed a package-lock.json file that comes by default with npm v5+.  You can find it in the [release notes](https://github.com/npm/npm/releases/tag/v5.0.0).  If you would prefer me to remove it and us to ignore it, that's cool.~~ I've removed the `package-lock.json` as it appears to cause issues with previous NPM versions.

For reference of what changed, all components of #1749 were tackled here.

#### Code Changes
 1. I've refined the `FilterParser` implementation on the server by turning off `autoParseStatements` (see #1454).  Every filter must be specified.  I've also changed cashReference comparison statement to convert the binary uuid in JS, since this is what most other modules do.
 2. I've migrated the filters to using the client `FilterService`.
 3. I've ensured the filter modal to latest standards.  This includes using `bhClear` (see #1702).
 4. The end-to-end tests have been updated to reflect these changes.  They now use a combined `SearchModal` service, shared with the Cash Payments Registry.  It provides simple control of the filters.
 5. Two options for downloading have been added: PDF and CSV.  For uniformity, they both use the server to serve files to an HTML `<a href download>` tag.  If needed, I can try to implement the CSV rendering with the Exporter service ... but I worry about the difference in UX.  I'll look to review for guidance.
 6. I've implemented the GridState service on this grid.  It works as the Journal's works.
 7. I've implemented the Grid Column service.  It also uses the GridState as described above... however, there is a bug in the Grid Column service reported in #1759.

#### A Note on End to End Tests
I've combined the Cash Payment Registry's `SearchModal` page object with the Invoice Registry's `SearchModal`.  Basically, any search form should be able to leverage this page object without writing their own massive test suite.  It prefers convention over configuration though, so you'll have to name your controllers `$ctrl`, your search parameters `searchQueries` (as done in the Journal and Cash Payments registry) and your default search paramters `defaultQueries`.

#### Screenshots
Pics or it didn't happen, right?

**New User Interface**
![invoiceregistrynewui](https://cloud.githubusercontent.com/assets/896472/26746668/ef0bcfa8-47e8-11e7-8bd7-819f9a4e1dea.png)

**New Menu Demo**
![invoiceregistrydropdownmenu](https://cloud.githubusercontent.com/assets/896472/26746674/f928966a-47e8-11e7-9026-2fe054b33142.png)

**Download as CSV Option**
![invoiceregistrydownloadascsv png](https://cloud.githubusercontent.com/assets/896472/26746727/42f4b8e6-47e9-11e7-8e63-fabd55f071fa.png)

**Column Configuration Modal**
![invoiceregistrycolumnconfigurationmodal](https://cloud.githubusercontent.com/assets/896472/26746763/7fb6e542-47e9-11e7-8669-2eb8536517f8.png)

**Column Changes Saved in Save State**
![invoiceregistrycolumnconfigurationsaved](https://cloud.githubusercontent.com/assets/896472/26746774/9109f7da-47e9-11e7-98b6-d01faf5ca512.png)

Closes #1749.

----
Thank you for contributing!

Before submitting this pull request, please verify that you have:
 - [x] Run your code through ESLint.  [Check out our styleguide](https://github.com/IMA-WorldHealth/bhima-2.X/blob/master/docs/STYLEGUIDE.md).
 - [x] Run integration tests.
 - [x] Run end-to-end tests.
 - [x] Accurately described the changes your are making in this pull request.

For a more detailed checklist, [see the official review checklist](https://docs.google.com/document/d/1nupLVLRXgSZJQo_acLgrwvPnN8RukfSiwRhSToj81uU/pub) that this PR will be evaluated against.

Ensuring that the above checkboxes are completed will help speed the review process and help build a stronger application.  Thanks!
@jniles
Copy link
Contributor

jniles commented Jul 17, 2017

Closing this since it has been in master for over a month at this point. We can always reference this later.

@jniles jniles closed this as completed Jul 17, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants