Merge branch '39825-sentry-issue-finder' into 'master'

Add Sentry Issue Finder

See merge request gitlab-org/gitlab!22727

app/finders/sentry_issue_finder.rb

+# 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

app/models/sentry_issue.rb

@@ -4,7 +4,11 @@ class SentryIssue < ApplicationRecord
   belongs_to :issue
 
   validates :issue, uniqueness: true, presence: true
-  validates :sentry_issue_identifier,
-    uniqueness: true,
-    presence: true
+  validates :sentry_issue_identifier, 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

db/migrate/20200114204949_add_index_to_sentry_issues_sentry_issue_identifier.rb

+# 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

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # 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
   enable_extension "pg_trgm"
@@ -3732,6 +3732,7 @@ ActiveRecord::Schema.define(version: 2020_01_14_113341) do
     t.bigint "issue_id", null: false
     t.bigint "sentry_issue_identifier", null: false
     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
 
   create_table "serverless_domain_cluster", primary_key: "uuid", id: :string, limit: 14, force: :cascade do |t|

spec/factories/sentry_issue.rb

@@ -3,6 +3,6 @@
 FactoryBot.define do
   factory :sentry_issue, class: 'SentryIssue' do
     issue
-    sentry_issue_identifier { 1234567891 }
+    sequence(:sentry_issue_identifier) { |n| 10000000 + n }
   end
 end

spec/finders/sentry_issue_finder_spec.rb

+# 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

spec/models/sentry_issue_spec.rb

@@ -13,6 +13,16 @@ describe SentryIssue do
     it { is_expected.to validate_presence_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_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