OBPIH-7626 Enable destination field on send page #5702

SebastianLib · 2026-01-08T13:16:37Z

✨ Description of Change

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

Description:

📷 Screenshots & Recordings (optional)

Copilot

Pull request overview

This pull request enables the destination field to be editable on the inbound stock movement send page and refactors form data structures in both create and send pages. The changes allow users to select different destination locations instead of being restricted to the current location.

Key Changes:

Added locationTypeCode parameter to debounceLocationsFetch utility function to filter locations by type
Enabled destination field editing on the send page with async location fetching filtered by DEPOT type
Restructured hook return values to use nested objects for better organization
Removed destination-matching validation logic that previously restricted editing
Reordered backend location filtering logic to prioritize locationTypeCode over direction filtering

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
`src/js/utils/option-utils.jsx`	Added locationTypeCode parameter to debounceLocationsFetch for type-based filtering
`src/js/hooks/inboundV2/send/useInboundSendForm.js`	Enabled destination field editing, added destination location fetching, restructured return value, removed matchesDestination checks
`src/js/hooks/inboundV2/create/useInboundCreateForm.js`	Added debounced location/people fetch functions, restructured return value, improved form reset logic
`src/js/components/stock-movement-wizard/inboundV2/sections/send/InboundSendFormHeader.jsx`	Removed destination-matching checks from button disabled states, replaced with isDispatched checks
`src/js/components/stock-movement-wizard/inboundV2/sections/send/InboundSendForm.jsx`	Connected destination field to async location fetching, removed disabled states from form fields, simplified button logic
`src/js/components/stock-movement-wizard/inboundV2/sections/create/InboundCreate.jsx`	Refactored to use restructured hook return values, removed hasErrors from destination field
`grails-app/services/org/pih/warehouse/core/LocationService.groovy`	Reordered filtering logic to check locationTypeCode before INBOUND direction

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-08T13:22:09Z

src/js/hooks/inboundV2/create/useInboundCreateForm.js

