-
-
Notifications
You must be signed in to change notification settings - Fork 494
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
WIP for Issue #487 #501
WIP for Issue #487 #501
Conversation
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.
Hi @jash-kothari, thanks for tackling this issue!
There are a few changes I'd like to see before this branch gets merged:
- In the PR description, include step-by-step instructions (including any necessary configuration steps) for how to verify your code changes and confirm behavior is good
- Add a simple spec that runs through the happy path for how this feature might get used
- Make sure
rubocop
is passing (runrubocop -a
to see failures and recommendations for how to fix)
I'll be able to give more concrete suggestions for code improvements once those first two are done.
Again, thanks for your hard work on this PR - it's going to make a big difference in the lives of our hard-working diaper banks' staff!
Hey @jash-kothari I really like how you started this PR! Is there any help you need or anything we can do to help you get this across the finish line? Please let us know if the changes that were requested don't make sense or you need additional feedback. |
@seanmarcia Hi need your help in writing test cases for mailer.I dont have any idea about writing test cases for mailers. |
|
Resolves Confirmation Email & Background Jobs #487
Description
Type of change
How Has This Been Tested?
Screenshots
Steps to check