Refactor Stock distribution #1905

DedrickEnc · 2017-07-29T22:55:35Z

This PR refactor the stock distribution page and closes #1854

Most of the work is about addressing points listed in #1854 and some refactor in order to reduce the code.
A new component bhStockEntryExitType has been introduced to remove repeated code found in exit.html and entry.html for selecting the type of entry or exit.

The main change is about stock distribution from depot to an other depot (Stock transfer)
Now the stock transfer is a set of two processes which are a stock exit to depot and a stock entry from depot. Both of them are independent in other to avoid mistakes and guaranty the data integrity.
Each time the stock in the depot will match the stock in the system.

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!

DedrickEnc · 2017-07-31T09:45:59Z

@mbayopanda @sfount
Can I get a review?

DedrickEnc · 2017-08-04T12:04:44Z

@IMA-WorldHealth/bhima-core
Can I get a review for this pull request?

DedrickEnc · 2017-08-14T11:18:02Z

This PR is a refactor based on the functionality review we made, so it should be reviewed.
Can I get a review for this pull request ?
@IMA-WorldHealth/bhima-core

jniles · 2017-08-14T11:19:07Z

@DedrickEnc, I can give you a review. Can you rebase to master first?

DedrickEnc · 2017-08-14T11:24:58Z

Thanks @jniles , doing that

DedrickEnc · 2017-08-15T11:21:06Z

@jniles
The branch is rebased

jniles

@DedrickEnc, I've done an initial review. There's a few broken pieces and a few optimizations to make.

Let me know if you have any questions.

jniles · 2017-08-02T20:09:49Z

client/src/i18n/fr/depot.json

+        "DESCRIPTION": "Ce module vous permet de Lister, Creer, Modifier et Supprimer un dépôt",
+        "EDIT_DEPOT": "Editer les informations d'un dépôt",
+        "ENTITY" : "Depot",


DEPOT.ENTITY needs to be translated into English.

I haven't checked further, but make sure all your translation keys have a definition in both languages. If you need any help with the English, feel free to ask.

jniles · 2017-08-02T20:24:55Z

client/src/js/components/bhStockEntryExitType.js

