OBPIH-7307 To Count and To Resolve products visible and disabled on All Products tab#5278
OBPIH-7307 To Count and To Resolve products visible and disabled on All Products tab#5278awalkowiak merged 6 commits intodevelopfrom
Conversation
github-actions
bot
added
domain: frontend
|
|
||
| // Products from the "To Count" and "To Resolve" tabs can have negative quantity, | ||
| // so we want to include them in the results. | ||
| if (command.includeNullStatus) { |
There was a problem hiding this comment.
I don't really like the name of this boolean, I'm still wondering what to change it to
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #5278 +/- ##
============================================
+ Coverage 8.32% 8.37% +0.05%
- Complexity 976 996 +20
============================================
Files 642 646 +4
Lines 43311 43406 +95
Branches 10523 10537 +14
============================================
+ Hits 3604 3637 +33
- Misses 39156 39204 +48
- Partials 551 565 +14 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| or { | ||
| if (command.statuses) { | ||
| inList("status", command.statuses) | ||
| } |
There was a problem hiding this comment.
in the code above when checking statuses, you say that if showCountAndResolveProducts, then show cycle counts with null status. Here you do something opposite. Could you explain the reason for double checking statuses here?
There was a problem hiding this comment.
I want to include all products that have statuses listed in command.statuses regardless of includeStockOnHandOrNegativeStock. For the rest, we filter (with statuses null) by hasStockOnHandOrNegativeStock, thanks to this the backend returns good results to me. am I right? Can i do it better?
| if (command.statuses) { | ||
| inList("status", command.statuses) | ||
| } | ||
| and { |
There was a problem hiding this comment.
what's the purpose of that and if you have only one statement inside?
| CycleCountCandidateStatus.COUNTED, | ||
| CycleCountCandidateStatus.INVESTIGATING, | ||
| ], | ||
| showCountAndResolveProducts: true, |
There was a problem hiding this comment.
btw I think something like showCycleCountsInProgress would sound better than "countAndResolve", since we might have also the approve step in the future and this would have to be changed if the cycle counts at the approval step should also be a part of that statement.
| if ( | ||
| status === CycleCountCandidateStatus.CREATED | ||
| || status === CycleCountCandidateStatus.REQUESTED | ||
| || status === CycleCountCandidateStatus.COUNTING |
There was a problem hiding this comment.
maybe it'd be a good idea to create some simple util methods to say "isInProgress", "isCounting" etc.
| // but we want to include them in the results | ||
| if (command.showCycleCountsInProgress) { | ||
| or { | ||
| inList("status", command.statuses) |
There was a problem hiding this comment.
I think this is unnecessary - this is handled in lines 81-86. I think you need only eq here. I might be wrong so please confirm if I'm right, but for me it looks like we write inList("status"....) twice.
There was a problem hiding this comment.
without this not all results from count and resolve are returned. example:
3 participants
✨ Description of Change
Link to GitHub issue or Jira ticket:
Description:
📷 Screenshots & Recordings (optional)