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

Misc fixes to allow dynamic mailer name and dynamic wait while sendin… #429

Closed
wants to merge 1 commit into from

Conversation

ausangshukla
Copy link

…g emails

Pull Request

Summary:
These changes enable for mailer and wait to take method names as symbols

Related Issue:

Description:
Its a pretty simple change

  1. mailer = evaluate_option(:mailer), to enable the mailer to be dynamic and based on a method defined in the event.
  2. option.is_a?(Symbol) && context.event.respond_to?(option), allows the wait time to be dynamic and based on a method call.

Testing:
Ive run my applications tests against this, but have not written any new tests specifically for these changes.

Screenshots (if applicable):

Checklist:

  • Code follows the project's coding standards
  • Tests have been added or updated to cover the changes
  • Documentation has been updated (if applicable)
  • All existing tests pass
  • Conforms to the contributing guidelines

Additional Notes:

@excid3
Copy link
Owner

excid3 commented Mar 20, 2024

I actually just commited a fix here: 8da57f8

Is there a case this doesn't handle?

@ausangshukla
Copy link
Author

This may work for the dynami mailer, but not for the wait time.

@excid3
Copy link
Owner

excid3 commented Mar 20, 2024

Can you provide an example?

@ausangshukla
Copy link
Author

This is what I have in my actual prod code. The reason is to prevent aws from getting flooded, which results in throttling errors. So we need to delay the sending of emails. However when sending a bunch of emails, we cant use the same delay, we need to spread the delay over some period, by randomizing it, so that aws does not get flooded. Hope this makes sense

deliver_by :email do |config|
config.mailer = :mailer_name
config.method = :email_method
config.params = :email_data
config.wait = :email_delay
end

def email_delay(_notification = nil)
# Randomize the delay so we dont flood aws SES
delay_seconds = entity.entity_setting.email_delay_seconds
delay_seconds = delay_seconds.positive? ? rand(1..delay_seconds) : rand(1..300)
Rails.env.development? ? 0 : delay_seconds
end

@ausangshukla
Copy link
Author

ausangshukla commented Mar 20, 2024

I agree with your changes in 8da57f8, can replace the one I suggested, for dynamic mailer. But you need the other change for dynamic wait

@ausangshukla
Copy link
Author

Also note that in version < 2 I could do the following
deliver_by :email, mailer: :mailer_name, method: :email_method, format: :email_data, delay: :email_delay

And the delay would be dynamic, this is the prob Im trying to solve above.

@excid3
Copy link
Owner

excid3 commented Mar 20, 2024

Wait should already work, but it will be evaluated against methods in the notification_methods block since it would apply to the individual recipients.

class MyNotifier < Noticed::Event
  deliver_by :email do |config|
    config.mailer = :mailer_name
    config.method = :email_method
    config.params = :email_data
    config.wait = :email_delay
  end

  notification_methods do
    def email_delay(_notification = nil)
      # Randomize the delay so we dont flood aws SES
      delay_seconds = entity.entity_setting.email_delay_seconds
      delay_seconds = delay_seconds.positive? ? rand(1..delay_seconds) : rand(1..300)
      Rails.env.development? ? 0 : delay_seconds
    end
  end
end

@ausangshukla
Copy link
Author

ausangshukla commented Mar 21, 2024 via email

@excid3 excid3 closed this Mar 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants