Commit 478f4cb5 authored by Sean Arnold's avatar Sean Arnold Committed by Nick Thomas

Use class method instead of scope

- Refactor specs a bit too
parent 2d2a6137
# frozen_string_literal: true
class SentryIssueFinder
attr_accessor :project, :current_user
def initialize(project, current_user: nil)
@project = project
@current_user = current_user
end
def execute(identifier)
return unless authorized?
SentryIssue
.for_project_and_identifier(project, identifier)
end
private
def authorized?
Ability.allowed?(current_user, :read_sentry_issue, project)
end
end
...@@ -4,7 +4,11 @@ class SentryIssue < ApplicationRecord ...@@ -4,7 +4,11 @@ class SentryIssue < ApplicationRecord
belongs_to :issue belongs_to :issue
validates :issue, uniqueness: true, presence: true validates :issue, uniqueness: true, presence: true
validates :sentry_issue_identifier, validates :sentry_issue_identifier, presence: true
uniqueness: true,
presence: true def self.for_project_and_identifier(project, identifier)
joins(:issue)
.where(issues: { project_id: project.id })
.find_by_sentry_issue_identifier(identifier)
end
end end
# frozen_string_literal: true
class AddIndexToSentryIssuesSentryIssueIdentifier < ActiveRecord::Migration[5.2]
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
disable_ddl_transaction!
def up
add_concurrent_index :sentry_issues, :sentry_issue_identifier
end
def down
remove_concurrent_index :sentry_issues, :sentry_issue_identifier
end
end
...@@ -10,7 +10,7 @@ ...@@ -10,7 +10,7 @@
# #
# It's strongly recommended that you check this file into your version control system. # It's strongly recommended that you check this file into your version control system.
ActiveRecord::Schema.define(version: 2020_01_14_113341) do ActiveRecord::Schema.define(version: 2020_01_14_204949) do
# These are extensions that must be enabled in order to support this database # These are extensions that must be enabled in order to support this database
enable_extension "pg_trgm" enable_extension "pg_trgm"
...@@ -3732,6 +3732,7 @@ ActiveRecord::Schema.define(version: 2020_01_14_113341) do ...@@ -3732,6 +3732,7 @@ ActiveRecord::Schema.define(version: 2020_01_14_113341) do
t.bigint "issue_id", null: false t.bigint "issue_id", null: false
t.bigint "sentry_issue_identifier", null: false t.bigint "sentry_issue_identifier", null: false
t.index ["issue_id"], name: "index_sentry_issues_on_issue_id", unique: true t.index ["issue_id"], name: "index_sentry_issues_on_issue_id", unique: true
t.index ["sentry_issue_identifier"], name: "index_sentry_issues_on_sentry_issue_identifier"
end end
create_table "serverless_domain_cluster", primary_key: "uuid", id: :string, limit: 14, force: :cascade do |t| create_table "serverless_domain_cluster", primary_key: "uuid", id: :string, limit: 14, force: :cascade do |t|
......
...@@ -3,6 +3,6 @@ ...@@ -3,6 +3,6 @@
FactoryBot.define do FactoryBot.define do
factory :sentry_issue, class: 'SentryIssue' do factory :sentry_issue, class: 'SentryIssue' do
issue issue
sentry_issue_identifier { 1234567891 } sequence(:sentry_issue_identifier) { |n| 10000000 + n }
end end
end end
# frozen_string_literal: true
require 'spec_helper'
describe SentryIssueFinder do
let(:user) { create(:user) }
let(:project) { create(:project, :repository) }
let(:issue) { create(:issue, project: project) }
let(:sentry_issue) { create(:sentry_issue, issue: issue) }
let(:finder) { described_class.new(project, current_user: user) }
describe '#execute' do
let(:identifier) { sentry_issue.sentry_issue_identifier }
subject { finder.execute(identifier) }
context 'when the user is not part of the project' do
it { is_expected.to be_nil }
end
context 'when the user is a project developer' do
before do
project.add_developer(user)
end
it { is_expected.to eq(sentry_issue) }
context 'when identifier is incorrect' do
let(:identifier) { 1234 }
it { is_expected.to be_nil }
end
context 'when accessing another projects identifier' do
let(:second_project) { create(:project) }
let(:second_issue) { create(:issue, project: second_project) }
let(:second_sentry_issue) { create(:sentry_issue, issue: second_issue) }
let(:identifier) { second_sentry_issue.sentry_issue_identifier }
it { is_expected.to be_nil }
end
end
end
end
...@@ -13,6 +13,16 @@ describe SentryIssue do ...@@ -13,6 +13,16 @@ describe SentryIssue do
it { is_expected.to validate_presence_of(:issue) } it { is_expected.to validate_presence_of(:issue) }
it { is_expected.to validate_uniqueness_of(:issue) } it { is_expected.to validate_uniqueness_of(:issue) }
it { is_expected.to validate_presence_of(:sentry_issue_identifier) } it { is_expected.to validate_presence_of(:sentry_issue_identifier) }
it { is_expected.to validate_uniqueness_of(:sentry_issue_identifier).with_message("has already been taken") } end
describe '.for_project_and_identifier' do
let!(:sentry_issue) { create(:sentry_issue) }
let(:project) { sentry_issue.issue.project }
let(:identifier) { sentry_issue.sentry_issue_identifier }
let!(:second_sentry_issue) { create(:sentry_issue) }
subject { described_class.for_project_and_identifier(project, identifier) }
it { is_expected.to eq(sentry_issue) }
end end
end end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment