Add dependent/inverse_of options to product-related models#6271
Merged
tvdeyen merged 6 commits intosolidusio:mainfrom Jun 20, 2025
Merged
Add dependent/inverse_of options to product-related models#6271tvdeyen merged 6 commits intosolidusio:mainfrom
tvdeyen merged 6 commits intosolidusio:mainfrom
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #6271 +/- ##
=======================================
Coverage 88.95% 88.95%
=======================================
Files 860 860
Lines 18422 18422
=======================================
+ Hits 16387 16388 +1
+ Misses 2035 2034 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
6a44e48 to
dd9bb7a
Compare
kennyadsl
reviewed
Jun 7, 2025
Member
kennyadsl
left a comment
There was a problem hiding this comment.
I'm all in for this, but if I got it correctly, this change might be breaking and require some preliminary work on the database before the migrations succeed.
Probably it's just a matter of being explicit about this possibility, I would not block the PR but let's talk about it.
core/db/migrate/20250604072105_add_fk_products_variant_property_rules.rb
Show resolved
Hide resolved
|
|
||
| class AddFkToProductProperties < ActiveRecord::Migration[7.0] | ||
| def change | ||
| add_foreign_key :spree_product_properties, :spree_products, column: :product_id, null: false |
Member
There was a problem hiding this comment.
Same here, if there are rows without these constraints respected, will the migration fail?
dd9bb7a to
50447f0
Compare
Variant property rules belong to the product, if the product goes, the property rules can go as well. For the master variant and the non-master variants, we specify to do nothing when deleting the product, because they will be deleted from the `variants_including_master` association.
When deleting a taxonomy, its root taxon will delete all taxons, so the taxons association does not have to do it. The has_many has the inverse for taxon already defined, so we won't define another inverse.
5a63bcd to
1d6044f
Compare
This model belongs to a product, and is useless without it. Remove the `optional` property from the `belongs_to` declaration and add a foreign key. This will also delete useless variant property rules.
Entries in this join table are useless if either "arm" of the join is not present. Remove the `optional` property from the `belongs_to` declarations, add a foreign key constraint.
This is a join table, both values must be present and pointing to an entry in the correct table.
This join table needs both references to be pointing to valid records. A uniqueness index for the taxon id in this table is already present.
1d6044f to
04493b1
Compare
kennyadsl
approved these changes
Jun 20, 2025
tvdeyen
approved these changes
Jun 20, 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.
Summary
This adds
dependent/inverse_ofoptions tohas_manyrelations around thespree_productstable. It also adds some foreign key constraints for join models.Extracted from #6240 .
Checklist
Check out our PR guidelines for more details.
The following are mandatory for all PRs:
The following are not always needed: