Skip to content

OBPIH-7292 Add “Assign count” buttons with the functionality#5326

Merged
awalkowiak merged 5 commits intoft/OBPIH-7287-cycle-count-assignmentfrom
OBPIH-7292
Jun 13, 2025
Merged

OBPIH-7292 Add “Assign count” buttons with the functionality#5326
awalkowiak merged 5 commits intoft/OBPIH-7287-cycle-count-assignmentfrom
OBPIH-7292

Conversation

@alannadolny
Copy link
Collaborator

No description provided.

@alannadolny alannadolny self-assigned this Jun 12, 2025
@github-actions github-actions bot added domain: frontend Changes or discussions relating to the frontend UI domain: backend Changes or discussions relating to the backend server domain: l10n Changes or discussions relating to localization & Internationalization labels Jun 12, 2025
@codecov
Copy link

codecov bot commented Jun 12, 2025

Codecov Report

❌ Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 8.32%. Comparing base (6f9c35c) to head (5c90eff).

Files with missing lines Patch % Lines
...g/pih/warehouse/inventory/CycleCountService.groovy 0.00% 2 Missing ⚠️
Additional details and impacted files
@@                            Coverage Diff                            @@
##             ft/OBPIH-7287-cycle-count-assignment   #5326      +/-   ##
=========================================================================
- Coverage                                    8.35%   8.32%   -0.04%     
+ Complexity                                   1002     999       -3     
=========================================================================
  Files                                         652     652              
  Lines                                       43796   43798       +2     
  Branches                                    10617   10618       +1     
=========================================================================
- Hits                                         3658    3645      -13     
- Misses                                      39571   39590      +19     
+ Partials                                      567     563       -4     

☔ 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.

Copy link
Collaborator

@awalkowiak awalkowiak left a comment

Choose a reason for hiding this comment

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

Overall I just dislike the existing AssignCycleCountModal. Is there still a PR pending with it? Could we improve it? Where? Does it require reopen of the modal related ticket? Rest lgtm

<>
{isAssignCountModalOpen && (
<AssignCycleCountModal
isOpen={isAssignCountModalOpen}
Copy link
Collaborator

Choose a reason for hiding this comment

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

I am not sure if this is needed if you are rendering the modal only if isAssignCountModalOpen, so this could be true by default / not required?

selectedCycleCountItems={assignCountModalData}
setSelectedCycleCountItems={setAssignCountModalData}
refetchData={fetchData}
isCount
Copy link
Collaborator

Choose a reason for hiding this comment

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

If this is AssignCycleCountModal I think isCount should be a default version, and there should be isRecount instead.

<AssignCycleCountModal
isOpen={isAssignCountModalOpen}
closeModal={closeAssignCountModal}
selectedCycleCountItems={assignCountModalData}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does assignCountModalData contain items?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

domain: backend Changes or discussions relating to the backend server domain: frontend Changes or discussions relating to the frontend UI domain: l10n Changes or discussions relating to localization & Internationalization

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants