Add dependent/inverse_of options to product-related models #6271
Conversation
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
github-actions
bot
added
the
changelog:solidus_core
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:
|
mamhoff
force-pushed
the
foreign-key-inverse-of-product-taxons
branch
from
6a44e48 to
dd9bb7a
Compare
kennyadsl
reviewed
Jun 7, 2025
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 |
There was a problem hiding this comment.
Same here, if there are rows without these constraints respected, will the migration fail?
mamhoff
force-pushed
the
foreign-key-inverse-of-product-taxons
branch
from
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.
mamhoff
force-pushed
the
foreign-key-inverse-of-product-taxons
branch
3 times, most recently
from
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.
mamhoff
force-pushed
the
foreign-key-inverse-of-product-taxons
branch
from
1d6044f to
04493b1
Compare
kennyadsl
approved these changes
Jun 20, 2025
tvdeyen
approved these changes
Jun 20, 2025
github-project-automation
bot
moved this from In Progress
to Done
in Solidus Public Roadmap
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: