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

fix(stock): improve movements registry #2074

Merged

Conversation

jniles
Copy link
Contributor

@jniles jniles commented Sep 2, 2017

This commit fixes a number of bugs on the movements registry.

  1. The input/output search parameter now actually filters correctly.
  2. The "reason" ui-select actually filters the registry.
  3. The "actions" column does not present a dropdown on header rows.
  4. The date and unit cost columns sort properly.
  5. The grid footer no longer gives untranslated english text in the
    footer.

It also improves the performance of the registry by pre-computing costs
in the same way that it pre-computes the flux mappings.

Closes #2073. Closes #2072.


Thank you for contributing!

Before submitting this pull request, please verify that you have:

  • Run your code through eslint.
  • Run integration tests.
  • Run end-to-end tests.
  • Accurately described the changes your are making in this pull request.

For a more detailed checklist, see the online 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!

@jniles jniles force-pushed the fix-stock-reason-search-parameter branch 3 times, most recently from f2ec4e8 to 108c4d1 Compare September 2, 2017 16:17
@jniles jniles added the Bug Fix label Sep 2, 2017
@DedrickEnc
Copy link
Contributor

@jniles
This P.R needs a rebase and then after I will do the review.

This commit fixes a number of bugs on the movements registry.
 1. The input/output search parameter now actually filters correctly.
 2. The "reason" ui-select actually filters the registry.
 3. The "actions" column does not present a dropdown on header rows.
 4. The date and unit cost columns sort properly.
 5. The grid footer no longer gives untranslated english text in the
 footer.

It also improves the performance of the registry by pre-computing costs
in the same way that it pre-computes the flux mappings.

Closes IMA-WorldHealth#2073.  Closes IMA-WorldHealth#2072.
@jniles jniles force-pushed the fix-stock-reason-search-parameter branch from 108c4d1 to b4af85d Compare September 4, 2017 13:57
@jniles
Copy link
Contributor Author

jniles commented Sep 4, 2017

@DedrickEnc, done.

Copy link
Contributor

@DedrickEnc DedrickEnc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The cleaning in this P.R is helpful, I am okay to pull it on master.
LGTM

<span translate>STOCK.INPUT</span>
</label>
</div>
<div class="radio">
<label>
<input type="radio" name="is_exit" value="0" ng-model="$ctrl.searchQueries.is_exit">
<input type="radio" name="is_exit" value="1" ng-model="$ctrl.searchQueries.is_exit">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Good catch

<ui-select name="inventory" ng-model="$ctrl.searchQueries.flux_id">
<ui-select-match>
<span translate>{{$select.selected.label}}</span>
</ui-select-match>
<ui-select-choices ui-select-focus-patch repeat="flux as flux in $ctrl.fluxes | filter:{ 'label': $select.search }">
<ui-select-choices ui-select-focus-patch repeat="flux.id as flux in $ctrl.fluxes | filter:{ 'label': $select.search }">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@@ -1,4 +1,4 @@
<div class="ui-grid-cell-contents text-action" uib-dropdown dropdown-append-to-body uib-dropdown-toggle>
<div class="ui-grid-cell-contents text-action" uib-dropdown dropdown-append-to-body uib-dropdown-toggle ng-if="!row.groupHeader">
Copy link
Contributor

@DedrickEnc DedrickEnc Sep 4, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As you suggested in the email, this enhancement should be done in every registry with grouping option. So an issue should be better.

@DedrickEnc
Copy link
Contributor

bors r+

bors bot added a commit that referenced this pull request Sep 4, 2017
2074: fix(stock): improve movements registry r=DedrickEnc a=jniles

This commit fixes a number of bugs on the movements registry.
 1. The input/output search parameter now actually filters correctly.
 2. The "reason" ui-select actually filters the registry.
 3. The "actions" column does not present a dropdown on header rows.
 4. The date and unit cost columns sort properly.
 5. The grid footer no longer gives untranslated english text in the
 footer.

It also improves the performance of the registry by pre-computing costs
in the same way that it pre-computes the flux mappings.

Closes #2073.  Closes #2072.

---
@bors
Copy link
Contributor

bors bot commented Sep 4, 2017

Build failed

@jniles
Copy link
Contributor Author

jniles commented Sep 5, 2017

bors r+

bors bot added a commit that referenced this pull request Sep 5, 2017
2074: fix(stock): improve movements registry r=jniles a=jniles

This commit fixes a number of bugs on the movements registry.
 1. The input/output search parameter now actually filters correctly.
 2. The "reason" ui-select actually filters the registry.
 3. The "actions" column does not present a dropdown on header rows.
 4. The date and unit cost columns sort properly.
 5. The grid footer no longer gives untranslated english text in the
 footer.

It also improves the performance of the registry by pre-computing costs
in the same way that it pre-computes the flux mappings.

Closes #2073.  Closes #2072.

---
@bors
Copy link
Contributor

bors bot commented Sep 5, 2017

Build succeeded

@bors bors bot merged commit b4af85d into IMA-WorldHealth:master Sep 5, 2017
@jniles jniles deleted the fix-stock-reason-search-parameter branch September 6, 2017 19:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants