Skip to content

ARCH-001 Phase 6: Shipment consolidation (Admin/Store/Vendor) - #796

Merged
KrzysztofPajak merged 12 commits into
developfrom
arch001/phase6-shipment-consolidation
Aug 26, 2026
Merged

ARCH-001 Phase 6: Shipment consolidation (Admin/Store/Vendor)#796
KrzysztofPajak merged 12 commits into
developfrom
arch001/phase6-shipment-consolidation

Conversation

@KrzysztofPajak

@KrzysztofPajak KrzysztofPajak commented Aug 26, 2026

Copy link
Copy Markdown
Member

No description provided.

KrzysztofPajak and others added 12 commits August 25, 2026 20:27
…endor/Routed)

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013XGyrBT97Wr9zjxgDmG2KR
…lete Vendor's duplicate

Injects IAdminDataScope<Shipment> into Grand.Web.AdminShared's ShipmentViewModelService and
merges in the two pieces of Vendor-specific behavior that Vendor's now-deleted duplicate
service used to do on its own:
- PrepareShipmentModel(Shipment,...): order-item visibility now goes through
  scope.FilterOrderItems(order.OrderItems) instead of an unfiltered lookup.
- PrepareShipmentModel(Order): same scope.FilterOrderItems() filter applied to the
  AddShipment order-item picker (Vendor previously additionally gated this on a
  "not IsStoreManager" check via IGroupService - dropped deliberately, not silently:
  see below).
