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

Allow emails to be turned off per deployment #720

Merged
merged 3 commits into from Nov 17, 2016

Conversation

chrisortman
Copy link
Contributor

We needed to be able to disable sending of emails to real users in our test and staging environments, but relying on a check for the Rails environment is problematic and was duplicated in several places in the code.

This change removes the duplication and contains the logic in a single place. It also moves the setting to application.yml.

This setting is intentionally off by default so that someone couldn't accidentally deploy to testing and have it start sending emails on them.

I created this branch based on the release-1.8.0 tag thinking that it would then not pick up any changes that were not ready to be released. But it looks like the tags get created off musc_production but musc_production has stuff that's not in master?

@wtholt
Copy link
Contributor

wtholt commented Nov 2, 2016

Please rebase this branch to our updated master. We just incorporated a lot of new changes into our current master branch. Fix the conflicts accordingly.

@@ -51,8 +51,7 @@ def notification_received(user, ssr)
private

def send_message subject, is_service_provider='false', ssr_id=''
email = Rails.env == 'production' ? @send_to.email : DEFAULT_MAIL_TO
subject = Rails.env == 'production' ? subject : "[#{Rails.env.capitalize} - EMAIL TO #{@send_to.email}] #{subject}"
email = @send_to.email
Copy link
Contributor

Choose a reason for hiding this comment

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

subject is missing here now. Is this intentional?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes intentional, subject is passed straight through to mail.

Otherwise it would have been subject = subject I think

@chrisortman
Copy link
Contributor Author

Rebase completed.

Please see also the lines I've marked with ##Review they didn't look correct in the original code to me.

@jleonardw9
Copy link
Contributor

Hey Chris. Can you look into the test failures? We'll get it merged in as soon as it's passing on Travis.

@jleonardw9 jleonardw9 merged commit 373109e into sparc-request:master Nov 17, 2016
@chrisortman chrisortman deleted the email-sending-rules branch July 19, 2017 13:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants