-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
set_position conflicts with acts_as_list #5509
Conversation
# Regression test for https://github.com/spree/spree/issues/2744 | ||
describe "set_position" do | ||
it "sets variant position after creation" do | ||
variant = create(:variant) | ||
expect(variant.position).to_not be_nil | ||
end | ||
end | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we really want to remove this test? We still want to persist this behaviour, even if we're removing the callback, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if I should write a test for "acts as list" only for Variant, but we feel safer with a spec.
So I added this test! 77aceb0
a560a6b
to
77aceb0
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5509 +/- ##
==========================================
- Coverage 88.65% 88.65% -0.01%
==========================================
Files 627 627
Lines 15037 15034 -3
==========================================
- Hits 13331 13328 -3
Misses 1706 1706 ☔ View full report in Codecov by Sentry. |
Summary
Fixes #5508
Remove set_position callback
I thought about changing the test as in this diff, but decided to just remove it since this is just testing acts_as_list.
Checklist
Check out our PR guidelines for more details.
The following are mandatory for all PRs:
The following are not always needed: