From 95419679f23f0628d1885dd9656cc159e9d55ea9 Mon Sep 17 00:00:00 2001 From: Patricio Cano <suprnova32@gmail.com> Date: Wed, 27 Jul 2016 19:03:06 -0500 Subject: [PATCH] Lay the ground works to submit information to Akismet - New concern `AkismetSubmittable` to allow issues and other `Spammable` models to be submitted to Akismet. - New model `UserAgentDetail` to store information needed for Akismet. - Services needed for their creation and tests. --- app/models/concerns/akismet_submittable.rb | 15 +++++++++++++ app/models/issue.rb | 1 + app/models/user_agent_detail.rb | 16 ++++++++++++++ app/services/issues/create_service.rb | 5 +++++ app/services/user_agent_detail_service.rb | 12 ++++++++++ ...0160727163552_create_user_agent_details.rb | 12 ++++++++++ db/schema.rb | 9 ++++++++ .../projects/issues_controller_spec.rb | 20 +++++++++++++++++ spec/factories/user_agent_details.rb | 10 +++++++++ spec/models/user_agent_detail_spec.rb | 22 +++++++++++++++++++ 10 files changed, 122 insertions(+) create mode 100644 app/models/concerns/akismet_submittable.rb create mode 100644 app/models/user_agent_detail.rb create mode 100644 app/services/user_agent_detail_service.rb create mode 100644 db/migrate/20160727163552_create_user_agent_details.rb create mode 100644 spec/factories/user_agent_details.rb create mode 100644 spec/models/user_agent_detail_spec.rb diff --git a/app/models/concerns/akismet_submittable.rb b/app/models/concerns/akismet_submittable.rb new file mode 100644 index 00000000000..17821688941 --- /dev/null +++ b/app/models/concerns/akismet_submittable.rb @@ -0,0 +1,15 @@ +module AkismetSubmittable + extend ActiveSupport::Concern + + included do + has_one :user_agent_detail, as: :subject + end + + def can_be_submitted? + if user_agent_detail + user_agent_detail.submittable? + else + false + end + end +end diff --git a/app/models/issue.rb b/app/models/issue.rb index d62ffb21467..6c2635498e5 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -8,6 +8,7 @@ class Issue < ActiveRecord::Base include Taskable include Spammable include FasterCacheKeys + include AkismetSubmittable DueDateStruct = Struct.new(:title, :name).freeze NoDueDate = DueDateStruct.new('No Due Date', '0').freeze diff --git a/app/models/user_agent_detail.rb b/app/models/user_agent_detail.rb new file mode 100644 index 00000000000..6d76dff20e3 --- /dev/null +++ b/app/models/user_agent_detail.rb @@ -0,0 +1,16 @@ +class UserAgentDetail < ActiveRecord::Base + belongs_to :subject, polymorphic: true + + validates :user_agent, + presence: true + validates :ip_address, + presence: true + validates :subject_id, + presence: true + validates :subject_type, + presence: true + + def submittable? + user_agent.present? && ip_address.present? + end +end diff --git a/app/services/issues/create_service.rb b/app/services/issues/create_service.rb index 5e2de2ccf64..8e9d74103c7 100644 --- a/app/services/issues/create_service.rb +++ b/app/services/issues/create_service.rb @@ -15,6 +15,7 @@ module Issues notification_service.new_issue(issue, current_user) todo_service.new_issue(issue, current_user) event_service.open_issue(issue, current_user) + user_agent_detail_service(issue, request).create issue.create_cross_references!(current_user) execute_hooks(issue, 'open') end @@ -27,5 +28,9 @@ module Issues def spam_check_service SpamCheckService.new(project, current_user, params) end + + def user_agent_detail_service(issue, request) + UserAgentDetailService.new(issue, request) + end end end diff --git a/app/services/user_agent_detail_service.rb b/app/services/user_agent_detail_service.rb new file mode 100644 index 00000000000..dd995955be3 --- /dev/null +++ b/app/services/user_agent_detail_service.rb @@ -0,0 +1,12 @@ +class UserAgentDetailService + attr_accessor :subject, :request + + def initialize(subject, request) + @subject, @request = subject, request + end + + def create + return unless request + subject.create_user_agent_detail(user_agent: request.env['HTTP_USER_AGENT'], ip_address: request.env['action_dispatch.remote_ip'].to_s) + end +end diff --git a/db/migrate/20160727163552_create_user_agent_details.rb b/db/migrate/20160727163552_create_user_agent_details.rb new file mode 100644 index 00000000000..05c21a476fa --- /dev/null +++ b/db/migrate/20160727163552_create_user_agent_details.rb @@ -0,0 +1,12 @@ +class CreateUserAgentDetails < ActiveRecord::Migration + def change + create_table :user_agent_details do |t| + t.string :user_agent, null: false + t.string :ip_address, null: false + t.integer :subject_id, null: false + t.string :subject_type, null: false + + t.timestamps null: false + end + end +end diff --git a/db/schema.rb b/db/schema.rb index f008a6bd7a7..2e5ffac5935 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -999,6 +999,15 @@ ActiveRecord::Schema.define(version: 20160810142633) do add_index "u2f_registrations", ["key_handle"], name: "index_u2f_registrations_on_key_handle", using: :btree add_index "u2f_registrations", ["user_id"], name: "index_u2f_registrations_on_user_id", using: :btree + create_table "user_agent_details", force: :cascade do |t| + t.string "user_agent", null: false + t.string "ip_address", null: false + t.integer "subject_id", null: false + t.string "subject_type", null: false + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + end + create_table "users", force: :cascade do |t| t.string "email", default: "", null: false t.string "encrypted_password", default: "", null: false diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb index b6a0276846c..4e39826d694 100644 --- a/spec/controllers/projects/issues_controller_spec.rb +++ b/spec/controllers/projects/issues_controller_spec.rb @@ -300,6 +300,26 @@ describe Projects::IssuesController do expect(spam_logs[0].title).to eq('Spam Title') end end + + context 'user agent details are saved' do + before do + request.env['action_dispatch.remote_ip'] = '127.0.0.1' + end + + def post_new_issue + sign_in(user) + project = create(:empty_project, :public) + post :create, { + namespace_id: project.namespace.to_param, + project_id: project.to_param, + issue: { title: 'Title', description: 'Description' } + } + end + + it 'creates a user agent detail' do + expect{ post_new_issue }.to change(UserAgentDetail, :count) + end + end end describe "DELETE #destroy" do diff --git a/spec/factories/user_agent_details.rb b/spec/factories/user_agent_details.rb new file mode 100644 index 00000000000..5fc40915911 --- /dev/null +++ b/spec/factories/user_agent_details.rb @@ -0,0 +1,10 @@ +FactoryGirl.define do + factory :user_agent_detail do + ip_address '127.0.0.1' + user_agent 'AppleWebKit/537.36' + + trait :on_issue do + association :subject, factory: :issue + end + end +end diff --git a/spec/models/user_agent_detail_spec.rb b/spec/models/user_agent_detail_spec.rb new file mode 100644 index 00000000000..8debcbbeba6 --- /dev/null +++ b/spec/models/user_agent_detail_spec.rb @@ -0,0 +1,22 @@ +require 'rails_helper' + +describe UserAgentDetail, type: :model do + describe '.submittable?' do + it 'should be submittable' do + detail = create(:user_agent_detail, :on_issue) + expect(detail.submittable?).to be_truthy + end + end + + describe '.valid?' do + it 'should be valid with a subject' do + detail = create(:user_agent_detail, :on_issue) + expect(detail).to be_valid + end + + it 'should not be valid without a subject' do + detail = build(:user_agent_detail) + expect(detail).not_to be_valid + end + end +end -- GitLab