Fix discount conditions on collections#2345
Merged
glennjacobs merged 15 commits intolunarphp:1.xfrom Dec 8, 2025
Merged
Conversation
- Added whereDiscountableType() checks to whereDoesntHave subqueries in scopeCollections, scopeProducts, and scopeProductVariants - Ensures proper filtering when discounts have mixed discountable types - Added comprehensive test coverage for all three scope methods - Tests verify correct behavior with mixed types and edge cases
- Add discountable_type filtering to products() and productVariants() scopes - Ensures scopes only match discountables of the correct type - Add comprehensive tests covering the new type filtering behavior
Keep local implementation of collections scope tests using pivot table relationships
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR fixes discount conditions on collections by adding brand checks and separating collection discount relationships from the generic discountable table. The changes ensure that collection discounts use a dedicated collection_discount pivot table while brand and product discounts use the brand_discount and discountables tables respectively.
Key Changes:
- Added brand scope filtering to discount queries
- Separated collection conditions into a dedicated relation manager
- Fixed table prefix handling for pivot table queries
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| packages/core/src/Models/Discount.php | Added brands scope method and updated collections/products/variants scopes to use proper pivot table names with prefix |
| packages/core/src/Managers/DiscountManager.php | Added brand condition checking to discount manager query |
| packages/admin/src/Filament/Resources/DiscountResource/RelationManagers/ProductConditionRelationManager.php | Removed collection handling from product conditions |
| packages/admin/src/Filament/Resources/DiscountResource/RelationManagers/CollectionConditionRelationManager.php | New relation manager for collection conditions using collection_discount pivot |
| packages/admin/src/Filament/Resources/DiscountResource/RelationManagers/CollectionLimitationRelationManager.php | Added pivot table type filtering |
| packages/admin/src/Filament/Resources/DiscountResource/Pages/EditDiscount.php | Grouped product and collection conditions together |
| packages/admin/src/Filament/Resources/DiscountResource.php | Removed duplicate relation managers and added collection condition manager |
| packages/admin/resources/lang/en/discount.php | Updated language strings to clarify product/variant vs collection conditions |
| tests/core/Unit/Models/DiscountTest.php | Added comprehensive tests for collections, brands, products, and variants scopes |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
glennjacobs
approved these changes
Dec 8, 2025
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.
This PR builds on the work done in #2250
It:
It's breaking, but for v2 we should consolidate everything into the discountable table - I missed these when I did this initial piece of work. It would make the UI much more streamlined and from a data point of view reduce the redundancy of the tables.