Skip to content

Bug 634911: [Subcontracting] Fix factbox UX issues - captions, DecimalPlaces, singular/plural#8172

Open
ChethanT wants to merge 5 commits into
mainfrom
bugs/Subcontracting/634911-FactboxInconsistencies
Open

Bug 634911: [Subcontracting] Fix factbox UX issues - captions, DecimalPlaces, singular/plural#8172
ChethanT wants to merge 5 commits into
mainfrom
bugs/Subcontracting/634911-FactboxInconsistencies

Conversation

@ChethanT
Copy link
Copy Markdown
Contributor

@ChethanT ChethanT commented May 17, 2026

Summary

Fix several UX issues in two Subcontracting factbox pages reported in AB#634911.

Subc. Transfer Line Factbox (page 99001501)

  • Caption Production ComponentProduction Components (plural, matches the count > 1 it displays and aligns with the sibling Subc. Purchase Line Factbox, which already uses the plural form).

Subc. Routing Info Factbox (page 99001502)

  • Caption Order QuantityPurch. Order Qty. — clarifies it is the purchase order quantity.
  • Caption Quantity receivedQuantity Received; matching tooltip Title Cased.
  • Caption Quantity invoicedQuantity Invoiced; matching tooltip Title Cased.
  • DecimalPlaces = 0 : 50 : 0 on the three integer line-count fields: Transfer Order Lines, Return Transfer Order Lines, and Components. The decimal quantity fields keep 0 : 5.

Test plan

  • App compiles and publishes cleanly.
  • Open a Transfer Order with subcontracting lines linked to a prod. order with multiple components — verify the Subcontracting Details factbox now reads Production Components.
  • Open a Prod. Order Routing page with subcontracting routing lines — verify the Subcontracting Routing Details factbox shows Purch. Order Qty., Quantity Received, Quantity Invoiced, and the three line-count fields display integers (no .00000).

Fixes AB#634911

…lPlaces, singular/plural

Fix UX issues in two Subcontracting factbox pages:

Transfer Line Factbox (page 99001501):
- Change singular caption 'Production Component' to plural 'Production Components'
  to match the count shown and align with the sibling Purchase Line Factbox.

Routing Info Factbox (page 99001502):
- Rename 'Order Quantity' to 'Purch. Order Qty.' to clarify it is the
  purchase order quantity.
- Apply Title Case to 'Quantity Received' and 'Quantity Invoiced' captions
  and their matching tooltips.
- Change DecimalPlaces from 0:5 to 0:0 on the three integer line-count
  fields 'Transfer Order Lines', 'Return Transfer Order Lines', and
  'Components'. The decimal quantity fields above keep DecimalPlaces 0:5.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@ChethanT ChethanT requested a review from a team as a code owner May 17, 2026 16:45
@github-actions github-actions Bot added the AL: Apps (W1) Add-on apps for W1 label May 17, 2026
@github-actions github-actions Bot added this to the Version 29.0 milestone May 17, 2026
@ChethanT ChethanT changed the title [Subcontracting] Bug 634911: Fix factbox UX issues - captions, DecimalPlaces, singular/plural Bug 634911: [Subcontracting] Fix factbox UX issues - captions, DecimalPlaces, singular/plural May 17, 2026
…ures

Follow-up to the previous caption/DecimalPlaces fix. The three integer
line-count fields on the Routing Info Factbox were calling procedures
typed as Decimal that only ever returned Record.Count(), which required
DecimalPlaces = 0:0 on the page to avoid showing '0.00000' for an
integer count.

Tighten the return type of both procedures to Integer to match what
they actually return, and drop the now-redundant DecimalPlaces and
AutoFormatType properties from the page fields:

- SubcPurchFactboxMgmt.ShowTransferOrdersAndReturnOrder: Decimal -> Integer
  (also make implicit 'exit' statements explicit as 'exit(0)')
- SubcRoutingFactboxMgmt.GetNoOfLinkedComponentsFromRouting: Decimal -> Integer
- SubcRoutingInfoFactbox.Page: drop AutoFormatType=0 from the three
  integer-count fields (AutoFormatType has no effect on Integer fields)

All callers of both procedures are within the Subcontracting app; no
external consumers are affected.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@alexei-dobriansky
Copy link
Copy Markdown
Contributor

Analysis

