Country and state: Restrict deletion if addresses present #6305
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
mamhoff
force-pushed
the
country-dependent
branch
from
1465d62 to
449838a
Compare
github-actions
bot
added
the
changelog:solidus_backend
mamhoff
changed the title
mamhoff
force-pushed
the
country-dependent
branch
6 times, most recently
from
10a9d0a to
601f6c8
Compare
mamhoff
force-pushed
the
country-dependent
branch
2 times, most recently
from
d71d3b1 to
60b8a8f
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #6305 +/- ##
=======================================
Coverage 88.99% 88.99%
=======================================
Files 863 863
Lines 18425 18427 +2
=======================================
+ Hits 16398 16400 +2
Misses 2027 2027 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
tvdeyen
approved these changes
Jul 9, 2025
|
@mamhoff we need some manual conflict resolving here. |
adammathys
approved these changes
Jul 24, 2025
benjaminwil
approved these changes
Jul 24, 2025
jarednorman
approved these changes
Jul 24, 2025
This also removes a bogus spec that tested admin behavior when dealing with the situation where a country was removed that had been used for an address. This commit prevents that entire situation. The original issue that was solved with the PR that introduced the spec was spree/spree#3571 - allowing admins to add/remove countries, but even here the original developers did not deal with dependent records. For the use case described there - i would like to change the countries my store services - I think using zones is a better solution that deleting the country.
mamhoff
force-pushed
the
country-dependent
branch
from
60b8a8f to
8caa042
Compare
We're using `spree_countries.iso` as a primary key between prices and countries. In order to add a foreign key constraint, primary keys have to be unique. This adds the necessary uniqueness constraint. A few tests had to be amended to not create the same country many times.
This adds database-level foreign key constraints to tables that reference the countries table. If trying to delete a country, and there are addresses referencing that country, we want to restrict that. Addresses are immutable and must stay valid. If there are prices referencing the country, the same applies, we want to restrict. If the only thing holding us back from deleting a country is states, we can delete the state records as well.
For the database, a state is optional on an address. What this commit ensures is that any state ID entered into the `state_id` column on `spree_addresses` is actually present in the `spree_states` table.
mamhoff
force-pushed
the
country-dependent
branch
from
8caa042 to
737f092
Compare
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
5 participants
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 foreign key constraints,
inverse_ofanddependentoptions to relations between addresses, countries, and states. We want these records to be rock-solid.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: