OBPIH-7043 add support for canceling a count by ewaterman · Pull Request #5206 · openboxes/openboxes

ewaterman · 2025-04-14T23:39:11Z

✨ Description of Change

Link to GitHub issue or Jira ticket: https://pihemr.atlassian.net/browse/OBPIH-7043

Description: Add in "cancel count" buttons to count and recount step which delete the selected cycle count, request, and items.

I also started adding logic in to disable the "next" button during a recount if there are no rows in the table (ie there are no recount items) for any of the products in the batch. This will hopefully prevent us from getting into a bad state where we try to submit the recount with no items. Users in this scenario can either add custom rows to the count, or cancel the count entirely.

📷 Screenshots & Recordings (optional)

Recording of cancel on to count / to resolve tabs:

Screencast.from.2025-04-14.04.35.53.PM.webm

Recording of cancel during recount when no items:

Screencast.from.2025-04-16.05.01.23.PM.webm

ewaterman · 2025-04-14T23:42:21Z

This is a WIP pull request!

The following remains to be fixed:

After "confirm" is pressed, we need to refresh the table on the count and recount tabs so that the items are removed.
Fix the width on the confirmation popup.
Fix the logic to disable the "next" button on recount if any of the products being counted have no items for the CURRENT count index (ie there are no rows in the table) -> please confirm with Manon that this is the desired behaviour!
Fix the styling on the "next" button so that it visually shows as disabled when it has that state

Additional (optional) considerations:

Do we need to add the same logic for disabling the "next" button on regular count? I'm wondering what happens in this scenario: We request a count, then zero the stock of the product via record stock, then start the count. Does it explode?

I'm going to be away tomorrow so likely someone will pick up this PR. Please fix each of the above before merging!

codecov · 2025-04-14T23:46:36Z

Codecov Report

Attention: Patch coverage is 3.84615% with 25 lines in your changes missing coverage. Please review.

Project coverage is 8.12%. Comparing base (aeb287b) to head (0a04c72).
Report is 14 commits behind head on release/0.9.4.

Files with missing lines	Patch %	Lines
...g/pih/warehouse/inventory/CycleCountService.groovy	0.00%	20 Missing ⚠️
...g/pih/warehouse/api/CycleCountApiController.groovy	0.00%	4 Missing ⚠️
...h/warehouse/inventory/CycleCountItemCommand.groovy	0.00%	1 Missing ⚠️

Additional details and impacted files

@@                Coverage Diff                 @@
##             release/0.9.4   #5206      +/-   ##
==================================================
- Coverage             8.12%   8.12%   -0.01%     
  Complexity             940     940              
==================================================
  Files                  638     638              
  Lines                43220   43245      +25     
  Branches             10507   10512       +5     
==================================================
+ Hits                  3511    3512       +1     
- Misses               39172   39196      +24     
  Partials               537     537

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

ewaterman · 2025-04-14T23:44:18Z

src/main/groovy/org/pih/warehouse/inventory/CycleCountItemCommand.groovy

-            return ReasonCode.listInventoryAdjustmentReasonCodes().contains(discrepancyReasonCode) ?
-                    true :
-                    ['cycleCountItemCommand.discrepancyReasonCode.invalid']
+            return ReasonCode.listInventoryAdjustmentReasonCodes().contains(discrepancyReasonCode) ? true : ['invalid']


this is unrelated. Just something I've been meaning to fix and didn't have a better place for it

ewaterman · 2025-04-14T23:44:43Z

...test/groovy/unit/org/pih/warehouse/inventory/CycleCountProductAvailabilityServiceSpec.groovy

        assert cycleCount.cycleCountItems.size() == 0  // The item has been removed!
    }

+    void 'refreshProductAvailability should only remove items from the most recent count when not in available items'() {


unrelated to the change. Just a test I wanted to get it.

ewaterman · 2025-04-15T00:05:41Z

src/js/hooks/cycleCount/useResolveStepValidation.js

+   * current count index. This is possible if the product had its quantities zeroed out while
+   * the count was in progress via a different process (such as record stock).
+   */
+  const anyCountHasNoItems = () => _.some(


I'm just realizing that this should check only the items of the most recent count index (we could hard code this to "1" if that's easier)

@ewaterman this method is probably also not needed anymore

src/js/components/cycleCount/toResolveTab/ResolveStepHeader.jsx

…odal, start building logic to map products without recount item

kchelstowski · 2025-04-16T14:30:43Z

src/js/components/cycleCount/toResolveTab/ResolveStepHeader.jsx

          label="react.default.button.next.label"
          defaultLabel="Next"
          variant="primary"
+          //disabled={disableNextButton()}


@ewaterman I think this should be removed and we don't need the disableNextButton anymore, but I left that for you to see, for the clarity.

kchelstowski · 2025-04-16T14:31:19Z

src/js/hooks/cycleCount/useResolveStep.js

+
+    // TODO: Display the modal, pass the productsWithNoRecount. Remember we might have a few products (tables) empty, and we eventually want to call Cancel for every product
+    // TODO: So we need to do call Cancel for each "productsWithNoRecount" (this is why I store the cycleCountId also)
+


here I explained what's left in terms of the modal. Please, also make sure we don't need any nullsafes around the cycleCountItems[0] (probably not, but just to be sure)

thanks for doing that! It was very helpful for me 👍

kchelstowski · 2025-04-16T14:32:24Z

@ewaterman I also rebased the branch and fixed the conflicts.

kchelstowski · 2025-04-17T08:04:20Z

src/js/hooks/cycleCount/useResolveStep.js

+
+  const openZeroRecountItemsModal = (productsWithNoRecountItems) => {
+    const requestIds = productsWithNoRecountItems.map((entry) => (entry.cycleCountRequestId));
+    const productCodes = productsWithNoRecountItems.map((entry) => (entry.product));


for the optimization I would suggest using .reduce method.

Suggested change

const productCodes = productsWithNoRecountItems.map((entry) => (entry.product));

const { productCodes, requestIds } = productsWithNoRecountItems.reduce((acc, entry) => ({

requestIds: [ ...acc.requestIds, entry.cycleCountRequestId ],

productCodes: [ ...acc.productCodes, entry.product ],

}, { requestIds: [], productCodes: [] }))

I could quickly change it, but it's not "must have" change

kchelstowski · 2025-04-17T08:14:15Z

grails-app/services/org/pih/warehouse/inventory/CycleCountService.groovy

+        }
+
+        cycleCount.cycleCountItems.each { it.delete() }
+        cycleCount.cycleCountItems.clear()


I think more appropriate way to do it would be to use cycleCount.removeFromCycleCountItems, but since we delete all, I'm probably fine with that.

ewaterman self-assigned this Apr 14, 2025

ewaterman added the warn: do not merge Marks a pull request that is not yet ready to be merged label Apr 14, 2025

ewaterman commented Apr 15, 2025

View reviewed changes

ewaterman requested review from SebastianLib, alannadolny, awalkowiak and kchelstowski April 15, 2025 00:14

ewaterman and others added 3 commits April 16, 2025 14:36

add support for canceling a count

88cb1a6

add docstrings

cd33521

OBPIH-7043 Refetch the data after canceling cycle count, resize the m…

dae682e

…odal, start building logic to map products without recount item

kchelstowski force-pushed the ft/OBPIH-7043-cancel-cycle-count branch from 55e57a2 to dae682e Compare April 16, 2025 14:29

kchelstowski reviewed Apr 16, 2025

View reviewed changes

don't allow proceeding with recount if no recount items

0a04c72

ewaterman removed the warn: do not merge Marks a pull request that is not yet ready to be merged label Apr 17, 2025

kchelstowski approved these changes Apr 17, 2025

View reviewed changes

awalkowiak approved these changes Apr 17, 2025

View reviewed changes

awalkowiak merged commit 30d1afc into release/0.9.4 Apr 17, 2025
8 checks passed

awalkowiak deleted the ft/OBPIH-7043-cancel-cycle-count branch April 17, 2025 11:11


		// TODO: Display the modal, pass the productsWithNoRecount. Remember we might have a few products (tables) empty, and we eventually want to call Cancel for every product
		// TODO: So we need to do call Cancel for each "productsWithNoRecount" (this is why I store the cycleCountId also)

-    const productCodes = productsWithNoRecountItems.map((entry) => (entry.product));
+    const { productCodes, requestIds } = productsWithNoRecountItems.reduce((acc, entry) => ({
+      requestIds: [ ...acc.requestIds, entry.cycleCountRequestId ],
+      productCodes: [ ...acc.productCodes, entry.product ],
+    }, { requestIds: [], productCodes: [] }))

Uh oh!

Conversation

ewaterman commented Apr 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✨ Description of Change

📷 Screenshots & Recordings (optional)

Uh oh!

ewaterman commented Apr 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Apr 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kchelstowski commented Apr 16, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ewaterman commented Apr 14, 2025 •

edited

Loading

ewaterman commented Apr 14, 2025 •

edited

Loading

codecov bot commented Apr 14, 2025 •

edited

Loading