[KrzysztofPajak]

Part of the architecture backlog (item S5: god classes, OrderViewModelService)
Type: feature

Not a pure refactor: besides the split, this PR changes behavior in the places listed under Fixes. Each fix is in its own commit, so it can be reviewed or reverted apart from the split.

Issue

Grand.Web.AdminShared/Services/OrderViewModelService.cs was 1358 lines with 38 constructor dependencies. Eight of its members (order notes, the address editor and the "add product to order" page) are used only by BaseOrderManagementController (Admin and Store) and brought in eight dependencies nothing else in the class used.

Solution

• Three services split out, registered as scoped next to IOrderViewModelService, the same lifetime as before:

  Service	Members	Dependencies
  IOrderNoteViewModelService	PrepareOrderNotes, InsertOrderNote, DeleteOrderNote	4
  IOrderAddressViewModelService	PrepareOrderAddressModel, UpdateOrderAddress	6
  IOrderProductViewModelService	PrepareAddOrderProductModel, PrepareAddProductToOrderModel, AddProductToOrderDetails	15

• Members moved unchanged (checked by script: every original block, with _field renamed to the constructor parameter, is in the new file).

• OrderViewModelService keeps the order list and filters, the grid query (PrepareOrderModel, also used by the customer and product order tabs and by the vendor panel), PrepareOrderDetailsModel, PrepareOrders (export) and SaveOrderTags: 968 lines, 6 members, 30 dependencies.

• The controllers are not split: BaseOrderManagementController takes the three services and the Admin and Store OrderController pass them through, so no route or view changes. The vendor panel (BaseOrderController) is unchanged.

Fixes

Found while reviewing the moved code (separate commits):

• AddProductToOrderDetails stored the incl. tax unit price as the excl. tax one and the other way round (since Admin Panel - Replacing IFormCollection with strongly typed objects #409). With a non-zero tax the added order item had the wrong unit prices.
• AddressEdit fell back to a new, unattached address when the address id did not belong to the order; the POST then "saved" it and added an "Address has been edited" note. The address is now taken from the order (billing or shipping, by the billingAddress flag), so only the order's own address can be edited.
• OrderNoteAdd, ShipmentNoteAdd, MerchandiseReturnNoteAdd, CustomerNoteAdd and VendorNoteAdd accept POST only. Every view already calls them with POST.
• AddProductToOrderDetails (GET and POST) checks the product against the order's store with IAclService.Authorize and returns to the search otherwise.

Found in the code reviews of this PR (commits 2f7699903, 6ae0ec4ef):

• Order-note files: OrderNoteAdd took any downloadId from the request, and deleting the note deleted that file. A store manager could point a note at another store's product file and delete it. A note now takes only a file uploaded for its own order (DownloadType.Order with the order id as ReferenceId, which is what the notes form uploads); any other file refuses the note (Result = false with Admin.Orders.OrderNotes.DownloadNotOfOrder, shown by the panel) instead of saving it without the file. Deleting a note deletes only such a file.
• Product store check: it used the panel's store, which is empty in Main Admin, so Main Admin could add a product another store sells to the order. It now uses order.StoreId, which covers every panel. The product search on that page lists only what the order's store sells (it now gets the order id and checks access to the order), and a refused product returns to the search with Admin.Orders.Products.AddNew.NotInOrderStore instead of silently.
• Address editor: the fallback described above first threw ArgumentException (a 500 page). An address stored without _id gets a new id on every load, so the id in the link never matched for such orders. The address is now picked by the flag, and an order without that address redirects to its edit page. When validation fails, the form shows what was typed (and keeps BillingAddress) instead of the saved address.
• Quantity: adding a product with a quantity below 1 is refused with ShoppingCart.QuantityShouldPositive (an existing resource) instead of creating an item with zero or negative totals.
• Fewer queries: the controller loads the product once and passes it, with the order, to PrepareAddProductToOrderModel and AddProductToOrderDetails (they used to load the product, and the order, again). The notes grid no longer looks up a file for notes that have none.
• Cleanup: PrepareAddOrderProductModel returns Task.FromResult without async; a redundant OrderId assignment in DeleteOrderNote is gone.

Breaking changes

• 8 members moved from IOrderViewModelService to the new interfaces. Code outside this repository that calls one of them must inject the matching IOrder<Area>ViewModelService. A class that implements IOrderViewModelService, or subclasses OrderViewModelService and overrides a moved virtual, must move that code to the new service.
• The constructors of OrderViewModelService, BaseOrderManagementController and the Admin and Store OrderController changed.
• AddProductToOrderDetails (GET and POST) takes IProductService and IAclService as [FromServices] parameters.
• IOrderProductViewModelService.PrepareAddProductToOrderModel(Order, Product) and AddProductToOrderDetails(Order, Product, AddProductToOrderModel) take the loaded entities instead of ids; OrderProductViewModelService no longer takes IOrderService and IProductService.
• The five note-add actions no longer answer GET.
• IOrderNoteViewModelService.InsertOrderNote returns Task<bool>: false when the file is not one uploaded for the order, and nothing is saved then.
• The add-product search (POST AddProductToOrder) needs OrderId; the view sends it.
• Two new resources, in DefaultLanguage.xml and Upgrade/en_250.xml: Admin.Orders.OrderNotes.DownloadNotOfOrder, Admin.Orders.Products.AddNew.NotInOrderStore. No route, permission or setting changes.
• Attachments of notes created before uploads were tagged with the order (Download.ReferenceId, 2023) are no longer deleted together with their note. Deleting them would need the same check that keeps other stores' files safe, so they stay in the database.

Testing

1. dotnet build GrandNode.slnx: no errors.
2. dotnet test: Grand.Web.Admin.Tests 1647 (after the review fixes), Grand.Web.Store.Tests 184, Grand.Web.Vendor.Tests 53 passed. New tests: unit prices of an added order item, address not on the order (GET and POST), product outside the panel's store (GET and POST), POST-only note-add actions; review fixes: note files of another order are neither attached nor deleted, notes without a file skip the lookup, the product is checked against the order's store, a stale address id edits the order's address, a missing address redirects, a failed address validation keeps the typed values, a quantity below 1 is refused, the product search is limited to the order's store and to orders the panel can access, a refused note file returns Result = false with a message.
3. Fresh install with sample data, driven by Playwright, on the split commit. Orders placed through the storefront (cash on delivery): Label at local plugins #1 store product only, Deactivate Two Factor Auth #2 vendor A + store product, Print feature missing products #3 two vendor A products, Reservation inventory #4 vendor B product, Allow manging bestsellers manually directly on the product #5 by a customer registered in the store (Customer:RegisterCustomersPerStore=true).
   • Admin and store manager: the list shows every order and each edit page its own items; notes added to Label at local plugins #1 and Print feature missing products #3 appear only there (displayed to the customer, the e-mail is queued), deleting Print feature missing products #3's leaves Label at local plugins #1's; editing Deactivate Two Factor Auth #2's shipping address leaves Deactivate Two Factor Auth #2's billing and Label at local plugins #1's addresses unchanged; a product added to Reservation inventory #4 through the add-product page appears only on Reservation inventory #4 and the total grows by its price.
   • Vendors: A lists Deactivate Two Factor Auth #2, Print feature missing products #3, Allow manging bestsellers manually directly on the product #5 and B lists Reservation inventory #4; each edit page shows only the vendor's items; another order's Edit and PdfInvoice redirect to the list; PurchasedWithOrders on the vendor's product lists exactly its orders.
   • Customer orders tab: Admin shows Label at local plugins #1-Reservation inventory #4 for the admin account and Allow manging bestsellers manually directly on the product #5 for the buyer; Store shows Allow manging bestsellers manually directly on the product #5 for the buyer.
   • No console errors, no exceptions in the log except the mail-sending task (no SMTP server).
4. The fix commits, including the review fixes (Admin, Store and Vendor build), are covered by the unit tests above; they were not run through the browser.

[Copilot]

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