+          ? {
+            id: data.origin.id,
+            name: data.origin.name,
+            label: `${data.origin.name} [${data.origin.locationType?.description ?? ''}]`,


The code uses optional chaining data.origin.locationType?.description but doesn't check if data.origin itself could be null/undefined before accessing locationType. While this won't cause an error with the existing null check data.origin ? { ... } : null, it's inconsistent with the pattern used for data.destination and could be misleading. Consider using consistent null-safety patterns throughout.

Suggested change

label: `${data.origin.name} [${data.origin.locationType?.description ?? ''}]`,

label: `${data.origin.name} [${data.origin?.locationType?.description ?? ''}]`,

Copilot · 2026-01-08T13:22:10Z

src/js/hooks/inboundV2/create/useInboundCreateForm.js

+    },
+    data: {
+      stockLists,
+      origin,


The hook returns origin in the data object but not destination, even though destination is watched at line 86. If any consumer needs to react to destination changes (similar to how origin is used), they won't have access to it. Consider whether destination should also be included in the return value for consistency and potential future use.

Suggested change

origin,

origin,

destination,

At the moment, we don’t need the destination outside this hook

Copilot · 2026-01-08T13:22:10Z

src/js/hooks/inboundV2/send/useInboundSendForm.js

    } = getValues();

    return {
+      destination: { id: destination.id },


Potential null pointer exception: The code accesses destination.id without null checking. If the destination field is cleared by the user (since it's now editable), this will throw a runtime error when getShipmentPayload is called. Add validation to ensure destination is not null before accessing its properties.

Suggested change

destination: { id: destination.id },

destination: destination ? { id: destination.id } : null,

we always trigger validation, before getShipmentPayload is called

Copilot · 2026-01-08T13:22:11Z

src/js/hooks/inboundV2/create/useInboundCreateForm.js


  useEffect(() => {
-    if (currentLocation) {
+    if (currentLocation && !destination?.id) {


The condition has been changed from if (currentLocation) to if (currentLocation && !destination?.id). This could lead to the destination not being set to currentLocation on subsequent renders if the destination field gets cleared. The original logic ensured the destination was always set when currentLocation exists. Consider whether this behavior change is intentional and whether it could cause issues with form initialization.

Copilot · 2026-01-08T13:22:11Z

src/js/hooks/inboundV2/create/useInboundCreateForm.js

      }),
      origin: { id: values.origin.id },
-      destination: { id: currentLocation.id },
+      destination: { id: values.destination.id },


The destination field validation schema is optional/nullable, but the form submission at line 98 assumes values.destination.id exists. This could cause a runtime error if the destination is null or undefined when the form is submitted. Consider adding proper null checking in the onSubmitStockMovementDetails function or making the destination field required in the validation schema.

We don't need to check whether destination.id exists because destination is now required in the zod schema. This function is called on onSubmit, so we check validation before reaching this point.

codecov · 2026-01-08T13:26:13Z

Codecov Report

❌ Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (feature/inbound-refactor@8bea964). Learn more about missing BASE report.

Files with missing lines	Patch %	Lines
...ices/org/pih/warehouse/core/LocationService.groovy	0.00%	2 Missing ⚠️

Additional details and impacted files

@@                    Coverage Diff                     @@
##             feature/inbound-refactor   #5702   +/-   ##
==========================================================
  Coverage                            ?   9.73%           
  Complexity                          ?    1325           
==========================================================
  Files                               ?     717           
  Lines                               ?   45805           
  Branches                            ?   10932           
==========================================================
  Hits                                ?    4458           
  Misses                              ?   40655           
  Partials                            ?     692

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

SebastianLib · 2026-01-08T13:28:27Z

grails-app/services/org/pih/warehouse/core/LocationService.groovy

+        if (direction == StockMovementDirection.INBOUND.name()) {
+            return locations.findAll {
+                it.locationType.locationTypeCode == LocationTypeCode.SUPPLIER
+            }
+        }
+


I’m not sure if this is the best solution and I hope this don't break anything, but I had to change the order of this if to the following:

if (params.locationTypeCode) { LocationTypeCode locationTypeCode = params.locationTypeCode as LocationTypeCode return locations.findAll { it.locationType.locationTypeCode == locationTypeCode } }

Previously, when we called the API with StockMovementDirection.INBOUND, we always received only suppliers.

We still want to return only suppliers for the origin select, but for the destination select we want to return only depots.

That’s why I decided to pass locationTypeCode to params. If locationTypeCode is provided, we filter locations by this code instead of using the default behavior (which returns suppliers for StockMovementDirection.INBOUND).

So before your change, if direction is inbound and you specify locationTypeCode, then it'd still only return suppliers. After your change, that scenario would return whatever locationTypeCode you specified.

But your change in option-utils.jsx means we were never specifying locationTypeCode for the call before, right? So I suspect this is fine.

Another way you could do it is keep the code where it was before and do:

if (direction == StockMovementDirection.INBOUND.name()) { LocationTypeCode code = (params.locationTypeCode as LocationTypeCode) ?: LocationTypeCode.SUPPLIER return locations.findAll { it.locationType.locationTypeCode == code } }

but the way you're doing it is fine. I think to make it truly "better" we'd need to refactor this whole method a bit so I don't mind you keeping it a small change for now

So before your change, if direction is inbound and you specify locationTypeCode, then it'd still only return suppliers. After your change, that scenario would return whatever locationTypeCode you specified.

But your change in option-utils.jsx means we were never specifying locationTypeCode for the call before, right? So I suspect this is fine.

@ewaterman From the frontend side nothing should break, because we weren't sending locationTypeCode before.

I just wanted to mention it here, because it’s possible I missed something on the backend side where this logic is used, so I preferred to be careful.

kchelstowski

how was the field disabled before? I can't see anything in the code responsible for "enabling" it now, and I see a lot of other "ad hoc" changes. Could you describe what is exactly fixed in this PR?

kchelstowski · 2026-01-12T10:56:58Z

grails-app/services/org/pih/warehouse/core/LocationService.groovy

+            LocationTypeCode code = (params.locationTypeCode as LocationTypeCode) ?: LocationTypeCode.SUPPLIER
            return locations.findAll {
-                it.locationType.locationTypeCode == LocationTypeCode.SUPPLIER
+                it.locationType.locationTypeCode == code


what's the context of that change?

I explained this here: https://github.com/openboxes/openboxes/pull/5702/files/815f77bfcfa97b44d4bc993d26315a1a432b4ce3#diff-6f0816430f13aa693679ac52a1d7c185aa95defdc7131a4f6dc4dd878fdeeeb7

kchelstowski · 2026-01-12T11:01:49Z

src/js/hooks/inboundV2/create/useInboundCreateForm.js

      });
    }
-  }, [currentLocation?.id]);
+  }, [currentLocation?.id, destination]);


destination if an object, you should include id in the dependency

good catch!

SebastianLib · 2026-01-12T11:44:49Z

@kchelstowski
So, initially Manon wanted the destination select to be disabled on the Create and Send page. Later, Kasia and Kelsey decided that we should enable the destination select, but only on the Send page. Therefore, on the Send page, I removed the disable boolean and implemented fetching locations.

Manon also wanted that if the location is changed during inbound and the selected destination (it was always that destination which was our current location during creating inbound) is not equal to the current location, all buttons should be blocked and the "Exit" button should be shown. This behavior was controlled by the matchesDestination boolean. That's why I have now removed the matchesDestination boolean.

The third change was related to the location selection. Previously, in the old inbound and this inbound before these changes, both the origin and destination selects only showed locations with locationTypeCode = SUPPLIER. Now, we want the destination select to include only depots in the options, as specified in the ticket: “- All depots should be selectable as destination.” The origin select, however, still has the same options as before.

Additionally, I did some minor code cleanup, mainly in the Create step, which wasn’t required for this ticket. Initially, as you can see in the ticket, I was supposed to enable the destination select also on the Create step. However, after discussing with Kasia, we decided it would be better to leave this select disabled. Before that, I made some code cleanup in this step, which is why you can see some changes there and I didn't remove that.

I hope that explained it clearly 😄 Let me know if you have any questions.

SebastianLib added 3 commits January 8, 2026 09:36

OBPIH-7626 Enable destination field on create and send page

ee5c4e1

OBPIH-7626 Disable destination field on first step

7dd4edc

OBPIH-7626 Rename variables

815f77b

SebastianLib requested review from alannadolny, awalkowiak, Copilot, ewaterman and kchelstowski January 8, 2026 13:16

github-actions bot added type: bug Addresses unintended behaviours of the app domain: frontend Changes or discussions relating to the frontend UI domain: backend Changes or discussions relating to the backend server labels Jan 8, 2026

Copilot started reviewing on behalf of SebastianLib January 8, 2026 13:17 View session

Copilot AI reviewed Jan 8, 2026

View reviewed changes

SebastianLib commented Jan 8, 2026

View reviewed changes

openboxes deleted a comment from Copilot AI Jan 8, 2026

OBPIH-7626 Corrections after copilot code review

c98b598

SebastianLib changed the title ~~OBPIH-7626 Enable destination field on create and send page~~ OBPIH-7626 Enable destination field on send page Jan 9, 2026

OBPIH-7626 Improve location filtering logic for inbound

ef161a3

alannadolny approved these changes Jan 9, 2026

View reviewed changes

kchelstowski reviewed Jan 12, 2026

View reviewed changes

OBPIH-7626 include destination?.id in the dependency

aa570e3

SebastianLib requested a review from kchelstowski January 12, 2026 11:53

	label: `${data.origin.name} [${data.origin.locationType?.description ?? ''}]`,
	label: `${data.origin.name} [${data.origin?.locationType?.description ?? ''}]`,

	destination: { id: destination.id },
	destination: destination ? { id: destination.id } : null,

Uh oh!

OBPIH-7626 Enable destination field on send page #5702

Are you sure you want to change the base?

OBPIH-7626 Enable destination field on send page #5702

Conversation

SebastianLib commented Jan 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✨ Description of Change

📷 Screenshots & Recordings (optional)

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Key Changes:

Reviewed changes

Uh oh!

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Jan 8, 2026 • 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!

kchelstowski left a comment

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!

Choose a reason for hiding this comment

Uh oh!

SebastianLib commented Jan 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

SebastianLib commented Jan 8, 2026 •

edited

Loading

codecov bot commented Jan 8, 2026 •

edited

Loading

SebastianLib commented Jan 12, 2026 •

edited

Loading