simplify project create/delete mailer logic

 - test logic
 - remove from default test setup since it is only a weird edge-case
 - dry-up duplicated logic

.env.test

@@ -6,7 +6,6 @@ DATADOG_APPLICATION_KEY=dappkey
 JENKINS_URL=http://www.test-url.com
 JENKINS_USERNAME=[email protected]
 JENKINS_API_KEY=japikey
-PROJECT_CREATED_NOTIFY_ADDRESS=[email protected]
 RUBY_MIME_TYPES_LAZY_LOAD=true
 AUTH_GITHUB=1
 GITHUB_CLIENT_ID=github-id

app/controllers/projects_controller.rb

@@ -56,17 +56,21 @@ def current_project
   end
 
   def create_callback
-    if Rails.application.config.samson.project_created_email
-      ProjectMailer.created_email(current_user, @project).deliver_now
+    if to = created_email
+      ProjectMailer.created_email(to, current_user, @project).deliver_now
     end
   end
 
   def destroy_callback
-    if Rails.application.config.samson.project_deleted_email
-      ProjectMailer.deleted_email(current_user, @project).deliver_now
+    if to = (ENV["PROJECT_DELETED_NOTIFY_ADDRESS"] || created_email)
+      ProjectMailer.deleted_email(to, current_user, @project).deliver_now
     end
   end
 
+  def created_email
+    ENV["PROJECT_CREATED_NOTIFY_ADDRESS"]
+  end
+
   def allowed_includes
     [
       :environment_variable_groups,

app/mailers/project_mailer.rb

@@ -1,11 +1,11 @@
 # frozen_string_literal: true
 class ProjectMailer < ApplicationMailer
-  def created_email(user, project)
-    build_mail user, project, Rails.application.config.samson.project_created_email, 'created'
+  def created_email(to, user, project)
+    build_mail user, project, to, 'created'
   end
 
-  def deleted_email(user, project)
-    build_mail user, project, Rails.application.config.samson.project_deleted_email, 'deleted'
+  def deleted_email(to, user, project)
+    build_mail user, project, to, 'deleted'
   end
 
   private

config/application.rb

@@ -86,11 +86,6 @@ class Application < Rails::Application
     config.samson.email.prefix = ENV["EMAIL_PREFIX"].presence || "DEPLOY"
     config.samson.email.sender_domain = ENV["EMAIL_SENDER_DOMAIN"].presence || "samson-deployment.com"
 
-    # Email notifications
-    config.samson.project_created_email = ENV["PROJECT_CREATED_NOTIFY_ADDRESS"]
-    config.samson.project_deleted_email = ENV["PROJECT_DELETED_NOTIFY_ADDRESS"].presence ||
-      ENV["PROJECT_CREATED_NOTIFY_ADDRESS"]
-
     # Tired of the i18n deprecation warning
     config.i18n.enforce_available_locales = true
 

test/controllers/projects_controller_test.rb

@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 require_relative '../test_helper'
 
-SingleCov.covered! uncovered: 2
+SingleCov.covered!
 
 describe ProjectsController do
   def fields_disabled?
@@ -268,14 +268,18 @@ def fields_disabled?
 
           assert_redirected_to projects_path
           request.flash[:notice].wont_be_nil
+
+          refute ActionMailer::Base.deliveries.last
         end
       end
 
       it "sends deletion notification" do
-        delete :destroy, params: {id: project.to_param}
-        mail = ActionMailer::Base.deliveries.last
-        mail.subject.include?("Samson Project Deleted")
-        mail.subject.include?(project.name)
+        with_env PROJECT_DELETED_NOTIFY_ADDRESS: '[email protected]' do
+          delete :destroy, params: {id: project.to_param}
+          mail = ActionMailer::Base.deliveries.last
+          mail.subject.include?("Samson Project Deleted")
+          mail.subject.include?(project.name)
+        end
       end
 
       it "does not fail when validations fail" do
@@ -303,41 +307,36 @@ def fields_disabled?
     end
 
     describe "#create" do
-      before do
-        post :create, params: params
-      end
-
-      describe "with valid parameters" do
-        let(:params) do
-          {
-            project: {
-              name: "Hello",
-              repository_url: "git://foo.com/bar"
-            }
+      let(:params) do
+        {
+          project: {
+            name: "Hello",
+            repository_url: "git://foo.com/bar"
           }
-        end
-        let(:project) { Project.where(name: "Hello").first }
-
-        it "redirects to the new project's page" do
-          assert_redirected_to project_path(project)
-        end
+        }
+      end
 
-        it "creates a new project" do
-          project.wont_be_nil
-          project.stages.must_be_empty
-        end
+      it "redirects to the new project's page" do
+        post :create, params: params
+        project = Project.last
+        assert_redirected_to project_path(project)
+        refute ActionMailer::Base.deliveries.last
+      end
 
-        it "notifies about creation" do
+      it "notifies about creation" do
+        with_env PROJECT_CREATED_NOTIFY_ADDRESS: '[email protected]' do
+          post :create, params: params
           mail = ActionMailer::Base.deliveries.last
           mail.subject.include?("Samson Project Created")
           mail.subject.include?(project.name)
         end
       end
 
       describe "with invalid parameters" do
-        let(:params) { {project: {name: ""}} }
+        before { params[:project][:name] = '' }
 
         it "renders new template" do
+          post :create, params: params
           assert_template :new
         end
       end

test/mailers/project_mailer_test.rb

@@ -9,7 +9,7 @@ class ProjectMailerTest < ActionMailer::TestCase
 
   describe "#created_email" do
     it "contains user and project name" do
-      ProjectMailer.created_email(user, project).deliver_now
+      ProjectMailer.created_email('[email protected]', user, project).deliver_now
       mail_sent = ActionMailer::Base.deliveries.last
       assert mail_sent.subject.include?('Foo')
       assert mail_sent.body.include?('Admin')
@@ -19,7 +19,7 @@ class ProjectMailerTest < ActionMailer::TestCase
 
   describe "#deleted_email" do
     it "contains user and project name" do
-      ProjectMailer.deleted_email(user, project).deliver_now
+      ProjectMailer.deleted_email('[email protected]', user, project).deliver_now
       mail_sent = ActionMailer::Base.deliveries.last
       assert mail_sent.subject.include?('Foo')
       assert mail_sent.body.include?('Admin')

test/test_helper.rb

@@ -280,6 +280,7 @@ def use_test_routes(controller)
 
   after do
     Warden.test_reset!
+    ActionMailer::Base.deliveries.clear
   end
 
   # overrides warden/test/helpers.rb which does not work in controller tests