Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FIX SQLException: no such column: spree_promotions.match_policy #6177

Closed
wants to merge 2 commits into from

Conversation

ringe
Copy link

@ringe ringe commented Mar 5, 2025

Summary

New Solidus install on existing app fails due to legacy column not present.

Versions

gem "rails", "~> 8.0.1"
gem "solidus", "~> 4.5"
gem "solidus_support", ">= 0.12.0"
gem "solidus_support", ">= 0.12.0"
gem "solidus_admin", ">= 0.2"

Commands

bundle add solidus
bundle remove solidus_auth_devise
bin/rails generate solidus:install --no-sample --authentication=none --payment-method=none --mount-point=/shop --frontend=none

Checklist

@ringe ringe requested a review from a team as a code owner March 5, 2025 14:09
@github-actions github-actions bot added the changelog:solidus_legacy_promotions Changes to the solidus_legacy_promotions gem label Mar 5, 2025
@kennyadsl
Copy link
Member

Thanks @ringe! Can you add the versions used and which commands you executed for this error to happen?

Copy link

codecov bot commented Mar 5, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.81%. Comparing base (7f148a4) to head (58501f3).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6177      +/-   ##
==========================================
+ Coverage   86.65%   88.81%   +2.16%     
==========================================
  Files         515      836     +321     
  Lines       11900    18164    +6264     
==========================================
+ Hits        10312    16133    +5821     
- Misses       1588     2031     +443     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@fthobe
Copy link
Contributor

fthobe commented Mar 8, 2025

@ringe are you still interested into bringing this upstream?
Please also provide a detailed commit message.

@ringe
Copy link
Author

ringe commented Mar 8, 2025

@ringe are you still interested into bringing this upstream? Please also provide a detailed commit message.

I would like to bring this yes, what are you missing in the commit message?

@fthobe
Copy link
Contributor

fthobe commented Mar 8, 2025

In your case a brief comment explaining the change and the origin of this issue if it's limited to certain versions.
You can find all details here: Contribution Guidelines

@jarednorman
Copy link
Member

jarednorman commented Mar 10, 2025

@ringe The PR/commit message is missing the answers to these questions:

  1. What command were you running that resulted in the error?
  2. What was the error?
  3. Why is what your change the correct solution?

I think what you've found is a gotcha where you're using the new promo system, but a migration that depends on the old promo stuff is running, so your fix very well may be correct. It's just hard for me to know without a little more digging/context.

@fthobe
Copy link
Contributor

fthobe commented Mar 21, 2025

@ringe you did great work here, can we get this over the finish line?

@ringe
Copy link
Author

ringe commented Mar 25, 2025

What command were you running that resulted in the error?
Unfortunately, I can't recreate the issue at the moment.
I tried adding solidus to an existing Rails 8 app at the time.

What was the error?
In case you run the SetPromotionsWithAnyPolicyToAllIfPossible migration after the :match_policy column was removed, or if it was never created - you get a "SQLException: no such column: spree_promotions.match_policy" error

Why is what your change the correct solution?
Maybe it isn't. Could be the install command should avoid generating this migration on a new install.
In case this migration is generated, this change avoids a crash by checking for the existence of the column before using it.
If the column do not exist, do not apply the migration.

I'm not sure what to write different in the commit message when I can't reproduce the error on a new rails 8.0.1 app.

@jarednorman
Copy link
Member

jarednorman commented Mar 25, 2025

Thanks for following up! I'm going to close this because you can't reproduce it, but if you (or anyone else) sees this issue, let's reopen this and get it resolved.

@ringe
Copy link
Author

ringe commented Mar 25, 2025

Thanks for your efforts and guidance. 😊

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog:solidus_legacy_promotions Changes to the solidus_legacy_promotions gem
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants