-
-
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
Consolidation of promotion code batch form fields into partial. #3957
Conversation
@cpfergus1 thanks, this looks good to me! I'm not too familiar with this specific part of the code, so I'll let @spaghetticode (who authored the original issue) chime in first to see if this is what he had in mind. |
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.
@cpfergus1 thanks for working on this, the changes look good to me 👍
No problem! Glad to head the changes are acceptable |
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.
@cpfergus1 thanks! Just have one small comment and then we're good. 🙂
backend/app/views/spree/admin/promotion_code_batches/_form_fields.html.erb
Outdated
Show resolved
Hide resolved
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 Connor!
Description
This pull request resolves issue 3952
Implemented field for email on pathway 1 for creating promotion code batches (during promotion creation).
Implemented partial for form fields when creating promotion code batches and refactored promotion code batch creation forms.
Implemented conditional for "per code usage limit" to display when promotion id is not present in the page's parameters to prevent breaking pathway 2 for creating promotion code batches
All tests are passing
Pathway 1
Pathway 2
If needed you can reference another PR or issue here, e.g.:
Ref #3952
Checklist: