Merge pull request #1529 from YaleSTC/1524_contact_form_55

[1524] Resolve issues with contact form

Author: orenyk

CHANGELOG.md

@@ -10,6 +10,7 @@
 * Terms of Service checkbox correctly determines if editable ([#1497](https://github.com/YaleSTC/reservations/issues/1497)).
 * Fixed some strange behavior with the availability calendar ([#1498](https://github.com/YaleSTC/reservations/issues/1498)).
 * Directly accessing /reservations/new now redirects to root_path ([#1502](https://github.com/YaleSTC/reservations/issues/1502)).
+* The contact form now correctly falls back to the admin e-mail if no separate e-mail is set ([#1524](https://github.com/YaleSTC/reservations/issues/1524)).
 
 ### Changed
 * Reservations now defaults to deleting missed reservations after one week ([#1491](https://github.com/YaleSTC/reservations/issues/1491)).

Gemfile

@@ -55,7 +55,7 @@ group :development, :test do
   gem 'pry-byebug', '~> 3.0.1'
   gem 'pry-stack_explorer', '~> 0.4.9.2'
   gem 'pry-remote', '~> 0.1.8'
-  gem 'letter_opener', '~> 1.3.0'
+  gem 'letter_opener', '~> 1.4.1'
   gem 'letter_opener_web', '~> 1.3.0'
   gem 'factory_girl_rails', '~> 4.5.0'
   gem 'rspec-rails', '~> 3.2.0'

Gemfile.lock

@@ -27,7 +27,7 @@ GEM
       minitest (~> 5.1)
       thread_safe (~> 0.1)
       tzinfo (~> 1.1)
-    addressable (2.3.7)
+    addressable (2.3.8)
     arel (5.0.1.20140414130214)
     ast (2.0.0)
     astrolabe (1.3.0)
@@ -183,7 +183,7 @@ GEM
     kgio (2.9.3)
     launchy (2.4.3)
       addressable (~> 2.3)
-    letter_opener (1.3.0)
+    letter_opener (1.4.1)
       launchy (~> 2.2)
     letter_opener_web (1.3.0)
       actionmailer (>= 3.2)
@@ -446,7 +446,7 @@ DEPENDENCIES
   jquery-rails (~> 3.1.2)
   jquery-ui-rails (~> 5.0.3)
   kaminari (~> 0.16.3)
-  letter_opener (~> 1.3.0)
+  letter_opener (~> 1.4.1)
   letter_opener_web (~> 1.3.0)
   momentjs-rails (~> 2.10.6)
   mysql2 (~> 0.3.18)

app/mailers/notifications_mailer.rb

@@ -1,7 +1,7 @@
 class NotificationsMailer < ActionMailer::Base
   def new_message(message)
     @message = message
-    mail(to: AppConfig.get(:contact_link_location),
+    mail(to: AppConfig.contact_email,
          subject: "[#{AppConfig.get(:site_title)}] #{message.subject}",
          from: @message.email)
   end

app/models/app_config.rb

@@ -10,6 +10,10 @@ class AppConfig < ActiveRecord::Base
   validates :site_title,   presence: true,
                            length: { maximum: 20 }
   validates :admin_email,
+            presence: true,
+            format: { with: /\A([\w\.%\+\-]+)@([\w\-]+\.)+([\w]{2,})\z/i }
+  validates :contact_link_location,
+            allow_blank: true,
             format: { with: /\A([\w\.%\+\-]+)@([\w\-]+\.)+([\w]{2,})\z/i }
   validates :default_per_cat_page, numericality: { only_integer: true }
 
@@ -23,6 +27,17 @@ def self.check(prop, val = false)
 
   def self.get(prop, val = false)
     # alias for semantics
-    AppConfig.check(prop, val)
+    check(prop, val)
+  end
+
+  # Returns the to e-mail for contact form submissions, defaulting to the admin
+  # e-mail address if no separate address is specified
+  def self.contact_email
+    contact_email = get(:contact_link_location, nil)
+    if contact_email.nil? || contact_email.empty?
+      get(:admin_email, nil)
+    else
+      contact_email
+    end
   end
 end

db/seeds.rb

@@ -394,7 +394,7 @@ def generate_objs(method, obj, n)
   ac.notify_admin_on_create = false
   ac.admin_email = '[email protected]'
   ac.department_name = 'Department'
-  ac.contact_link_location = 'link'
+  ac.contact_link_location = '[email protected]'
   ac.home_link_text = 'home_link'
   ac.home_link_location = 'Canada'
   ac.deleted_missed_reservation_email_body = DELETED_MISSED_RES_EMAIL

lib/tasks/setup_application.rake

@@ -145,11 +145,7 @@ namespace :app do
         home_link_location = STDIN.gets.chomp
         puts 'Contact Email (this email address will receive contact form '\
           'submissions). Leave blank to default to the admin e-mail.'
-        if STDIN.gets.chomp.empty?
-          contact_link_location = admin_email
-        else
-          contact_link_location = STDIN.gets.chomp
-        end
+        contact_link_location = STDIN.gets.chomp unless STDIN.gets.chomp.empty?
 
         ActiveRecord::Base.transaction do
           begin

spec/controllers/contact_controller_spec.rb

@@ -46,6 +46,29 @@
       end
     end
   end
+  context 'with contact e-mail set' do
+    before do
+      AppConfig.first
+        .update_attributes(contact_link_location: '[email protected]')
+      post :create, message: FactoryGirl.attributes_for(:message)
+    end
+
+    it 'sends the message to the contact address' do
+      expect(ActionMailer::Base.deliveries.last.to).to\
+        include('[email protected]')
+    end
+  end
+  context 'with contact e-mail not set' do
+    before do
+      AppConfig.first.update_attributes(contact_link_location: '')
+      post :create, message: FactoryGirl.attributes_for(:message)
+    end
+
+    it 'sends the message to the admin address' do
+      expect(ActionMailer::Base.deliveries.last.to).to\
+        include(AppConfig.first.admin_email)
+    end
+  end
   after(:all) do
     @app_config.destroy
   end

spec/factories/app_configs.rb

@@ -8,7 +8,7 @@
     site_title 'Reservations Specs'
     admin_email '[email protected]'
     department_name 'MyString'
-    contact_link_location 'MyString'
+    contact_link_location '[email protected]'
     home_link_text 'MyString'
     home_link_location 'MyString'
     default_per_cat_page 1

spec/models/app_config_spec.rb

@@ -1,9 +1,9 @@
 require 'spec_helper'
 
 describe AppConfig, type: :model do
-  before(:each) do
-    @ac = FactoryGirl.build(:app_config)
-  end
+  before(:all) { AppConfig.delete_all }
+  before(:each) { @ac = FactoryGirl.build(:app_config) }
+
   it 'has a working factory' do
     expect(@ac.save).to be_truthy
   end
@@ -28,6 +28,13 @@
     @ac.admin_email = '[email protected]'
     expect(@ac).to be_valid
   end
+  it "shouldn't have an invalid contact e-mail" do
+    emails = ['ana@com', 'anda@pres,com']
+    emails.each do |invalid|
+      @ac.contact_link_location = invalid
+      expect(@ac).not_to be_valid
+    end
+  end
   # it "has an attachment that could serve as favicon" do
   #   @ac.favicon_file_name = "icon.ico"
   #   @ac.should be_valid
@@ -44,4 +51,24 @@
     end
     @ac.favicon_content_type = 'image/vnd.microsoft.icon'
   end
+
+  context '.contact_email' do
+    it 'returns the contact e-mail if it is set' do
+      @ac.contact_link_location = '[email protected]'
+      @ac.save
+
+      expect(AppConfig.contact_email).to eq('[email protected]')
+
+      AppConfig.delete_all
+    end
+
+    it 'returns the admin e-mail if no contact e-mail is set' do
+      @ac.contact_link_location = ''
+      @ac.save
+
+      expect(AppConfig.contact_email).to eq(AppConfig.check(:admin_email))
+
+      AppConfig.delete_all
+    end
+  end
 end