Remove user prereq from First Order promorule #2928
Conversation
|
I can see a possible case for reintroducing the guard with a switch related to the config setting that requires login. |
There was a problem hiding this comment.
Hi @fastjames, could you please have a look at why the CI builds are failing?
I also left some comments you should address once your branch is rebased 🙌
Thanks for the PR!
fastjames
force-pushed
the
first_order_promo_prereq
branch
from
a54c1a6 to
61c8b07
Compare
|
@aitbw thanks for the feedback, I have made the requested changes and rebased against today's master. |
fastjames
force-pushed
the
first_order_promo_prereq
branch
2 times, most recently
from
e4cba8b to
764630f
Compare
fastjames
force-pushed
the
first_order_promo_prereq
branch
from
764630f to
c7fe5c9
Compare
There was a problem hiding this comment.
@fastjames I left a comment for a small possible change, thank you for this PR 👏
| end | ||
| end | ||
|
|
||
| def fill_in_address |
There was a problem hiding this comment.
I think that, by defining this method here, you're polluting the main namespace. I think that this method can be moved inside the Rspec.feature block or, since this method is used only once here, it could be removed and the code inlined directly where it's used.
There was a problem hiding this comment.
Good point, I don't want to be a polluter. I have moved it into the feature block.
fastjames
force-pushed
the
first_order_promo_prereq
branch
from
c7fe5c9 to
1ab7f10
Compare
| fill_in "#{address}_phone", with: "(555) 555-5555" | ||
| end | ||
| end | ||
|
|
There was a problem hiding this comment.
Layout/TrailingBlankLines: 1 trailing blank lines detected.
fastjames
force-pushed
the
first_order_promo_prereq
branch
from
1ab7f10 to
c11cddf
Compare
When a customer applies a promotion with a "first order" rule, the rule checks for a User or email associated with the order. This is not strictly necessary, since the order can be checked after the coupon is applied and if the user is found to have an existing order the coupon gets removed then.
fastjames
force-pushed
the
first_order_promo_prereq
branch
from
c11cddf to
0e0b126
Compare
When a customer applies a promotion with a "first order" rule, the rule
checks for a User or email associated with the order. This is not
strictly necessary, since the order can be checked after the coupon is
applied and if the user is found to have an existing order the coupon
gets removed then.