Commit ca28f7f1 authored by Dmitriy Zaporozhets's avatar Dmitriy Zaporozhets

Merge branch '63690-issue-trackers-title' into 'master'

Use title and description fields for issue trackers

Closes #63690

See merge request gitlab-org/gitlab-ce!30096
parents ff75b6b2 ddbbf453
......@@ -3,22 +3,14 @@
class BugzillaService < IssueTrackerService
validates :project_url, :issues_url, :new_issue_url, presence: true, public_url: true, if: :activated?
prop_accessor :title, :description, :project_url, :issues_url, :new_issue_url
prop_accessor :project_url, :issues_url, :new_issue_url
def title
if self.properties && self.properties['title'].present?
self.properties['title']
else
'Bugzilla'
end
def default_title
'Bugzilla'
end
def description
if self.properties && self.properties['description'].present?
self.properties['description']
else
'Bugzilla issue tracker'
end
def default_description
s_('IssueTracker|Bugzilla issue tracker')
end
def self.to_param
......
......@@ -5,24 +5,12 @@ class CustomIssueTrackerService < IssueTrackerService
prop_accessor :title, :description, :project_url, :issues_url, :new_issue_url
def title
if self.properties && self.properties['title'].present?
self.properties['title']
else
'Custom Issue Tracker'
end
def default_title
'Custom Issue Tracker'
end
def title=(value)
self.properties['title'] = value if self.properties
end
def description
if self.properties && self.properties['description'].present?
self.properties['description']
else
'Custom issue tracker'
end
def default_description
s_('IssueTracker|Custom issue tracker')
end
def self.to_param
......
......@@ -5,10 +5,18 @@ class GitlabIssueTrackerService < IssueTrackerService
validates :project_url, :issues_url, :new_issue_url, presence: true, public_url: true, if: :activated?
prop_accessor :title, :description, :project_url, :issues_url, :new_issue_url
prop_accessor :project_url, :issues_url, :new_issue_url
default_value_for :default, true
def default_title
'GitLab'
end
def default_description
s_('IssueTracker|GitLab issue tracker')
end
def self.to_param
'gitlab'
end
......
......@@ -5,6 +5,8 @@ class IssueTrackerService < Service
default_value_for :category, 'issue_tracker'
before_save :handle_properties
# Pattern used to extract links from comments
# Override this method on services that uses different patterns
# This pattern does not support cross-project references
......@@ -18,6 +20,37 @@ class IssueTrackerService < Service
end
end
# this will be removed as part of https://gitlab.com/gitlab-org/gitlab-ce/issues/63084
def title
if title_attribute = read_attribute(:title)
title_attribute
elsif self.properties && self.properties['title'].present?
self.properties['title']
else
default_title
end
end
# this will be removed as part of https://gitlab.com/gitlab-org/gitlab-ce/issues/63084
def description
if description_attribute = read_attribute(:description)
description_attribute
elsif self.properties && self.properties['description'].present?
self.properties['description']
else
default_description
end
end
def handle_properties
properties.slice('title', 'description').each do |key, _|
current_value = self.properties.delete(key)
value = attribute_changed?(key) ? attribute_change(key).last : current_value
write_attribute(key, value)
end
end
def default?
default
end
......
......@@ -17,7 +17,7 @@ class JiraService < IssueTrackerService
# Jira Cloud version is deprecating authentication via username and password.
# We should use username/password for Jira Server and email/api_token for Jira Cloud,
# for more information check: https://gitlab.com/gitlab-org/gitlab-ce/issues/49936.
prop_accessor :username, :password, :url, :api_url, :jira_issue_transition_id, :title, :description
prop_accessor :username, :password, :url, :api_url, :jira_issue_transition_id
before_update :reset_password
......@@ -37,7 +37,6 @@ class JiraService < IssueTrackerService
def initialize_properties
super do
self.properties = {
title: issues_tracker['title'],
url: issues_tracker['url'],
api_url: issues_tracker['api_url']
}
......@@ -74,20 +73,12 @@ class JiraService < IssueTrackerService
[Jira service documentation](#{help_page_url('user/project/integrations/jira')})."
end
def title
if self.properties && self.properties['title'].present?
self.properties['title']
else
'Jira'
end
def default_title
'Jira'
end
def description
if self.properties && self.properties['description'].present?
self.properties['description']
else
s_('JiraService|Jira issue tracker')
end
def default_description
s_('JiraService|Jira issue tracker')
end
def self.to_param
......
......@@ -3,22 +3,14 @@
class RedmineService < IssueTrackerService
validates :project_url, :issues_url, :new_issue_url, presence: true, public_url: true, if: :activated?
prop_accessor :title, :description, :project_url, :issues_url, :new_issue_url
prop_accessor :project_url, :issues_url, :new_issue_url
def title
if self.properties && self.properties['title'].present?
self.properties['title']
else
'Redmine'
end
def default_title
'Redmine'
end
def description
if self.properties && self.properties['description'].present?
self.properties['description']
else
'Redmine issue tracker'
end
def default_description
s_('IssueTracker|Redmine issue tracker')
end
def self.to_param
......
......@@ -3,7 +3,7 @@
class YoutrackService < IssueTrackerService
validates :project_url, :issues_url, presence: true, public_url: true, if: :activated?
prop_accessor :description, :project_url, :issues_url
prop_accessor :project_url, :issues_url
# {PROJECT-KEY}-{NUMBER} Examples: YT-1, PRJ-1, gl-030
def self.reference_pattern(only_long: false)
......@@ -14,16 +14,12 @@ class YoutrackService < IssueTrackerService
end
end
def title
def default_title
'YouTrack'
end
def description
if self.properties && self.properties['description'].present?
self.properties['description']
else
'YouTrack issue tracker'
end
def default_description
s_('IssueTracker|YouTrack issue tracker')
end
def self.to_param
......
......@@ -129,7 +129,7 @@ class Service < ApplicationRecord
def api_field_names
fields.map { |field| field[:name] }
.reject { |field_name| field_name =~ /(password|token|key)/ }
.reject { |field_name| field_name =~ /(password|token|key|title|description)/ }
end
def global_fields
......
# frozen_string_literal: true
# See http://doc.gitlab.com/ce/development/migration_style_guide.html
# for more information on how to write migrations for GitLab.
class AddDescriptionToServices < ActiveRecord::Migration[5.1]
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
def change
add_column :services, :description, :string, limit: 500
end
end
......@@ -3029,6 +3029,7 @@ ActiveRecord::Schema.define(version: 20190628145246) do
t.boolean "job_events", default: false, null: false
t.boolean "confidential_note_events", default: true
t.boolean "deployment_events", default: false, null: false
t.string "description", limit: 500
t.index ["project_id"], name: "index_services_on_project_id", using: :btree
t.index ["template"], name: "index_services_on_template", using: :btree
t.index ["type"], name: "index_services_on_type", using: :btree
......
......@@ -5607,6 +5607,21 @@ msgstr ""
msgid "IssueBoards|Boards"
msgstr ""
msgid "IssueTracker|Bugzilla issue tracker"
msgstr ""
msgid "IssueTracker|Custom issue tracker"
msgstr ""
msgid "IssueTracker|GitLab issue tracker"
msgstr ""
msgid "IssueTracker|Redmine issue tracker"
msgstr ""
msgid "IssueTracker|YouTrack issue tracker"
msgstr ""
msgid "Issues"
msgstr ""
......
......@@ -6,8 +6,6 @@ FactoryBot.define do
factory :custom_issue_tracker_service, class: CustomIssueTrackerService do
project
type 'CustomIssueTrackerService'
category 'issue_tracker'
active true
properties(
project_url: 'https://project.url.com',
......@@ -54,6 +52,38 @@ FactoryBot.define do
)
end
factory :bugzilla_service do
project
active true
issue_tracker
end
factory :redmine_service do
project
active true
issue_tracker
end
factory :youtrack_service do
project
active true
issue_tracker
end
factory :gitlab_issue_tracker_service do
project
active true
issue_tracker
end
trait :issue_tracker do
properties(
project_url: 'http://issue-tracker.example.com',
issues_url: 'http://issue-tracker.example.com',
new_issue_url: 'http://issue-tracker.example.com'
)
end
factory :jira_cloud_service, class: JiraService do
project
active true
......
......@@ -429,6 +429,7 @@ Service:
- confidential_issues_events
- confidential_note_events
- deployment_events
- description
ProjectHook:
- id
- url
......
......@@ -32,4 +32,49 @@ describe BugzillaService do
it { is_expected.not_to validate_presence_of(:new_issue_url) }
end
end
context 'overriding properties' do
let(:default_title) { 'JIRA' }
let(:default_description) { 'JiraService|Jira issue tracker' }
let(:url) { 'http://bugzilla.example.com' }
let(:access_params) do
{ project_url: url, issues_url: url, new_issue_url: url }
end
# this will be removed as part of https://gitlab.com/gitlab-org/gitlab-ce/issues/63084
context 'when data are stored in properties' do
let(:properties) { access_params.merge(title: title, description: description) }
let(:service) { create(:bugzilla_service, properties: properties) }
include_examples 'issue tracker fields'
end
context 'when data are stored in separated fields' do
let(:service) do
create(:bugzilla_service, title: title, description: description, properties: access_params)
end
include_examples 'issue tracker fields'
end
context 'when data are stored in both properties and separated fields' do
let(:properties) { access_params.merge(title: 'wrong title', description: 'wrong description') }
let(:service) do
create(:bugzilla_service, title: title, description: description, properties: properties)
end
include_examples 'issue tracker fields'
end
context 'when no title & description are set' do
let(:service) do
create(:bugzilla_service, properties: access_params)
end
it 'returns default values' do
expect(service.title).to eq('Bugzilla')
expect(service.description).to eq('Bugzilla issue tracker')
end
end
end
end
......@@ -48,4 +48,47 @@ describe CustomIssueTrackerService do
end
end
end
context 'overriding properties' do
let(:url) { 'http://custom.example.com' }
let(:access_params) do
{ project_url: url, issues_url: url, new_issue_url: url }
end
# this will be removed as part of https://gitlab.com/gitlab-org/gitlab-ce/issues/63084
context 'when data are stored in properties' do
let(:properties) { access_params.merge(title: title, description: description) }
let(:service) { create(:custom_issue_tracker_service, properties: properties) }
include_examples 'issue tracker fields'
end
context 'when data are stored in separated fields' do
let(:service) do
create(:custom_issue_tracker_service, title: title, description: description, properties: access_params)
end
include_examples 'issue tracker fields'
end
context 'when data are stored in both properties and separated fields' do
let(:properties) { access_params.merge(title: 'wrong title', description: 'wrong description') }
let(:service) do
create(:custom_issue_tracker_service, title: title, description: description, properties: properties)
end
include_examples 'issue tracker fields'
end
context 'when no title & description are set' do
let(:service) do
create(:custom_issue_tracker_service, properties: access_params)
end
it 'returns default values' do
expect(service.title).to eq('Custom Issue Tracker')
expect(service.description).to eq('Custom issue tracker')
end
end
end
end
......@@ -51,4 +51,47 @@ describe GitlabIssueTrackerService do
end
end
end
context 'overriding properties' do
let(:url) { 'http://gitlab.example.com' }
let(:access_params) do
{ project_url: url, issues_url: url, new_issue_url: url }
end
# this will be removed as part of https://gitlab.com/gitlab-org/gitlab-ce/issues/63084
context 'when data are stored in properties' do
let(:properties) { access_params.merge(title: title, description: description) }
let(:service) { create(:gitlab_issue_tracker_service, properties: properties) }
include_examples 'issue tracker fields'
end
context 'when data are stored in separated fields' do
let(:service) do
create(:gitlab_issue_tracker_service, title: title, description: description, properties: access_params)
end
include_examples 'issue tracker fields'
end
context 'when data are stored in both properties and separated fields' do
let(:properties) { access_params.merge(title: 'wrong title', description: 'wrong description') }
let(:service) do
create(:gitlab_issue_tracker_service, title: title, description: description, properties: properties)
end
include_examples 'issue tracker fields'
end
context 'when no title & description are set' do
let(:service) do
create(:gitlab_issue_tracker_service, properties: access_params)
end
it 'returns default values' do
expect(service.title).to eq('GitLab')
expect(service.description).to eq('GitLab issue tracker')
end
end
end
end
......@@ -115,6 +115,70 @@ describe JiraService do
end
end
describe '#create' do
let(:params) do
{
project: create(:project), title: 'custom title', description: 'custom description'
}
end
subject { described_class.create(params) }
it 'does not store title & description into properties' do
expect(subject.properties.keys).not_to include('title', 'description')
end
it 'sets title & description correctly' do
service = subject
expect(service.title).to eq('custom title')
expect(service.description).to eq('custom description')
end
end
context 'overriding properties' do
let(:url) { 'http://issue_tracker.example.com' }
let(:access_params) do
{ url: url, username: 'username', password: 'password' }
end
# this will be removed as part of https://gitlab.com/gitlab-org/gitlab-ce/issues/63084
context 'when data are stored in properties' do
let(:properties) { access_params.merge(title: title, description: description) }
let(:service) { create(:jira_service, properties: properties) }
include_examples 'issue tracker fields'
end
context 'when data are stored in separated fields' do
let(:service) do
create(:jira_service, title: title, description: description, properties: access_params)
end
include_examples 'issue tracker fields'
end
context 'when data are stored in both properties and separated fields' do
let(:properties) { access_params.merge(title: 'wrong title', description: 'wrong description') }
let(:service) do
create(:jira_service, title: title, description: description, properties: properties)
end
include_examples 'issue tracker fields'
end
context 'when no title & description are set' do
let(:service) do
create(:jira_service, properties: access_params)
end
it 'returns default values' do
expect(service.title).to eq('Jira')
expect(service.description).to eq('Jira issue tracker')
end
end
end
describe '#close_issue' do
let(:custom_base_url) { 'http://custom_url' }
let(:user) { create(:user) }
......@@ -450,36 +514,54 @@ describe JiraService do
end
describe 'description and title' do
let(:project) { create(:project) }
let(:title) { 'Jira One' }
let(:description) { 'Jira One issue tracker' }
let(:properties) do
{
url: 'http://jira.example.com/web',
username: 'mic',
password: 'password',
title: title,
description: description
}
end
context 'when it is not set' do
before do
@service = project.create_jira_service(active: true)
end
it 'default values are returned' do
service = create(:jira_service)
after do
@service.destroy!
expect(service.title).to eq('Jira')
expect(service.description).to eq('Jira issue tracker')
end
end
it 'is initialized' do
expect(@service.title).to eq('Jira')
expect(@service.description).to eq('Jira issue tracker')
context 'when it is set in properties' do
it 'values from properties are returned' do
service = create(:jira_service, properties: properties)
expect(service.title).to eq(title)
expect(service.description).to eq(description)
end
end
context 'when it is set' do
before do
properties = { 'title' => 'Jira One', 'description' => 'Jira One issue tracker' }
@service = project.create_jira_service(active: true, properties: properties)
end
context 'when it is in title & description fields' do
it 'values from title and description fields are returned' do
service = create(:jira_service, title: title, description: description)
after do
@service.destroy!
expect(service.title).to eq(title)
expect(service.description).to eq(description)
end
end
context 'when it is in both properites & title & description fields' do
it 'values from title and description fields are returned' do
title2 = 'Jira 2'
description2 = 'Jira description 2'
it 'is correct' do
expect(@service.title).to eq('Jira One')
expect(@service.description).to eq('Jira One issue tracker')
service = create(:jira_service, title: title2, description: description2, properties: properties)
expect(service.title).to eq(title2)
expect(service.description).to eq(description2)
end
end
end
......@@ -505,29 +587,21 @@ describe JiraService do
end
describe 'project and issue urls' do
let(:project) { create(:project) }
context 'when gitlab.yml was initialized' do
before do
it 'is prepopulated with the settings' do
settings = {
'jira' => {
'title' => 'Jira',
'url' => 'http://jira.sample/projects/project_a',
'api_url' => 'http://jira.sample/api'
}
}
allow(Gitlab.config).to receive(:issues_tracker).and_return(settings)
@service = project.create_jira_service(active: true)
end
after do
@service.destroy!
end
project = create(:project)
service = project.create_jira_service(active: true)
it 'is prepopulated with the settings' do
expect(@service.properties['title']).to eq('Jira')
expect(@service.properties['url']).to eq('http://jira.sample/projects/project_a')
expect(@service.properties['api_url']).to eq('http://jira.sample/api')
expect(service.properties['url']).to eq('http://jira.sample/projects/project_a')
expect(service.properties['api_url']).to eq('http://jira.sample/api')
end
end
end
......
......@@ -40,4 +40,47 @@ describe RedmineService do
expect(described_class.reference_pattern.match('#123')[:issue]).to eq('123')
end
end
context 'overriding properties' do
let(:url) { 'http://redmine.example.com' }
let(:access_params) do
{ project_url: url, issues_url: url, new_issue_url: url }
end
# this will be removed as part of https://gitlab.com/gitlab-org/gitlab-ce/issues/63084
context 'when data are stored in properties' do
let(:properties) { access_params.merge(title: title, description: description) }
let(:service) { create(:redmine_service, properties: properties) }
include_examples 'issue tracker fields'
end
context 'when data are stored in separated fields' do
let(:service) do
create(:redmine_service, title: title, description: description, properties: access_params)
end
include_examples 'issue tracker fields'
end
context 'when data are stored in both properties and separated fields' do
let(:properties) { access_params.merge(title: 'wrong title', description: 'wrong description') }
let(:service) do
create(:redmine_service, title: title, description: description, properties: properties)
end
include_examples 'issue tracker fields'
end
context 'when no title & description are set' do
let(:service) do
create(:redmine_service, properties: access_params)
end
it 'returns default values' do
expect(service.title).to eq('Redmine')
expect(service.description).to eq('Redmine issue tracker')
end
end
end
end
......@@ -37,4 +37,47 @@ describe YoutrackService do
expect(described_class.reference_pattern.match('YT-123')[:issue]).to eq('YT-123')
end
end
context 'overriding properties' do
let(:url) { 'http://youtrack.example.com' }
let(:access_params) do
{ project_url: url, issues_url: url, new_issue_url: url }
end
# this will be removed as part of https://gitlab.com/gitlab-org/gitlab-ce/issues/63084
context 'when data are stored in properties' do
let(:properties) { access_params.merge(title: title, description: description) }
let(:service) { create(:youtrack_service, properties: properties) }
include_examples 'issue tracker fields'
end
context 'when data are stored in separated fields' do
let(:service) do
create(:youtrack_service, title: title, description: description, properties: access_params)
end
include_examples 'issue tracker fields'
end
context 'when data are stored in both properties and separated fields' do
let(:properties) { access_params.merge(title: 'wrong title', description: 'wrong description') }
let(:service) do
create(:youtrack_service, title: title, description: description, properties: properties)
end
include_examples 'issue tracker fields'
end
context 'when no title & description are set' do
let(:service) do
create(:youtrack_service, properties: access_params)
end
it 'returns default values' do
expect(service.title).to eq('YouTrack')
expect(service.description).to eq('YouTrack issue tracker')
end
end
end
end
......@@ -244,7 +244,8 @@ describe Service do
let(:service) do
GitlabIssueTrackerService.create(
project: create(:project),
title: 'random title'
title: 'random title',
project_url: 'http://gitlab.example.com'
)
end
......@@ -252,8 +253,12 @@ describe Service do
expect { service }.not_to raise_error
end
it 'sets title correctly' do
expect(service.title).to eq('random title')
end
it 'creates the properties' do
expect(service.properties).to eq({ "title" => "random title" })
expect(service.properties).to eq({ "project_url" => "http://gitlab.example.com" })
end
end
......
# frozen_string_literal: true
shared_examples 'issue tracker fields' do
let(:title) { 'custom title' }
let(:description) { 'custom description' }
let(:url) { 'http://issue_tracker.example.com' }
context 'when data are stored in the properties' do
describe '#update' do
before do
service.update(title: 'new_title', description: 'new description')
end
it 'removes title and description from properties' do
expect(service.reload.properties).not_to include('title', 'description')
end
it 'stores title & description in services table' do
expect(service.read_attribute(:title)).to eq('new_title')
expect(service.read_attribute(:description)).to eq('new description')
end
end
describe 'reading fields' do
it 'returns correct values' do
expect(service.title).to eq(title)
expect(service.description).to eq(description)
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