Skip to content

OBLS-507 Do not display move to staging when track internal transactions disabled for location - #445

Open
adambalcerzak wants to merge 2 commits into
developfrom
OBLS-507
Open

OBLS-507 Do not display move to staging when track internal transactions disabled for location#445
adambalcerzak wants to merge 2 commits into
developfrom
OBLS-507

Conversation

@adambalcerzak

Copy link
Copy Markdown
Collaborator

backend changes openboxes/openboxes#6144

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

The newly added Location.hasInternalTransactionSupport field is modeled as required despite the flow logic treating it as potentially absent, creating a type/behavior mismatch that should be reconciled.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR updates the mobile picking flow to skip (or avoid navigating into) the move-to-staging step when the current task’s facility does not support internal transactions, aligning the UI behavior with the new backend capability flags.

Changes:

  • Add skipStagingStep to the picking context and thread it through picking flow helpers to bypass staging completion when internal transactions are disabled.
  • Update picking screens to pass skipStagingStep/resetSession into shared navigation helpers.
  • Filter out non-staging-eligible tasks in the Move To Staging screen and add the hasInternalTransactionSupport flag to the Location model.
File summaries
File Description
src/screens/Picking/PickingPickQuantityScreen.tsx Passes skipStagingStep and resetSession into completion/navigation helper when finishing via short-pick flow.
src/screens/Picking/PickingPickOutboundContainerScreen.tsx Passes skipStagingStep and resetSession into revalidation/navigation helper after picking/short-picking.
src/screens/Picking/PickingMoveToStaging.tsx Filters fetched picked tasks to only those that require staging based on internal transaction support.
src/screens/Picking/PickingContext.tsx Introduces skipStagingStep in context and uses it to change end-of-session routing when no new tasks are created.
src/screens/Picking/lib.ts Extends flow helpers to optionally skip staging completion and reset the session before returning home.
src/data/location/Location.ts Adds hasInternalTransactionSupport to the Location type used by picking tasks.
Review details
  • Files reviewed: 6/6 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/data/location/Location.ts Outdated
hasBinLocationSupport: boolean;
hasPackingSupport: boolean;
hasPartialReceivingSupport: boolean;
hasInternalTransactionSupport: boolean;

@awalkowiak awalkowiak left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@adambalcerzak Is the move to outbound container disabled already? I think this also should not be required in this case. Could you write what is the picking proces on mobile app with and without internal transactions enabled?

@adambalcerzak

Copy link
Copy Markdown
Collaborator Author

@awalkowiak I think on mobile app the picking process (until now) was identical regardless of track internal transactions value. Scan pick location - scan product - quantity - scan outbound container - scan staging location.
The difference was in backend. e.g. on transfer to an outbound container it returned immediately
image

OBLS-507 only mentioned to hide a move to staging.

@awalkowiak

Copy link
Copy Markdown
Contributor

Marking this as on hold since:
a) I need confirmation from Kasia on how it should work in the full pick task flow, since the move to the outbound container is an internal transaction as well (and she is ooo)
b) This is a flow version that won't be used by VVG (probably at all), so no need to rush it, to avoid random regressions.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants