Automatic merge of gitlab-org/gitlab-ce master

app/serializers/error_tracking/project_entity.rb

+# frozen_string_literal: true
+
+module ErrorTracking
+  class ProjectEntity < Grape::Entity
+    expose(*Gitlab::ErrorTracking::Project::ACCESSORS)
+  end
+end

app/serializers/error_tracking/project_serializer.rb

+# frozen_string_literal: true
+
+module ErrorTracking
+  class ProjectSerializer < BaseSerializer
+    entity ProjectEntity
+  end
+end

app/views/admin/runners/_runner.html.haml

@@ -18,12 +18,12 @@
     .table-mobile-content
       = link_to runner.short_sha, admin_runner_path(runner)
 
-  .table-section.section-15
+  .table-section.section-20
     .table-mobile-header{ role: 'rowheader' }= _('Description')
     .table-mobile-content.str-truncated.has-tooltip{ title: runner.description }
       = runner.description
 
-  .table-section.section-15
+  .table-section.section-10
     .table-mobile-header{ role: 'rowheader' }= _('Version')
     .table-mobile-content.str-truncated.has-tooltip{ title: runner.version }
       = runner.version

app/views/admin/runners/index.html.haml

@@ -106,8 +106,8 @@
         .gl-responsive-table-row.table-row-header{ role: 'row' }
           .table-section.section-10{ role: 'rowheader' }= _('Type')
           .table-section.section-10{ role: 'rowheader' }= _('Runner token')
-          .table-section.section-15{ role: 'rowheader' }= _('Description')
-          .table-section.section-15{ role: 'rowheader' }= _('Version')
+          .table-section.section-20{ role: 'rowheader' }= _('Description')
+          .table-section.section-10{ role: 'rowheader' }= _('Version')
           .table-section.section-10{ role: 'rowheader' }= _('IP Address')
           .table-section.section-5{ role: 'rowheader' }= _('Projects')
           .table-section.section-5{ role: 'rowheader' }= _('Jobs')

changelogs/unreleased/hnk-master-patch-61932.yml

+---
+title: Update runner admin page to make description field larger
+merge_request: 23593
+author: Sascha Reynolds
+type: fixed

lib/gitlab/error_tracking/project.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module ErrorTracking
+    class Project
+      include ActiveModel::Model
+
+      ACCESSORS = [
+        :id, :name, :status, :slug, :organization_name,
+        :organization_id, :organization_slug
+      ].freeze
+
+      attr_accessor(*ACCESSORS)
+    end
+  end
+end

lib/sentry/client.rb

@@ -3,6 +3,7 @@
 module Sentry
   class Client
     Error = Class.new(StandardError)
+    SentryError = Class.new(StandardError)
 
     attr_accessor :url, :token
 
@@ -16,6 +17,13 @@ module Sentry
       map_to_errors(issues)
     end
 
+    def list_projects
+      projects = get_projects
+      map_to_projects(projects)
+    rescue KeyError => e
+      raise Client::SentryError, "Sentry API response is missing keys. #{e.message}"
+    end
+
     private
 
     def request_params
@@ -27,18 +35,23 @@ module Sentry
       }
     end
 
-    def get_issues(issue_status:, limit:)
-      resp = Gitlab::HTTP.get(
-        issues_api_url,
-        **request_params.merge(query: {
-          query: "is:#{issue_status}",
-          limit: limit
-        })
-      )
+    def http_get(url, params = {})
+      resp = Gitlab::HTTP.get(url, **request_params.merge(params))
 
       handle_response(resp)
     end
 
+    def get_issues(issue_status:, limit:)
+      http_get(issues_api_url, query: {
+        query: "is:#{issue_status}",
+        limit: limit
+      })
+    end
+
+    def get_projects
+      http_get(projects_api_url)
+    end
+
     def handle_response(response)
       unless response.code == 200
         raise Client::Error, "Sentry response error: #{response.code}"
@@ -47,6 +60,13 @@ module Sentry
       response.as_json
     end
 
+    def projects_api_url
+      projects_url = URI(@url)
+      projects_url.path = '/api/0/projects/'
+
+      projects_url
+    end
+
     def issues_api_url
       issues_url = URI(@url + '/issues/')
       issues_url.path.squeeze!('/')
@@ -55,9 +75,11 @@ module Sentry
     end
 
     def map_to_errors(issues)
-      issues.map do |issue|
-        map_to_error(issue)
-      end
+      issues.map(&method(:map_to_error))
+    end
+
+    def map_to_projects(projects)
+      projects.map(&method(:map_to_project))
     end
 
     def issue_url(id)
@@ -100,5 +122,19 @@ module Sentry
         project_slug: project.fetch('slug', nil)
       )
     end
+
+    def map_to_project(project)
+      organization = project.fetch('organization')
+
+      Gitlab::ErrorTracking::Project.new(
+        id: project.fetch('id'),
+        name: project.fetch('name'),
+        slug: project.fetch('slug'),
+        status: project.dig('status'),
+        organization_name: organization.fetch('name'),
+        organization_id: organization.fetch('id'),
+        organization_slug: organization.fetch('slug')
+      )
+    end
   end
 end

spec/factories/error_tracking/project.rb

+# frozen_string_literal: true
+
+FactoryBot.define do
+  factory :error_tracking_project, class: Gitlab::ErrorTracking::Project do
+    id '1'
+    name 'Sentry Example'
+    slug 'sentry-example'
+    status 'active'
+    organization_name 'Sentry'
+    organization_id '1'
+    organization_slug 'sentry'
+
+    skip_create
+  end
+end

spec/fixtures/api/schemas/error_tracking/list_projects.json

+{
+  "type": "object",
+  "required": [
+    "projects"
+  ],
+  "properties": {
+    "projects": {
+      "type": "array",
+      "items": { "$ref": "project.json" }
+    }
+  },
+  "additionalProperties": false
+}

spec/fixtures/api/schemas/error_tracking/project.json

+{
+  "type": "object",
+  "required" : [
+    "id",
+    "slug",
+    "organization_slug",
+    "name"
+  ],
+  "properties" : {
+    "id": { "type": "string"},
+    "name": { "type": "string" },
+    "slug": { "type": "string" },
+    "status": { "type": "string" },
+    "organization_name": { "type": "string" },
+    "organization_slug": { "type": "string" },
+    "organization_id": { "type": "string" }
+  },
+  "additionalProperties": false
+}

spec/fixtures/sentry/list_projects_sample_response.json

+[
+  {
+    "status": "active",
+    "features": [
+      "data-forwarding",
+      "rate-limits",
+      "releases"
+    ],
+    "color": "#5c3fbf",
+    "isInternal": false,
+    "isPublic": false,
+    "dateCreated": "2018-12-11T10:41:22.476Z",
+    "id": "2",
+    "slug": "sentry-example",
+    "name": "sentry-example",
+    "hasAccess": true,
+    "isBookmarked": false,
+    "platform": "node",
+    "firstEvent": "2018-12-12T15:07:18Z",
+    "avatar": {
+      "avatarUuid": null,
+      "avatarType": "letter_avatar"
+    },
+    "isMember": true,
+    "organization": {
+      "status": {
+        "id": "active",
+        "name": "active"
+      },
+      "require2FA": false,
+      "avatar": {
+        "avatarUuid": null,
+        "avatarType": "letter_avatar"
+      },
+      "name": "Sentry",
+      "dateCreated": "2018-12-11T10:21:47.431Z",
+      "id": "1",
+      "isEarlyAdopter": false,
+      "slug": "sentry"
+    }
+  },
+  {
+    "status": "active",
+    "features": [
+      "data-forwarding",
+      "rate-limits"
+    ],
+    "color": "#bf873f",
+    "isInternal": true,
+    "isPublic": false,
+    "dateCreated": "2018-12-11T10:21:47.440Z",
+    "id": "1",
+    "slug": "internal",
+    "name": "Internal",
+    "hasAccess": true,
+    "isBookmarked": false,
+    "platform": null,
+    "firstEvent": "2018-12-11T10:54:35Z",
+    "avatar": {
+      "avatarUuid": null,
+      "avatarType": "letter_avatar"
+    },
+    "isMember": true,
+    "organization": {
+      "status": {
+        "id": "active",
+        "name": "active"
+      },
+      "require2FA": false,
+      "avatar": {
+        "avatarUuid": null,
+        "avatarType": "letter_avatar"
+      },
+      "name": "Sentry",
+      "dateCreated": "2018-12-11T10:21:47.431Z",
+      "id": "1",
+      "isEarlyAdopter": false,
+      "slug": "sentry"
+    }
+  }
+]

spec/lib/sentry/client_spec.rb

@@ -3,30 +3,76 @@
 require 'spec_helper'
 
 describe Sentry::Client do
-  let(:issue_status) { 'unresolved' }
-  let(:limit) { 20 }
   let(:sentry_url) { 'https://sentrytest.gitlab.com/api/0/projects/sentry-org/sentry-project' }
   let(:token) { 'test-token' }
 
-  let(:sample_response) do
+  let(:issues_sample_response) do
     Gitlab::Utils.deep_indifferent_access(
-      JSON.parse(File.read(Rails.root.join('spec/fixtures/sentry/issues_sample_response.json')))
+      JSON.parse(fixture_file('sentry/issues_sample_response.json'))
+    )
+  end
+
+  let(:projects_sample_response) do
+    Gitlab::Utils.deep_indifferent_access(
+      JSON.parse(fixture_file('sentry/list_projects_sample_response.json'))
     )
   end
 
   subject(:client) { described_class.new(sentry_url, token) }
 
-  describe '#list_issues' do
-    subject { client.list_issues(issue_status: issue_status, limit: limit) }
+  # Requires sentry_api_url and subject to be defined
+  shared_examples 'no redirects' do
+    let(:redirect_to) { 'https://redirected.example.com' }
+    let(:other_url) { 'https://other.example.org' }
+
+    let!(:redirected_req_stub) { stub_sentry_request(other_url) }
+
+    let!(:redirect_req_stub) do
+      stub_sentry_request(
+        sentry_api_url,
+        status: 302,
+        headers: { location: redirect_to }
+      )
+    end
 
-    before do
-      stub_sentry_request(sentry_url + '/issues/?limit=20&query=is:unresolved', body: sample_response)
+    it 'does not follow redirects' do
+      expect { subject }.to raise_exception(Sentry::Client::Error, 'Sentry response error: 302')
+      expect(redirect_req_stub).to have_been_requested
+      expect(redirected_req_stub).not_to have_been_requested
     end
+  end
 
-    it 'returns objects of type ErrorTracking::Error' do
-      expect(subject.length).to eq(1)
-      expect(subject[0]).to be_a(Gitlab::ErrorTracking::Error)
+  shared_examples 'has correct return type' do |klass|
+    it "returns objects of type #{klass}" do
+      expect(subject).to all( be_a(klass) )
     end
+  end
+
+  shared_examples 'has correct length' do |length|
+    it { expect(subject.length).to eq(length) }
+  end
+
+  # Requires sentry_api_request and subject to be defined
+  shared_examples 'calls sentry api' do
+    it 'calls sentry api' do
+      subject
+
+      expect(sentry_api_request).to have_been_requested
+    end
+  end
+
+  describe '#list_issues' do
+    let(:issue_status) { 'unresolved' }
+    let(:limit) { 20 }
+
+    let!(:sentry_api_request) { stub_sentry_request(sentry_url + '/issues/?limit=20&query=is:unresolved', body: issues_sample_response) }
+
+    subject { client.list_issues(issue_status: issue_status, limit: limit) }
+
+    it_behaves_like 'calls sentry api'
+
+    it_behaves_like 'has correct return type', Gitlab::ErrorTracking::Error
+    it_behaves_like 'has correct length', 1
 
     context 'error object created from sentry response' do
       using RSpec::Parameterized::TableSyntax
@@ -50,7 +96,7 @@ describe Sentry::Client do
       end
 
       with_them do
-        it { expect(subject[0].public_send(error_object)).to eq(sample_response[0].dig(*sentry_response)) }
+        it { expect(subject[0].public_send(error_object)).to eq(issues_sample_response[0].dig(*sentry_response)) }
       end
 
       context 'external_url' do
@@ -61,24 +107,9 @@ describe Sentry::Client do
     end
 
     context 'redirects' do
-      let(:redirect_to) { 'https://redirected.example.com' }
-      let(:other_url) { 'https://other.example.org' }
-
-      let!(:redirected_req_stub) { stub_sentry_request(other_url) }
-
-      let!(:redirect_req_stub) do
-        stub_sentry_request(
-          sentry_url + '/issues/?limit=20&query=is:unresolved',
-          status: 302,
-          headers: { location: redirect_to }
-        )
-      end
+      let(:sentry_api_url) { sentry_url + '/issues/?limit=20&query=is:unresolved' }
 
-      it 'does not follow redirects' do
-        expect { subject }.to raise_exception(Sentry::Client::Error, 'Sentry response error: 302')
-        expect(redirect_req_stub).to have_been_requested
-        expect(redirected_req_stub).not_to have_been_requested
-      end
+      it_behaves_like 'no redirects'
     end
 
     # Sentry API returns 404 if there are extra slashes in the URL!
@@ -99,7 +130,75 @@ describe Sentry::Client do
           anything
         ).and_call_original
 
-        client.list_issues(issue_status: issue_status, limit: limit)
+        subject
+
+        expect(valid_req_stub).to have_been_requested
+      end
+    end
+  end
+
+  describe '#list_projects' do
+    let(:sentry_list_projects_url) { 'https://sentrytest.gitlab.com/api/0/projects/' }
+
+    let!(:sentry_api_request) { stub_sentry_request(sentry_list_projects_url, body: projects_sample_response) }
+
+    subject { client.list_projects }
+
+    it_behaves_like 'calls sentry api'
+
+    it_behaves_like 'has correct return type', Gitlab::ErrorTracking::Project
+    it_behaves_like 'has correct length', 2
+
+    context 'keys missing in API response' do
+      it 'raises exception' do
+        projects_sample_response[0].delete(:slug)
+
+        stub_sentry_request(sentry_list_projects_url, body: projects_sample_response)
+
+        expect { subject }.to raise_error(Sentry::Client::SentryError, 'Sentry API response is missing keys. key not found: "slug"')
+      end
+    end
+
+    context 'error object created from sentry response' do
+      using RSpec::Parameterized::TableSyntax
+
+      where(:sentry_project_object, :sentry_response) do
+        :id                | :id
+        :name              | :name
+        :status            | :status
+        :slug              | :slug
+        :organization_name | [:organization, :name]
+        :organization_id   | [:organization, :id]
+        :organization_slug | [:organization, :slug]
+      end
+
+      with_them do
+        it { expect(subject[0].public_send(sentry_project_object)).to eq(projects_sample_response[0].dig(*sentry_response)) }
+      end
+    end
+
+    context 'redirects' do
+      let(:sentry_api_url) { sentry_list_projects_url }
+
+      it_behaves_like 'no redirects'
+    end
+
+    # Sentry API returns 404 if there are extra slashes in the URL!
+    context 'extra slashes in URL' do
+      let(:sentry_url) { 'https://sentrytest.gitlab.com/api//0/projects//' }
+      let(:client) { described_class.new(sentry_url, token) }
+
+      let!(:valid_req_stub) do
+        stub_sentry_request(sentry_list_projects_url)
+      end
+
+      it 'removes extra slashes in api url' do
+        expect(Gitlab::HTTP).to receive(:get).with(
+          URI(sentry_list_projects_url),
+          anything
+        ).and_call_original
+
+        subject
 
         expect(valid_req_stub).to have_been_requested
       end