+    $ctrl.display = function () {
+        if ($ctrl.isEntry) {
+            return $ctrl.reference || '';
+        } else {


Since you have return, this else is unnecessary. It will increase readability to remove it.

jniles · 2017-08-02T20:27:52Z

client/src/modules/stock/entry/entry.html

-        </button>
-      </div>
-
+      <bh-stock-entry-exit-type 


This was a great idea to make a component 👍

jniles · 2017-08-02T20:29:31Z

client/src/js/components/bhStockEntryExitType.js

+            onEntryExitTypeSelectCallback: '&?',
+            reference: '<?',
+            displayName: '<?',
+            isEntry: '<'


I think this should probably be @. The < is for binding a variable from the parent scope, but it doesn't look like the bindings are ever expected to change.

jniles · 2017-08-15T13:16:54Z

client/src/js/components/bhStockEntryExitType.js

+        templateUrl: 'modules/templates/bhStockEntryExitType.tmpl.html',
+        controller: StockEntryExitTypeController,
+        bindings: {
+            onEntryExitTypeSelectCallback: '&?',


You probably want to make this callback required (&).

jniles · 2017-08-15T15:02:28Z

server/controllers/stock/index.js

-      entity_uuid   : null,
-      is_exit       : 0,
-      flux_id       : core.flux.FROM_OTHER_DEPOT,
+      entity_uuid   : isExit ? db.bid(parameters.to_depot) : db.bid(parameters.from_depot),


Instead of doing this on every turn of the loop, can you do something like this:

const depot_uuid = db.bid(isExit ? parameters.from_depot : parameters.to_depot); const entity_uuid = db.bid(isExit ? parameters.to_depot : parameters.from_depot); const is_exit = isExit ? 1 : 0; const flux_id = isExit ? core.flux.TO_OTHER_DEPOT : core.flux.FROM_OTHER_DEPOT; parameters.lots.forEach((lot) => { record = { depot_uuid, is_exit, entity_uuid, // ... etc ... }; });

jniles · 2017-08-15T15:03:36Z

server/controllers/stock/reports/stock_exit_depot.receipt.handlebars

@@ -36,15 +36,15 @@
      </div>
      <div class="col-xs-6">
        <h4>{{translate 'STOCK.TO'}}</h4>
-        <span class="text-capitalize">{{translate 'STOCK.DEPOT'}}</span>: <strong>{{entry.details.depot_name}}</strong> <br>
-        <span class="text-capitalize">{{translate 'FORM.LABELS.DOCUMENT'}}</span>: <strong>{{entry.details.document_reference}}</strong> <br>
+        <span class="text-capitalize">{{translate 'STOCK.DEPOT'}}</span>: <strong>{{exit.details.otherDepotName}}</strong> <br>


jniles · 2017-08-15T15:04:10Z

test/end-to-end/stock/stock.adjustment.spec.js

@@ -52,4 +52,4 @@ function StockAdjustmentTests() {
  });
 }

-describe('Stock Adustment Test', StockAdjustmentTests);
+describe('Stock Adjustment Test', StockAdjustmentTests);


jniles · 2017-08-15T15:05:55Z

client/src/modules/stock/entry/modals/findTransfer.modal.html

+      FORM.BUTTONS.CLOSE
+    </button>
+
+    <bh-loading-button loading-state="FindForm.$loading">


I can submit this without any choices and it loads every lot. Is this intended behavior?

jniles · 2017-08-15T15:07:16Z

client/src/modules/stock/entry/modals/findTransfer.modal.html

+          ui-grid-resize-columns
+          ui-grid-selection>
+          <bh-grid-loading-indicator
+            loading-state="$ctrl.loading"


Is this loading property ever defined? I couldn't find it.

…n from receipt, use moment for expiration date

… own stock only

…ic to filter stock movement by

DedrickEnc · 2017-08-17T13:38:47Z

@jniles
Changes has been integrated

jniles

Just one more thing. After that, LGTM.

jniles · 2017-08-17T14:24:17Z

client/src/js/components/bhStockEntryExitType.js

+function StockEntryExitTypeController() {
+    var $ctrl = this;
+    $ctrl.selectedEntryExitType = null;
+    $ctrl.isEntry === 'true';


Does this actually work?

I think you might be missing an assignment - this is just an evaluation. Maybe...

$ctrl.isEntry = $ctrl.isEntry === 'true'

?

Either way, this should probably go in an $onInit method to make sure the bindings are correctly initialized.

jniles · 2017-08-17T15:09:02Z

bors r+

1905: Refactor Stock distribution r=jniles This PR refactor the stock distribution page and closes #1854 Most of the work is about addressing points listed in #1854 and some refactor in order to reduce the code. A new component ```bhStockEntryExitType``` has been introduced to remove repeated code found in ```exit.html``` and ```entry.html``` for selecting the type of entry or exit. The main change is about stock distribution from depot to an other depot (Stock transfer) Now the stock transfer is a set of two processes which are a stock exit to depot and a stock entry from depot. Both of them are independent in other to avoid mistakes and guaranty the data integrity. Each time the stock in the depot will match the stock in the system.

bors · 2017-08-17T15:53:48Z

Build succeeded

continuous-integration/travis-ci/push

jniles added the Refactor label Jul 30, 2017

jniles requested review from mbayopanda and sfount July 30, 2017 05:20

DedrickEnc force-pushed the dedrick-stock-dist branch from 45fe7e0 to 8c2d7f6 Compare July 31, 2017 09:28

DedrickEnc force-pushed the dedrick-stock-dist branch 2 times, most recently from 782c0c8 to 5be31bd Compare August 4, 2017 12:03

DedrickEnc force-pushed the dedrick-stock-dist branch from 5be31bd to 7791e65 Compare August 9, 2017 10:33

DedrickEnc force-pushed the dedrick-stock-dist branch 4 times, most recently from 5c0cd27 to 3a7a071 Compare August 15, 2017 10:59

DedrickEnc force-pushed the dedrick-stock-dist branch from 3a7a071 to b395465 Compare August 15, 2017 14:55

jniles requested changes Aug 15, 2017

View reviewed changes

DedrickEnc force-pushed the dedrick-stock-dist branch 2 times, most recently from 8a73799 to 08ed8b4 Compare August 17, 2017 12:09

DedrickEnc added 9 commits August 17, 2017 13:32

creating a cmponent to replace all stock destination box

99fd704

amend the code translation and filter only lot which are not empty

e28decb

move uniformSelectedExitType function to a service, remove descriptio…

9927355

…n from receipt, use moment for expiration date

add a new component for e2e, update e2e

dd032c1

clean bhDistributionDestination component

d5ca403

update the stock transfer exit, now the source depot will changes his…

2b5815e

… own stock only

adding a component for stock entry type

a0753f2

reception of transfer done, test update remain

ac384c3

stock movement depot to depot done

7f0ea16

DedrickEnc added 3 commits August 17, 2017 13:32

updating e2e test

036edb2

polish the entry exit component UI

c2e794a

fix(test and server): Fix the end to end test and the server side log…

999b9a8

…ic to filter stock movement by

DedrickEnc force-pushed the dedrick-stock-dist branch from 08ed8b4 to 8123855 Compare August 17, 2017 12:53

fix(review): Update the pull requst according the review suggestion

fb2ec1f

jniles reviewed Aug 17, 2017

View reviewed changes

DedrickEnc force-pushed the dedrick-stock-dist branch from 8123855 to fb2ec1f Compare August 17, 2017 14:56

jniles approved these changes Aug 17, 2017

View reviewed changes

bors bot merged commit fb2ec1f into Third-Culture-Software:master Aug 17, 2017

DedrickEnc deleted the dedrick-stock-dist branch August 18, 2017 11:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor Stock distribution #1905

Refactor Stock distribution #1905

DedrickEnc commented Jul 29, 2017 •

edited

Loading

DedrickEnc commented Jul 31, 2017

DedrickEnc commented Aug 4, 2017

DedrickEnc commented Aug 14, 2017

jniles commented Aug 14, 2017

DedrickEnc commented Aug 14, 2017

DedrickEnc commented Aug 15, 2017

jniles left a comment

jniles Aug 2, 2017

jniles Aug 2, 2017

jniles Aug 2, 2017

jniles Aug 2, 2017

jniles Aug 15, 2017

jniles Aug 15, 2017

jniles Aug 15, 2017

jniles Aug 15, 2017

jniles Aug 15, 2017

jniles Aug 15, 2017

DedrickEnc commented Aug 17, 2017

jniles left a comment

jniles Aug 17, 2017

jniles commented Aug 17, 2017

bors bot commented Aug 17, 2017

Refactor Stock distribution #1905

Refactor Stock distribution #1905

Conversation

DedrickEnc commented Jul 29, 2017 • edited Loading

DedrickEnc commented Jul 31, 2017

DedrickEnc commented Aug 4, 2017

DedrickEnc commented Aug 14, 2017

jniles commented Aug 14, 2017

DedrickEnc commented Aug 14, 2017

DedrickEnc commented Aug 15, 2017

jniles left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DedrickEnc commented Aug 17, 2017

jniles left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jniles commented Aug 17, 2017

bors bot commented Aug 17, 2017

Build succeeded

DedrickEnc commented Jul 29, 2017 •

edited

Loading