Fix the 'Send Mailer' checkbox selection #1659
Conversation
|
also #1659 |
|
lol... of course! this wasn't meant to be posted here. Sorry about the confusion. |
| expect { | ||
| find(".ship-shipment-button").click | ||
| wait_for_ajax | ||
| }.not_to change{ ActionMailer::Base.deliveries.count } |
There was a problem hiding this comment.
I'm not a big fan of the test.
It's testing with .not_to without a corresponding test to check when it does change. For all we know, it could not deliver anything, or this test isn't configured correctly and we wouldn't know it.
There was a problem hiding this comment.
We can wrap the test above this one (it "can ship a completed order") with the opposite email expectation. You'll probably want to rebase first, since that spec was changed in #1668
it "can ship a completed order" do
expect {
find(".ship-shipment-button").click
expect(page).to have_content("shipped package")
expect(order.reload.shipment_state).to eq("shipped")
}.to change{ ActionMailer::Base.deliveries.count }.by(1)
end
|
|
||
| expect { | ||
| find(".ship-shipment-button").click | ||
| wait_for_ajax |
There was a problem hiding this comment.
I would like to see the wait_for_ajax eliminated. We should expect something on the page to happen.
There was a problem hiding this comment.
@cbrunsdon has removed all the wait_for_ajax in #1668 🎉.
We can replace the wait_for_ajax here with expect(page).to have_content("shipped package")
| @@ -37,6 +37,15 @@ | |||
| expect(page).to have_content("shipped package") | |||
| expect(order.reload.shipment_state).to eq("shipped") | |||
| end | |||
|
|
|||
| it "doesn't send a shipping confirmation email when ask to suppress the mailer" do | |||
| find("#send_mailer").set(false) | |||
There was a problem hiding this comment.
This can be uncheck 'Send Mailer', which reads a little nicer :)
|
|
||
| expect { | ||
| find(".ship-shipment-button").click | ||
| wait_for_ajax |
There was a problem hiding this comment.
@cbrunsdon has removed all the wait_for_ajax in #1668 🎉.
We can replace the wait_for_ajax here with expect(page).to have_content("shipped package")
| expect { | ||
| find(".ship-shipment-button").click | ||
| wait_for_ajax | ||
| }.not_to change{ ActionMailer::Base.deliveries.count } |
There was a problem hiding this comment.
We can wrap the test above this one (it "can ship a completed order") with the opposite email expectation. You'll probably want to rebase first, since that spec was changed in #1668
it "can ship a completed order" do
expect {
find(".ship-shipment-button").click
expect(page).to have_content("shipped package")
expect(order.reload.shipment_state).to eq("shipped")
}.to change{ ActionMailer::Base.deliveries.count }.by(1)
end
The user selection for suppressing the shipping confirmation email is not respected when shipping a package from the admin. It always sends it.