Sync Vendor ProductViewModelService to AdminShared's primary-constructor style - #788
Merged
Merged
Conversation
…tor style Grand.Web.AdminShared/Services/ProductViewModelService.cs (which Admin and Store both use directly) was refactored to a C# primary constructor at some point; this fork in Vendor never got the same treatment, so it still carried a classic constructor + 31 private fields. That style drift alone accounted for roughly half of the (unavoidable, real) diff between the two files - every method referencing an injected service showed as changed purely because one side used `_service` and the other `service`. Pure mechanical rename, scripted (not hand-edited): removed the field declarations and explicit constructor, converted the class declaration to a primary constructor with the same parameter list/order, and renamed every `_fieldName` reference to the matching parameter name throughout the file. No logic touched, no line reordered beyond what the syntax change requires. Diff to AdminShared's service dropped from ~1412 to 960 changed lines (mechanical noise removed; the remaining diff is the real vendor-specific behavior: VendorId assignment, vendor-scoped search, HasAccessToProduct filtering). Grand.Web.Vendor.Tests: 12/12 green, unmodified - the tests exercise exactly the renamed identifiers (constructor injection, field usage inside PrepareProducts/InsertProductModel/etc.), so a mismatched rename would have surfaced immediately as a compile error or a failing test. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
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.
Summary
Continuation of the
ProductController/ProductViewModelServiceduplication cleanup (#785, #786). This one targets the style half of the diff betweenGrand.Web.AdminShared/Services/ProductViewModelService.cs(used directly by Admin and Store) and its fork inGrand.Web.Vendor/Services/ProductViewModelService.cs.What changed
Grand.Web.AdminShared's service was refactored to a C# primary constructor at some point; the Vendor fork never got the same treatment and still carried a classic constructor + 31 private fields. That drift alone was responsible for roughly half of the (real, unavoidable) diff between the two files - every method touching an injected service showed as changed purely because one side wrote_serviceand the otherservice.This is a pure, scripted rename:
_fieldNamereference to the matching parameter name throughout the file.No logic touched, no line reordered beyond what the syntax change forces.
Result
Diff to
AdminShared's service dropped from ~1412 to 960 changed lines. The remaining diff is the real vendor-specific behavior (VendorIdassignment, vendor-scoped search,HasAccessToProductfiltering) - not noise.Testing
dotnet build src/Web/Grand.Web.Vendor→ cleandotnet test src/Tests/Grand.Web.Vendor.Tests→ 12/12, unmodified - the existing tests exercise exactly the renamed identifiers (constructor injection, field usage insidePrepareProducts/InsertProductModel/etc.), so a mismatched rename would have surfaced immediately as a compile error or a failing assertion.Scope note
This does not eliminate the fork - Vendor still has its own copy of the service, because it needs vendor-scoping instead of store-scoping and genuinely different authorization logic. Merging the two implementations into one parameterized service was considered and deliberately deferred: it's a much larger, higher-risk change needing far more test coverage than exists today. This PR only removes the stylistic noise that made the real diff harder to see.
🤖 Generated with Claude Code