- PrepareShipment(Order,...): new Shipment.VendorId now set from scope.DefaultVendorId
  (null for Admin/Store, CurrentVendor.Id for Vendor - matches Vendor's prior hardcoding).

Deletes Grand.Web.Vendor/Interfaces/IShipmentViewModelService.cs and
Grand.Web.Vendor/Services/ShipmentViewModelService.cs, and removes the now-redundant DI
registration in Grand.Web.Vendor/Startup/StartupApplication.cs (AdminShared's own
StartupApplication registers the single implementation, discovered via the existing
IStartupApplication assembly scan).

Disclosed, deliberate behavior note: Vendor's original GET-path service method gated its
order-item filter on a "not IsStoreManager" check, while Vendor's own POST-path controller
action (AddShipment) filtered unconditionally with no IsStoreManager check at all - the two
call sites already disagreed before this consolidation. VendorShipmentDataScope.FilterOrderItems
(Task 1) has no IsStoreManager branch, so this change makes the GET path consistent with the
already-existing POST path behavior, not the other way around. Not resolving which of the two
pre-existing behaviors was actually intended - flagging for product owner input, out of scope
here.

Grand.Web.Vendor/Controllers/ShipmentController.cs is left in its already-broken state
(Task 2) referencing the now-deleted types; that cutover is Task 9's job.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013XGyrBT97Wr9zjxgDmG2KR
Adds BaseShipmentController.cs to Grand.Web.AdminShared/Controllers with
the List/ShipmentListSelect/ShipmentsByOrder/ShipmentsItemsByShipmentId
region only; later tasks (5-8) append further regions to the same file,
and Task 9 does the per-host cutover.

Behavior notes carried over from the design:
- ShipmentsByOrder filters per-shipment via scope.HasAccess rather than
  gating on the parent order (see in-file doc comment for the equivalence
  argument per host).
- ShipmentsItemsByShipmentId is a deliberate, disclosed behavior change for
  Store only: Store's original returned a soft Content("") on a store
  mismatch; unified onto the throwing ArgumentException form used by
  Admin/Vendor's originals (2 of 3 hosts), since LoadAuthorizedShipment's
  redirect is the wrong fit for a JSON-grid endpoint.

Adds BaseShipmentControllerTests.cs covering List, the conditional
DefaultStoreId/DefaultVendorId forcing in ShipmentListSelect (global vs.
store vs. vendor scope), per-shipment access filtering in ShipmentsByOrder,
and the ArgumentException thrown by ShipmentsItemsByShipmentId on denied
access.

Grand.Web.AdminShared/Grand.Web.AdminShared.csproj builds with 0 errors.
All 6 new tests pass.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013XGyrBT97Wr9zjxgDmG2KR
…IAdminDataScope<Order>

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013XGyrBT97Wr9zjxgDmG2KR
…er region

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013XGyrBT97Wr9zjxgDmG2KR
…ervice layer complete

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013XGyrBT97Wr9zjxgDmG2KR
… BaseShipmentController subclasses

- Rewrite Admin/Store/Vendor ShipmentController.cs as thin subclasses of
  Grand.Web.AdminShared.Controllers.BaseShipmentController (Tasks 4-8).
- Admin keeps its own EditUserFields action (never shared, per spec).
- Attribute sets verified against each host's real, already-merged
  OrderController.cs rather than the plan brief's transcription:
  Admin: [AuthorizeAdmin][AutoValidateAntiforgeryToken][Area(Constants.AreaAdmin)][AuthorizeMenu]
  Store: [AutoValidateAntiforgeryToken][Area(Constants.AreaStore)][AuthorizeStore][AuthorizeMenu]
  Vendor: [AutoValidateAntiforgeryToken][Area(Constants.AreaVendor)][AuthorizeVendor][AuthorizeMenu]
  (Vendor's OrderController.cs does declare [AuthorizeMenu], unlike the brief's
  Vendor ShipmentController snippet which omitted it.)
- None of the three restate [PermissionAuthorize(PermissionSystemName.Shipments)]
  at the subclass level, matching BaseOrderController's established precedent —
  it lives once on BaseShipmentController.
- No per-host ShipmentControllerTests.cs existed to trim; BaseShipmentControllerTests.cs
  (behavioral, base-level) is untouched.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013XGyrBT97Wr9zjxgDmG2KR
…dd per-host WidgetZone overrides

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013XGyrBT97Wr9zjxgDmG2KR
…AddShipment view

Review found the NotAvailabe key was left hardcoded to 'Admin.' instead of using
the same $"{prefix}..." templating as every other key in the file, so it could
never resolve for Vendor even though a matching (misspelled) Vendor resource
entry exists.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013XGyrBT97Wr9zjxgDmG2KR
…troller (C1)

C1 (critical): Store's ShipmentDetails "User fields" tab posts to EditUserFields with no
host gating, but Store's thin subclass never got that action during consolidation (the spec's
claim that it was Admin-exclusive was wrong - Store's original controller had its own copy).
Moved EditUserFields into BaseShipmentController so all 3 hosts share it, same authorization
(PermissionActionName.Edit + scope.HasAccess) as before. Deleted the duplicate from Admin's
subclass. Added denial-path and success-path tests.

M1: Vendor's NoShipmentsSelected error used a hardcoded "Admin." resource key instead of
scope.ResourceKeyPrefix, so Vendor never saw its own Vendor.Orders.Shipments.NoShipmentsSelected
message. Fixed both call sites (PdfPackagingSlipAll, PdfPackagingSlipSelected).

M2: Rewrote 3 doc/inline comments in BaseShipmentController that referenced the SDD planning
process (task numbers, commit-message instructions, a dangling reference to a nonexistent note)
so they explain the code's behavior on their own.

M3: Updated IAdminDataScope.FilterOrderItems doc comment - VendorShipmentDataScope now also
overrides it, not just VendorOrderDataScope.

M4: PdfPackagingSlipSelected's access-filtering test now asserts the PDF service actually
received only the accessible shipment, not just that HasAccess was probed.

M5: Added defense-in-depth null/empty-identity guards to Store/VendorShipmentDataScope.HasAccess
plus tests for the null/empty-identity case.

M7: Clarified StoreShipmentDataScope's doc comment that CanView is inherited from the interface
default while touching the file for M5.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013XGyrBT97Wr9zjxgDmG2KR
Copilot AI lite review requested due to automatic review settings August 26, 2026 05:34

Copilot AI 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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Comment thread src/Tests/Grand.Web.Admin.Tests/Controllers/BaseShipmentControllerTests.cs Dismissed
Comment thread src/Tests/Grand.Web.Admin.Tests/Controllers/BaseShipmentControllerTests.cs Dismissed
Comment thread src/Tests/Grand.Web.Admin.Tests/Controllers/BaseShipmentControllerTests.cs Dismissed
Comment thread src/Tests/Grand.Web.Admin.Tests/Controllers/BaseShipmentControllerTests.cs Dismissed
Comment thread src/Tests/Grand.Web.Admin.Tests/Controllers/BaseShipmentControllerTests.cs Dismissed
Comment thread src/Tests/Grand.Web.Admin.Tests/Controllers/BaseShipmentControllerTests.cs Dismissed
Comment thread src/Tests/Grand.Web.Admin.Tests/Controllers/BaseShipmentControllerTests.cs Dismissed
Comment thread src/Tests/Grand.Web.Admin.Tests/Controllers/BaseShipmentControllerTests.cs Dismissed
Comment thread src/Tests/Grand.Web.Admin.Tests/Controllers/BaseShipmentControllerTests.cs Dismissed
Comment thread src/Tests/Grand.Web.Admin.Tests/Controllers/BaseShipmentControllerTests.cs Dismissed
@KrzysztofPajak
KrzysztofPajak merged commit 05dcd2e into develop Aug 26, 2026
6 checks passed
@KrzysztofPajak
KrzysztofPajak deleted the arch001/phase6-shipment-consolidation branch August 26, 2026 15:07
KrzysztofPajak added a commit that referenced this pull request Aug 26, 2026
…dation

Resolves the expected additive conflict in Grand.Web.AdminShared/Startup/StartupApplication.cs
between this phase's IAdminDataScope<PaymentTransaction> registration and Shipment's
(PR #796, merged to develop) IAdminDataScope<Shipment> registration — both blocks kept, no
logic changed on either side.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013XGyrBT97Wr9zjxgDmG2KR
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants