diff options
-rw-r--r-- | app/controllers/user_callouts_controller.rb | 6 | ||||
-rw-r--r-- | app/helpers/user_callouts_helper.rb | 5 | ||||
-rw-r--r-- | app/models/user_callout.rb | 9 | ||||
-rw-r--r-- | db/migrate/20180125214301_create_user_callouts.rb | 2 | ||||
-rw-r--r-- | db/schema.rb | 2 | ||||
-rw-r--r-- | spec/controllers/user_callouts_controller_spec.rb | 36 | ||||
-rw-r--r-- | spec/factories/user_callouts.rb | 2 | ||||
-rw-r--r-- | spec/models/user_callout_spec.rb | 2 |
8 files changed, 42 insertions, 22 deletions
diff --git a/app/controllers/user_callouts_controller.rb b/app/controllers/user_callouts_controller.rb index 69dc444f13a..aa048a9a1fc 100644 --- a/app/controllers/user_callouts_controller.rb +++ b/app/controllers/user_callouts_controller.rb @@ -1,6 +1,6 @@ class UserCalloutsController < ApplicationController def create - if ensure_callout + if check_feature_name && ensure_callout respond_to do |format| format.json { head :ok } end @@ -13,6 +13,10 @@ class UserCalloutsController < ApplicationController private + def check_feature_name + UserCallout.feature_names.keys.include?(callout_param) + end + def ensure_callout current_user.callouts.find_or_create_by(feature_name: callout_param) end diff --git a/app/helpers/user_callouts_helper.rb b/app/helpers/user_callouts_helper.rb index 3725545d4cc..a292f676331 100644 --- a/app/helpers/user_callouts_helper.rb +++ b/app/helpers/user_callouts_helper.rb @@ -1,11 +1,6 @@ module UserCalloutsHelper GKE_CLUSTER_INTEGRATION = 'gke_cluster_integration'.freeze - # Higher value = higher priority - PRIORITY = { - GKE_CLUSTER_INTEGRATION: 0 - }.freeze - def show_gke_cluster_integration_callout?(project) current_user && !user_dismissed?(GKE_CLUSTER_INTEGRATION) && can?(current_user, :create_cluster, project) diff --git a/app/models/user_callout.rb b/app/models/user_callout.rb index 8b31aa6fa5c..a7cfe5df7c0 100644 --- a/app/models/user_callout.rb +++ b/app/models/user_callout.rb @@ -1,6 +1,13 @@ class UserCallout < ActiveRecord::Base belongs_to :user + enum feature_name: { + gke_cluster_integration: 0 + } + validates :user, presence: true - validates :feature_name, presence: true, uniqueness: { scope: :user_id } + validates :feature_name, + presence: true, + uniqueness: { scope: :user_id }, + inclusion: { in: UserCallout.feature_names.keys } end diff --git a/db/migrate/20180125214301_create_user_callouts.rb b/db/migrate/20180125214301_create_user_callouts.rb index 945f4181274..856eff36ae0 100644 --- a/db/migrate/20180125214301_create_user_callouts.rb +++ b/db/migrate/20180125214301_create_user_callouts.rb @@ -7,7 +7,7 @@ class CreateUserCallouts < ActiveRecord::Migration def change create_table :user_callouts do |t| - t.string :feature_name, null: false + t.integer :feature_name, null: false t.references :user, index: true, foreign_key: { on_delete: :cascade }, null: false end diff --git a/db/schema.rb b/db/schema.rb index 6e50b7681d0..14024164359 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -1770,7 +1770,7 @@ ActiveRecord::Schema.define(version: 20180202111106) do add_index "user_agent_details", ["subject_id", "subject_type"], name: "index_user_agent_details_on_subject_id_and_subject_type", using: :btree create_table "user_callouts", force: :cascade do |t| - t.string "feature_name", null: false + t.integer "feature_name", null: false t.integer "user_id", null: false end diff --git a/spec/controllers/user_callouts_controller_spec.rb b/spec/controllers/user_callouts_controller_spec.rb index ac295aa72bb..48e2ff75cac 100644 --- a/spec/controllers/user_callouts_controller_spec.rb +++ b/spec/controllers/user_callouts_controller_spec.rb @@ -8,27 +8,41 @@ describe UserCalloutsController do end describe "POST #create" do - subject { post :create, feature_name: 'feature_name', format: :json } + subject { post :create, feature_name: feature_name, format: :json } - context 'when callout entry does not exist' do - it 'should create a callout entry with dismissed state' do - expect { subject }.to change { UserCallout.count }.by(1) + context 'with valid feature name' do + let(:feature_name) { UserCallout.feature_names.keys.first } + + context 'when callout entry does not exist' do + it 'should create a callout entry with dismissed state' do + expect { subject }.to change { UserCallout.count }.by(1) + end + + it 'should return success' do + subject + + expect(response).to have_gitlab_http_status(:ok) + end end - it 'should return success' do - subject + context 'when callout entry already exists' do + let!(:callout) { create(:user_callout, feature_name: UserCallout.feature_names.keys.first, user: user) } + + it 'should return success' do + subject - expect(response).to have_gitlab_http_status(:ok) + expect(response).to have_gitlab_http_status(:ok) + end end end - context 'when callout entry already exists' do - let!(:callout) { create(:user_callout, feature_name: 'feature_name', user: user) } + context 'with invalid feature name' do + let(:feature_name) { 'bogus_feature_name' } - it 'should return success' do + it 'should return bad request' do subject - expect(response).to have_gitlab_http_status(:ok) + expect(response).to have_gitlab_http_status(:bad_request) end end end diff --git a/spec/factories/user_callouts.rb b/spec/factories/user_callouts.rb index ae83d65da2e..528e442c14b 100644 --- a/spec/factories/user_callouts.rb +++ b/spec/factories/user_callouts.rb @@ -1,6 +1,6 @@ FactoryBot.define do factory :user_callout do - feature_name 'test_callout' + feature_name :gke_cluster_integration user end diff --git a/spec/models/user_callout_spec.rb b/spec/models/user_callout_spec.rb index b39d12e48ac..64ba17c81fe 100644 --- a/spec/models/user_callout_spec.rb +++ b/spec/models/user_callout_spec.rb @@ -11,6 +11,6 @@ describe UserCallout 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) } + it { is_expected.to validate_uniqueness_of(:feature_name).scoped_to(:user_id).ignoring_case_sensitivity } end end |