Remove dismissed_state from Callout model

8 files changed, 20 insertions, 25 deletions

diff --git a/app/controllers/callouts_controller.rb b/app/controllers/callouts_controller.rb
index a7bccfcce78..e24172edf5d 100644
--- a/app/controllers/callouts_controller.rb
+++ b/app/controllers/callouts_controller.rb
@@ -1,24 +1,16 @@
 class CalloutsController < ApplicationController
-  before_action :callout, only: [:dismiss]
-
   def dismiss
-    respond_to do |format|
-      format.json do
-        if @callout
-          @callout.update(dismissed_state: true)
-        else
-          Callout.create(feature_name: callout_param, dismissed_state: true, user: current_user)
-        end
-
-        head :ok
-      end
+    if ensure_callout
+      respond_to { |format| format.json { head :ok } }
+    else
+      respond_to { |format| format.json { head :bad_request } }
     end
   end
 
   private
 
-  def callout
-    @callout = Callout.find_by(user: current_user, feature_name: callout_param)
+  def ensure_callout
+    current_user.callouts.find_or_create_by(feature_name: callout_param)
   end
 
   def callout_param
diff --git a/app/helpers/callouts_helper.rb b/app/helpers/callouts_helper.rb
index f93df287512..3841d288f52 100644
--- a/app/helpers/callouts_helper.rb
+++ b/app/helpers/callouts_helper.rb
@@ -15,6 +15,6 @@ module CalloutsHelper
   private
 
   def user_dismissed?(feature_name)
-    Callout.find_by(user: current_user, feature_name: feature_name)&.dismissed_state?
+    current_user&.callouts&.find_by(feature_name: feature_name)
   end
 end
diff --git a/app/models/callout.rb b/app/models/callout.rb
index b8131beb518..c8f41a39cf3 100644
--- a/app/models/callout.rb
+++ b/app/models/callout.rb
@@ -1,3 +1,6 @@
 class Callout < ActiveRecord::Base
   belongs_to :user
+
+  validates :user, presence: true
+  validates :feature_name, presence: true, uniqueness: { scope: :user_id }
 end
diff --git a/db/migrate/20180125214301_create_callouts.rb b/db/migrate/20180125214301_create_callouts.rb
index 5dc9638a845..b6749b4cfe6 100644
--- a/db/migrate/20180125214301_create_callouts.rb
+++ b/db/migrate/20180125214301_create_callouts.rb
@@ -8,12 +8,11 @@ class CreateCallouts < ActiveRecord::Migration
   def change
     create_table :callouts do |t|
       t.string :feature_name, null: false
-      t.boolean :dismissed_state, null: false
       t.references :user, index: true, foreign_key: { on_delete: :cascade }, null: false
 
       t.timestamps_with_timezone null: false
     end
 
-    add_index :callouts, :feature_name, unique: true
+    add_index :callouts, [:user_id, :feature_name], unique: true
   end
 end
diff --git a/db/schema.rb b/db/schema.rb
index fc12e5752b3..1b837db4e8d 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -205,13 +205,12 @@ ActiveRecord::Schema.define(version: 20180125214301) do
 
   create_table "callouts", force: :cascade do |t|
     t.string "feature_name", null: false
-    t.boolean "dismissed_state", null: false
     t.integer "user_id", null: false
     t.datetime_with_timezone "created_at", null: false
     t.datetime_with_timezone "updated_at", null: false
   end
 
-  add_index "callouts", ["feature_name"], name: "index_callouts_on_feature_name", unique: true, using: :btree
+  add_index "callouts", ["user_id", "feature_name"], name: "index_callouts_on_user_id_and_feature_name", unique: true, using: :btree
   add_index "callouts", ["user_id"], name: "index_callouts_on_user_id", using: :btree
 
   create_table "chat_names", force: :cascade do |t|
diff --git a/spec/controllers/callouts_controller_spec.rb b/spec/controllers/callouts_controller_spec.rb
index bf2aae190f2..7a183d757a1 100644
--- a/spec/controllers/callouts_controller_spec.rb
+++ b/spec/controllers/callouts_controller_spec.rb
@@ -25,10 +25,6 @@ describe CalloutsController do
     context 'when callout entry already exists' do
       let!(:callout) { create(:callout, feature_name: 'feature_name', user: user) }
 
-      it 'should update it with a dismissed state' do
-        expect { subject }.to change { callout.reload.dismissed_state }.from(false).to(true)
-      end
-
       it 'should return success' do
         subject
 
diff --git a/spec/factories/callouts.rb b/spec/factories/callouts.rb
index b8ea879933e..f28df49b00a 100644
--- a/spec/factories/callouts.rb
+++ b/spec/factories/callouts.rb
@@ -1,7 +1,6 @@
 FactoryBot.define do
   factory :callout do
     feature_name 'test_callout'
-    dismissed_state false
 
     user
   end
diff --git a/spec/models/callout_spec.rb b/spec/models/callout_spec.rb
index 8328ce06139..29299678ebc 100644
--- a/spec/models/callout_spec.rb
+++ b/spec/models/callout_spec.rb
@@ -1,9 +1,16 @@
 require 'rails_helper'
 
 describe Callout do
-  let(:callout) { create(:callout) }
+  let!(:callout) { create(:callout) }
 
   describe 'relationships' do
     it { is_expected.to belong_to(:user) }
   end
+
+  describe 'validations' do
+    it { is_expected.to validate_presence_of(:user) }
+
+    it { is_expected.to validate_presence_of(:feature_name) }
+    it { is_expected.to validate_uniqueness_of(:feature_name).scoped_to(:user_id) }
+  end
 end