Scenario Validity:

  • The linked bug is clear and valid, but it is a low-severity UX issue rather than a functional failure. The [AI-REPRO] comment is concrete: it says the transfer-line factbox shows Production Component even when the count is > 1, the routing factbox uses ambiguous Order Quantity, inconsistent Quantity received / Quantity invoiced casing, and integer counts can render with decimals because the page uses DecimalPlaces = 0:5.
  • The work item is specific enough to reproduce. I do not see any ambiguity about the symptom.

Correctness:

  • The caption changes themselves look correct. SubcTransferLineFactbox.Page.al:48-56 now uses Production Components, which matches the sibling purchase factbox (SubcPurchaseLineFactbox.Page.al:66-69). SubcRoutingInfoFactbox.Page.al:30-57 also moves the routing factbox captions to the clearer/title-cased forms requested by the bug.
  • The integer-display fix also works mechanically: SubcRoutingInfoFactbox.Page.al:63-85 now binds those three count fields to integer-returning expressions, so the UI will stop rendering 3.00000.
  • The problem is that the PR achieves that display fix by changing two public codeunit procedure signatures: SubcPurchFactboxMgmt.Codeunit.al:224 changes ShowTransferOrdersAndReturnOrder(...): Decimal to ...: Integer, and SubcRoutingFactboxMgmt.Codeunit.al:219 changes GetNoOfLinkedComponentsFromRouting(...): Decimal to ...: Integer.
  • Those procedures are not local, and the app exposes symbols/source (App\app.json:28-31). That means partner extensions can compile against them. Changing the return type is therefore a breaking API change for a cosmetic bug.
  • The signature change also looks unnecessary. The app already uses page-local integer wrappers elsewhere for count fields; for example SubcPurchaseLineFactbox.Page.al:134-137 exposes an integer count on the page while keeping the codeunit API shape separate. A similarly narrow page-level fix would avoid broadening the blast radius.
  • No BaseApp coupling issues are involved here; this PR does not add or move subscribers.

Side Effects:

  • Caption-only changes are harmless.
  • The return-type changes are the real side effect. Existing BCApps call sites shown in this repo mostly ignore the return value on drilldown paths, but any external extension that expects a Decimal return type from these procedures can break at compile time or require source changes for no functional gain.
  • I do not see event-ordering or subscriber-timing regressions.

Risk Assessment:

  • Medium. The underlying bug is low-risk UI polish, but the chosen implementation introduces avoidable public-API risk in a partner-extensible app.
  • Financial sensitivity is low because this does not change subcontracting calculations or posting logic.
  • The blast radius is broader than it needs to be because a page-formatting problem was solved by modifying reusable codeunit signatures.
  • What if we do not make this change? Users keep seeing slightly misleading captions and decimal-formatted counts in the factboxes, but subcontracting still works and the drilldowns still function. That workaround/impact profile is much less severe than silently breaking extension consumers.

Test Coverage:

  • The PR adds no tests. The changed file list contains only four app files and nothing under src/Apps/W1/Subcontracting/Test.
  • Per the review bar, that alone floors this at Request Changes for a behavior/UI change.
  • The good news is that the test infrastructure already exists. src/Apps/W1/Subcontracting/Test/src/Codeunits/Tests/SubcSubcontractingUITest.Codeunit.al already contains metadata-oriented UI tests, and the test app already includes page extensions such as SubcProdOrderRtngTST.PageExt.al for routing-page scenarios.
  • At minimum, add regression tests that (1) verify the two factboxes expose the corrected captions and (2) exercise a routing line with multiple linked transfer/component rows so the count fields are validated as whole-number displays. Even if the implementation is changed to avoid the signature break, the UX fix still needs tests.

Recommendation: Request Changes

  1. Do not change public return types for this UI fix — Restore the Decimal signatures of SubcPurchFactboxMgmt.ShowTransferOrdersAndReturnOrder and SubcRoutingFactboxMgmt.GetNoOfLinkedComponentsFromRouting, then implement the formatting fix at the page level (for example via local integer wrappers or page formatting metadata). Right now the PR turns a cosmetic bug into a breaking API change.
  2. Add regression tests in the Subcontracting test app — Extend SubcSubcontractingUITest.Codeunit.al (and any needed page-test helpers) so the corrected captions and integer-count rendering are covered. With no modified or added tests, this cannot clear review.

@alexei-dobriansky
Copy link
Copy Markdown
Contributor

Tests are not required, breaking change can be ignored since the feature hasn't been released yet. Approved.

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AL: Apps (W1) Add-on apps for W1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants