Calling deliver_later for forms instead of deliver_now#16
Conversation
|
deliver_later needs to be serialized and several of the forms don't have db entries so needed to be reworked a bit. The request_mailer views for several of these use dot notation. I think this approach should work for those. (e.g. AffilliateBorrowRequestForm, SecurityIncidentReportForm) |
awilfox
left a comment
awilfox
left a comment
There was a problem hiding this comment.
Looks good overall, a few questions on some of the details.
| expect do | ||
| @form.submit! | ||
| end.to have_enqueued_job(ActionMailer::MailDeliveryJob).with( |
There was a problem hiding this comment.
We have expect { @form.submit! } in other places (included the AffiliateBorrowRequestFormSpec just above this file in the diff). I think we should be consistent with block syntax with these tests.
There was a problem hiding this comment.
rubocop complained and said that the format above "expect { @form.submit! }" shouldn't be used for multi line blocks. I don't agree with rubocop though. I think it looks better having be consistent
| expect do | ||
| invoice.submit! | ||
| end.to have_enqueued_job(ActionMailer::MailDeliveryJob) |
There was a problem hiding this comment.
As above, this should probably be expect { invoice.submit! }.to …
There was a problem hiding this comment.
same as above
anarchivist
left a comment
anarchivist
left a comment
There was a problem hiding this comment.
generally looks good to me - thank you for taking this on!
added struct file for affiliate borrow request moved struct into model for borrow_request removed borrow_request.rb struct file Changing doemoff_patron_email_form to use struct similar to affiliate_borrower Changing stack pass for to use deliver_later Changed deliver_now to deliver_later for more forms Update request_mailer to work with update to efees which uses deliver_later now removed ActiveJob::Base.queue_adapter = :test from affiliate_borrow_request_form, already a default
davezuckerman
force-pushed
the
AP-512-deliver-later
branch
from
c280680 to
10f2d80
Compare
|
@davezuckerman I've opened a new pull request, #19, to work on those changes. Once the pull request is ready, I'll request review from you. |
added struct file for affiliate borrow request
moved struct into model for borrow_request
removed borrow_request.rb struct file
Changing doemoff_patron_email_form to use struct similar to affiliate_borrower
Changing stack pass for to use deliver_later
Changed deliver_now to deliver_later for more forms