refactor: update invoice registry to latest standards #1760
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jniles
force-pushed
the
refactor-invoice-registry-to-standards
branch
from
44df95e
to
dde37f8
Compare
|
Before this gets merged, I need to figure out how to show the grid footer on the same page. I think it's just a CSS class on the grid. 👍 |
|
This looks like a great refactor! From the feature review this morning there was only one comment:
|
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.
This commit migrates the client side of the Invoice Registry to use the FilterService. To do this, the logic is heavily changed in the Invoice Registry controller, removing the old DeprecatedFilterService and replacing it completely. The InvoiceService now manages caching filters. Additionally, the form is separated in two tabs with default filters and specialized filters. I've also taken the liberty to remove some style hacks and implemented a search-modal class in bhima-bootstrap.less. This class simply transplants the styles to a location where they can be shared. Progress towards IMA-WorldHealth#1749.
This commit migrates the cash modal search harness into a global search form harness. It is used by the Invoice Registry, updating the invoice registry's end to end tests.
This commit adds a dropdown menu with two links - the CSV downloader and the PDF downloader. Both of these use the server-side download at the moment. We could easily switch the CSV exporter to use the client-side exporter. It also removes the bhPrintButton in doing so. Partially addresses IMA-WorldHealth#1749.
This commit implements ui-grid's save-state feature on the invoice registry, and compliments it with a column configuration service. This service is slightly buggy, but that is outside the scope of these changes. Closes IMA-WorldHealth#1749.
This commit renders the registry footer visible on all screen sizes by default. It was accidentally nuked while introducing the new filters.
jniles
force-pushed
the
refactor-invoice-registry-to-standards
branch
from
dde37f8
to
5aeb188
Compare
|
@sfount, changes have been made. |
sfount
approved these changes
Jun 5, 2017
| <div class="toolbar-item"> | ||
| <div uib-dropdown dropdown-append-to-body data-action="open-menu"> | ||
| <a class="btn btn-default" uib-dropdown-toggle> | ||
| <span class="fa fa-bars"></span> <span class="hidden-xs" translate>FORM.LABELS.MENU</span> <span class="caret"></span> |
|
@bors r+ |
bors bot
added a commit
that referenced
this pull request
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**  **New Menu Demo**  **Download as CSV Option**  **Column Configuration Modal**  **Column Changes Saved in Save State**  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!
Build succeeded |
5 tasks
10 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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. If you would prefer me to remove it and us to ignore it, that's cool.I've removed thepackage-lock.jsonas it appears to cause issues with previous NPM versions.For reference of what changed, all components of #1749 were tackled here.
Code Changes
FilterParserimplementation on the server by turning offautoParseStatements(see (refactor) Deprecate automatic equals filters #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.FilterService.bhClear(see refactor: use bhClear for all grid search modals #1702).SearchModalservice, shared with the Cash Payments Registry. It provides simple control of the filters.<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.A Note on End to End Tests
I've combined the Cash Payment Registry's
SearchModalpage object with the Invoice Registry'sSearchModal. 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 parameterssearchQueries(as done in the Journal and Cash Payments registry) and your default search paramtersdefaultQueries.Screenshots
Pics or it didn't happen, right?
New User Interface
New Menu Demo
Download as CSV Option
Column Configuration Modal
Column Changes Saved in Save State
Closes #1749.
Thank you for contributing!
Before submitting this pull request, please verify that you have:
For a more detailed checklist, see the official review checklist 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!