Repository navigation
refactor(arch): publish DiscountCeiling in Commerce's contract - #1118
Merged
Merged
Conversation
Part of #1116. Farm stores the ceiling on Account and validates it in its settings handler, while Commerce applies it at confirm; both now read one published value type from Domain/Modules/Commerce/Contracts. The CW1004 census falls from 57 to 53. Namespace move only: no behaviour change.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of #1116 (cleanup PR 1). It does not close the issue.
Moves
DiscountCeilingfromDomain/Modules/Commerce/Sales/toDomain/Modules/Commerce/Contracts/, unchanged apart from its namespace. Farm stops reaching a Commerce type outside the contract.CW1004 census (
tools/architecture/cw1004-census.sh): 57 → 53. The 4 removed rows are Farm →Commerce.Sales.DiscountCeilingatAccount.cs:90,FarmModule.cs:64,UpdateFarmSettingsHandler.cs:150andUpdateFarmSettingsValidator.cs:98. ThreeConfirmSaleHandlerrows only moved down one line because of the addedusing. No other row changed.Proofs and the policy review follow in an edit.
Policy review: why
DiscountCeilingbelongs in Commerce's public APIMoving a type into
Contracts/changes what the guards let peers and adapters name, so this is a policy change, not just a move.It is already a cross-module value.
Account.MaxDiscountBasisPointsis exposed asAccount.MaxDiscount.UpdateFarmSettingsValidatorandUpdateFarmSettingsHandleruseDiscountCeiling.TryParsePercent.FarmModulefillsFarmSettingsDetailsfrom.Percent.ConfirmSaleHandler,SalesOrder).Publishing one type keeps one rule for parsing, range (
MaxBasisPoints) and comparison. Otherwise Farm would need a parser of its own that could drift.It is a value, not behaviour or state: a
readonly record structholding basis points, with parse, format and compare members. It names no entity, aggregate or persistence type.ModuleContractRealAssemblyTestsnow walks it (see proofs).What it opens: any peer or adapter may now name
DiscountCeiling. That is intended: the ceiling is part of the farm-settings surface thatIFarmModulealready exposes as a percent. It gives no access to orders, lines or the confirm path.Not done: moving ownership of the type to Farm. That would be a design change, and Commerce would then reach into Farm's contract for its own pricing rule.
Proofs (head
38cefb15)DiscountCeilingrows removed; 3ConfirmSaleHandlerrows moved down one line (addedusing)DiscountCeilingby qualified name;TenantBypassRealTreeTests4/4 with the registries untoucheddotnet ef migrations has-pending-model-changes: "No changes have been made to the model since the last migration" (AccountConfigurationignoresMaxDiscount; the snapshot never named the type)SalesOrder? PurityProbetoDiscountCeilingmakesModuleContractRealAssemblyTestsfail ("DiscountCeiling.FromBasisPointsexposes …SalesOrderviaDiscountCeiling.PurityProbe"); restoredAdapterReachRealTreeTests2,PeerContractRealTreeTests1,ModuleContractRealAssemblyTests1,ContractDerivationTests1,ModuleLedgerRealTreeTests2,CouplingMatrixRealTreeTests3,ModuleEdgeAnalyzerTests14,NamespaceFolderAgreementTests10,AdapterTierRealTreeTests3,AccessOwnershipClaimTests2,InsightsReadOnlyTests25,TenantBypassRealTreeTests4,SeamSurfaceRealAssemblyTests2,TableOwnerRealModelTests2,CompatibilityExceptionRealTreeTests2DiscountCeilingTests38,SalesOrderCeilingTests16,AccountSettingsTests39,UpdateFarmSettingsValidatorTests62dotnet build Cluckwork.slnxwith repo defaults: 0 warnings, 0 errorsFive now-unused
using Cluckwork.Domain.Modules.Commerce.Sales;lines were removed: each removal was kept only if the solution still built. The Farm → Commerce edge reason inModules/Farm.csnow names the type's new place; the edge's symbols are unchanged.