Skip to content
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

3298 no send update if no change #3361

Closed
wants to merge 8 commits into from
Closed

Conversation

ivar
Copy link
Contributor

@ivar ivar commented Aug 15, 2018

Fixes #3347 plus lots of cleanups.

  • merged in Rob's work from seller_order_update_no_recipient_3347 branch
  • removes packing list PDF & csv from supplier update emails when supplier's part of the order was cancelled. (As per Rob in slack ) (though it's TBD if this code works / ever gets executed anyway...)

The following specs have been reviewed and run green

  • spec/interactors/send_update_emails_spec.rb
  • spec/mailers/order_mailer_spec.rb
  • spec/models/order_spec.rb

That said, I couldn't reproduce the issue (#3298) via tests locally so there is still a possibility that the root cause is still outstanding.

@ivar ivar added the WIP label Aug 15, 2018
@ivar ivar force-pushed the 3298-no_send_update_if_no_change branch from b80435e to 56bb7d1 Compare August 15, 2018 22:58
@ivar ivar force-pushed the 3298-no_send_update_if_no_change branch from d751f1e to d195149 Compare August 27, 2018 20:40
@ivar ivar force-pushed the 3298-no_send_update_if_no_change branch from 8acdeff to a32b1a7 Compare August 28, 2018 16:41
mail(
to: to_list,
subject: "New Invoice"
subject: 'Thank you for your order'
Copy link
Contributor Author

@ivar ivar Aug 28, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is a bit misleading, I moved some of the methods around and with the changes/simplification of the class, the diff algorithm isn't making it clear that this is actually a different method.

@@ -2,48 +2,8 @@ class SendUpdateEmails
include Interactor

def perform
#return if Rails.env.production?
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add require_in_context

@@ -118,6 +113,7 @@
end

describe ".orders_for_buyer" do

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extra line

@ivar ivar closed this Aug 28, 2018
@ivar ivar deleted the 3298-no_send_update_if_no_change branch August 28, 2018 21:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready for review hey! this PR needs your love
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants