-
-
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
Deprecate simple_current_order #1915
Conversation
2032ce5
to
f361b4f
Compare
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.
Great!
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 have a minor nitpick about writing the deprecation in a different way.
Would you also elaborate a little bit in the commit message why we think it's a good idea to deprecate this method?
We would also want this to appear in the CHANGELOG.md
.
@@ -14,8 +14,9 @@ module Order | |||
helper_method :simple_current_order | |||
end | |||
|
|||
# Used in the link_to_cart helper. | |||
def simple_current_order |
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.
You can use
deprecate simple_current_order: :current_order, deprecator: Spree::Deprecation
Which does the same thing more concisely.
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 like this a lot better, I'll make the change. Thanks!
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.
👍 thanks!
Using the current_order_params sets the store and user similar to the current_order method.
The `#simple_current_order` method is duplicating many of the functions already found in `#current_order`. `simple_current_order` was finds an order by token or user, updates ip_address, and creates an order if one is not found. All functions that are already accomplised in `#current_order` which also does more than `#simple_current_order` making it the preferred method.
f361b4f
to
26d81b2
Compare
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.
👍
Fixes the bug found in #1700 and deprecates
simple_current_order
because it's function is duplicated withcurrent_order