Commit 20794440 authored by Reuben Pereira's avatar Reuben Pereira Committed by Sean McGivern

DB and model changes for Sentry project selection dropdown

parent 39abe2bb
......@@ -8,9 +8,13 @@ module ErrorTracking
belongs_to :project
validates :api_url, length: { maximum: 255 }, public_url: true, url: { enforce_sanitization: true }
validates :api_url, length: { maximum: 255 }, public_url: true, url: { enforce_sanitization: true, ascii_only: true }, allow_nil: true
validate :validate_api_url_path
validates :api_url, presence: true, if: :enabled
validate :validate_api_url_path, if: :enabled
validates :token, presence: true, if: :enabled
attr_encrypted :token,
mode: :per_attribute_iv,
......@@ -19,6 +23,31 @@ module ErrorTracking
after_save :clear_reactive_cache!
def project_name
super || project_name_from_slug
end
def organization_name
super || organization_name_from_slug
end
def project_slug
project_slug_from_api_url
end
def organization_slug
organization_slug_from_api_url
end
def self.build_api_url_from(api_host:, project_slug:, organization_slug:)
uri = Addressable::URI.parse("#{api_host}/api/0/projects/#{organization_slug}/#{project_slug}/")
uri.path = uri.path.squeeze('/')
uri.to_s
rescue Addressable::URI::InvalidURIError
api_host
end
def sentry_client
Sentry::Client.new(api_url, token)
end
......@@ -33,6 +62,10 @@ module ErrorTracking
end
end
def list_sentry_projects
{ projects: sentry_client.list_projects }
end
def calculate_reactive_cache(request, opts)
case request
when 'list_issues'
......@@ -47,13 +80,53 @@ module ErrorTracking
url.sub('api/0/projects/', '')
end
def api_host
return if api_url.blank?
# This returns http://example.com/
Addressable::URI.join(api_url, '/').to_s
end
private
def project_name_from_slug
@project_name_from_slug ||= project_slug_from_api_url&.titleize
end
def organization_name_from_slug
@organization_name_from_slug ||= organization_slug_from_api_url&.titleize
end
def project_slug_from_api_url
extract_slug(:project)
end
def organization_slug_from_api_url
extract_slug(:organization)
end
def extract_slug(capture)
return if api_url.blank?
begin
url = Addressable::URI.parse(api_url)
rescue Addressable::URI::InvalidURIError
return nil
end
@slug_match ||= url.path.match(%r{^/api/0/projects/+(?<organization>[^/]+)/+(?<project>[^/|$]+)}) || {}
@slug_match[capture]
end
def validate_api_url_path
unless URI(api_url).path.starts_with?('/api/0/projects')
errors.add(:api_url, 'path needs to start with /api/0/projects')
return if api_url.blank?
begin
unless Addressable::URI.parse(api_url).path.starts_with?('/api/0/projects')
errors.add(:api_url, 'path needs to start with /api/0/projects')
end
rescue Addressable::URI::InvalidURIError
end
rescue URI::InvalidURIError
end
end
end
# frozen_string_literal: true
class AddColumnsProjectErrorTrackingSettings < ActiveRecord::Migration[5.0]
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
def change
add_column :project_error_tracking_settings, :project_name, :string
add_column :project_error_tracking_settings, :organization_name, :string
change_column_default :project_error_tracking_settings, :enabled, from: true, to: false
change_column_null :project_error_tracking_settings, :api_url, true
end
end
......@@ -1574,10 +1574,12 @@ ActiveRecord::Schema.define(version: 20190131122559) do
end
create_table "project_error_tracking_settings", primary_key: "project_id", id: :integer, force: :cascade do |t|
t.boolean "enabled", default: true, null: false
t.string "api_url", null: false
t.boolean "enabled", default: false, null: false
t.string "api_url"
t.string "encrypted_token"
t.string "encrypted_token_iv"
t.string "project_name"
t.string "organization_name"
end
create_table "project_features", force: :cascade do |t|
......
......@@ -166,6 +166,7 @@ excluded_attributes:
error_tracking_setting:
- :encrypted_token
- :encrypted_token_iv
- :enabled
methods:
labels:
......
......@@ -6,5 +6,7 @@ FactoryBot.define do
api_url 'https://gitlab.com/api/0/projects/sentry-org/sentry-project'
enabled true
token 'access_token_123'
project_name 'Sentry Project'
organization_name 'Sentry Org'
end
end
......@@ -603,5 +603,6 @@ ResourceLabelEvent:
ErrorTracking::ProjectErrorTrackingSetting:
- id
- api_url
- enabled
- project_id
- project_name
- organization_name
......@@ -15,9 +15,11 @@ describe ErrorTracking::ProjectErrorTrackingSetting do
describe 'Validations' do
context 'when api_url is over 255 chars' do
it 'fails validation' do
before do
subject.api_url = 'https://' + 'a' * 250
end
it 'fails validation' do
expect(subject).not_to be_valid
expect(subject.errors.messages[:api_url]).to include('is too long (maximum is 255 characters)')
end
......@@ -31,6 +33,34 @@ describe ErrorTracking::ProjectErrorTrackingSetting do
end
end
context 'presence validations' do
using RSpec::Parameterized::TableSyntax
valid_api_url = 'http://example.com/api/0/projects/org-slug/proj-slug/'
valid_token = 'token'
where(:enabled, :token, :api_url, :valid?) do
true | nil | nil | false
true | nil | valid_api_url | false
true | valid_token | nil | false
true | valid_token | valid_api_url | true
false | nil | nil | true
false | nil | valid_api_url | true
false | valid_token | nil | true
false | valid_token | valid_api_url | true
end
with_them do
before do
subject.enabled = enabled
subject.token = token
subject.api_url = api_url
end
it { expect(subject.valid?).to eq(valid?) }
end
end
context 'URL path' do
it 'fails validation with wrong path' do
subject.api_url = 'http://gitlab.com/project1/something'
......@@ -45,6 +75,16 @@ describe ErrorTracking::ProjectErrorTrackingSetting do
expect(subject).to be_valid
end
end
context 'non ascii chars in api_url' do
before do
subject.api_url = 'http://gitlab.com/api/0/projects/project1/something€'
end
it 'fails validation' do
expect(subject).not_to be_valid
end
end
end
describe '#sentry_external_url' do
......@@ -106,4 +146,138 @@ describe ErrorTracking::ProjectErrorTrackingSetting do
end
end
end
describe '#list_sentry_projects' do
let(:projects) { [:list, :of, :projects] }
let(:sentry_client) { spy(:sentry_client) }
it 'calls sentry client' do
expect(subject).to receive(:sentry_client).and_return(sentry_client)
expect(sentry_client).to receive(:list_projects).and_return(projects)
result = subject.list_sentry_projects
expect(result).to eq(projects: projects)
end
end
context 'slugs' do
shared_examples_for 'slug from api_url' do |method, slug|
context 'when api_url is correct' do
before do
subject.api_url = 'http://gitlab.com/api/0/projects/org-slug/project-slug/'
end
it 'returns slug' do
expect(subject.public_send(method)).to eq(slug)
end
end
context 'when api_url is blank' do
before do
subject.api_url = nil
end
it 'returns nil' do
expect(subject.public_send(method)).to be_nil
end
end
end
it_behaves_like 'slug from api_url', :project_slug, 'project-slug'
it_behaves_like 'slug from api_url', :organization_slug, 'org-slug'
end
context 'names from api_url' do
shared_examples_for 'name from api_url' do |name, titleized_slug|
context 'name is present in DB' do
it 'returns name from DB' do
subject[name] = 'Sentry name'
subject.api_url = 'http://gitlab.com/api/0/projects/org-slug/project-slug'
expect(subject.public_send(name)).to eq('Sentry name')
end
end
context 'name is null in DB' do
it 'titleizes and returns slug from api_url' do
subject[name] = nil
subject.api_url = 'http://gitlab.com/api/0/projects/org-slug/project-slug'
expect(subject.public_send(name)).to eq(titleized_slug)
end
it 'returns nil when api_url is incorrect' do
subject[name] = nil
subject.api_url = 'http://gitlab.com/api/0/projects/'
expect(subject.public_send(name)).to be_nil
end
it 'returns nil when api_url is blank' do
subject[name] = nil
subject.api_url = nil
expect(subject.public_send(name)).to be_nil
end
end
end
it_behaves_like 'name from api_url', :organization_name, 'Org Slug'
it_behaves_like 'name from api_url', :project_name, 'Project Slug'
end
describe '.build_api_url_from' do
it 'correctly builds api_url with slugs' do
api_url = described_class.build_api_url_from(
api_host: 'http://sentry.com/',
organization_slug: 'org-slug',
project_slug: 'proj-slug'
)
expect(api_url).to eq('http://sentry.com/api/0/projects/org-slug/proj-slug/')
end
it 'correctly builds api_url without slugs' do
api_url = described_class.build_api_url_from(
api_host: 'http://sentry.com/',
organization_slug: nil,
project_slug: nil
)
expect(api_url).to eq('http://sentry.com/api/0/projects/')
end
it 'does not raise exception with invalid url' do
api_url = described_class.build_api_url_from(
api_host: ':::',
organization_slug: 'org-slug',
project_slug: 'proj-slug'
)
expect(api_url).to eq(':::')
end
end
describe '#api_host' do
context 'when api_url exists' do
before do
subject.api_url = 'https://example.com/api/0/projects/org-slug/proj-slug/'
end
it 'extracts the api_host from api_url' do
expect(subject.api_host).to eq('https://example.com/')
end
end
context 'when api_url is nil' do
before do
subject.api_url = nil
end
it 'returns nil' do
expect(subject.api_url).to eq(nil)
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