From d5e38b00cffacf2f0599c99a1bb1515b6f56aa9b Mon Sep 17 00:00:00 2001 From: Kotau Yauhen Date: Thu, 21 Feb 2019 10:39:44 +0000 Subject: [PATCH] Remove new_issue_url field from YouTrack integration service --- .../project_services/youtrack_service.rb | 18 ++-- doc/user/project/integrations/youtrack.md | 1 - lib/api/services.rb | 6 -- spec/factories/projects.rb | 3 +- .../user_activates_issue_tracker_spec.rb | 1 - .../services/user_activates_youtrack_spec.rb | 89 +++++++++++++++++++ .../project_services/youtrack_service_spec.rb | 3 - 7 files changed, 101 insertions(+), 20 deletions(-) create mode 100644 spec/features/projects/services/user_activates_youtrack_spec.rb diff --git a/app/models/project_services/youtrack_service.rb b/app/models/project_services/youtrack_service.rb index ad9d26f3950..bf4a4166045 100644 --- a/app/models/project_services/youtrack_service.rb +++ b/app/models/project_services/youtrack_service.rb @@ -1,9 +1,9 @@ # frozen_string_literal: true class YoutrackService < IssueTrackerService - validates :project_url, :issues_url, :new_issue_url, presence: true, public_url: true, if: :activated? + validates :project_url, :issues_url, presence: true, public_url: true, if: :activated? - prop_accessor :title, :description, :project_url, :issues_url, :new_issue_url + prop_accessor :title, :description, :project_url, :issues_url # {PROJECT-KEY}-{NUMBER} Examples: YT-1, PRJ-1 def self.reference_pattern(only_long: false) @@ -15,11 +15,7 @@ class YoutrackService < IssueTrackerService end def title - if self.properties && self.properties['title'].present? - self.properties['title'] - else - 'YouTrack' - end + 'YouTrack' end def description @@ -33,4 +29,12 @@ class YoutrackService < IssueTrackerService def self.to_param 'youtrack' end + + def fields + [ + { type: 'text', name: 'description', placeholder: description }, + { type: 'text', name: 'project_url', placeholder: 'Project url', required: true }, + { type: 'text', name: 'issues_url', placeholder: 'Issue url', required: true } + ] + end end diff --git a/doc/user/project/integrations/youtrack.md b/doc/user/project/integrations/youtrack.md index 85c339d5fa9..3e9295dfcbd 100644 --- a/doc/user/project/integrations/youtrack.md +++ b/doc/user/project/integrations/youtrack.md @@ -10,7 +10,6 @@ in the table below. | `description` | A name for the issue tracker (to differentiate between instances, for example) | | `project_url` | The URL to the project in YouTrack which is being linked to this GitLab project | | `issues_url` | The URL to the issue in YouTrack project that is linked to this GitLab project. Note that the `issues_url` requires `:id` in the URL. This ID is used by GitLab as a placeholder to replace the issue number. | - | `new_issue_url` | This is the URL to create a new issue in YouTrack for the project linked to this GitLab project. **This is currently not being used and will be removed in a future release.** | Once you have configured and enabled YouTrack you'll see the YouTrack link on the GitLab project pages that takes you to the appropriate YouTrack project. diff --git a/lib/api/services.rb b/lib/api/services.rb index fcaec06061b..7967d4fe5e4 100644 --- a/lib/api/services.rb +++ b/lib/api/services.rb @@ -593,12 +593,6 @@ module API } ], 'youtrack' => [ - { - required: true, - name: :new_issue_url, - type: String, - desc: 'The new issue URL' - }, { required: true, name: :project_url, diff --git a/spec/factories/projects.rb b/spec/factories/projects.rb index 0efb15337ed..30d3b22d868 100644 --- a/spec/factories/projects.rb +++ b/spec/factories/projects.rb @@ -321,8 +321,7 @@ FactoryBot.define do active: true, properties: { 'project_url' => 'http://youtrack/projects/project_guid_in_youtrack', - 'issues_url' => 'http://youtrack/issues/:id', - 'new_issue_url' => 'http://youtrack/newIssue' + 'issues_url' => 'http://youtrack/issues/:id' } ) end diff --git a/spec/features/projects/services/user_activates_issue_tracker_spec.rb b/spec/features/projects/services/user_activates_issue_tracker_spec.rb index 4a71eea1add..7cd5b12802b 100644 --- a/spec/features/projects/services/user_activates_issue_tracker_spec.rb +++ b/spec/features/projects/services/user_activates_issue_tracker_spec.rb @@ -87,7 +87,6 @@ describe 'User activates issue tracker', :js do end it_behaves_like 'external issue tracker activation', tracker: 'Redmine' - it_behaves_like 'external issue tracker activation', tracker: 'YouTrack' it_behaves_like 'external issue tracker activation', tracker: 'Bugzilla' it_behaves_like 'external issue tracker activation', tracker: 'Custom Issue Tracker' end diff --git a/spec/features/projects/services/user_activates_youtrack_spec.rb b/spec/features/projects/services/user_activates_youtrack_spec.rb new file mode 100644 index 00000000000..bb6a030c1cf --- /dev/null +++ b/spec/features/projects/services/user_activates_youtrack_spec.rb @@ -0,0 +1,89 @@ +require 'spec_helper' + +describe 'User activates issue tracker', :js do + let(:user) { create(:user) } + let(:project) { create(:project) } + + let(:url) { 'http://tracker.example.com' } + + def fill_form(active = true) + check 'Active' if active + + fill_in 'service_project_url', with: url + fill_in 'service_issues_url', with: "#{url}/:id" + end + + before do + project.add_maintainer(user) + sign_in(user) + + visit project_settings_integrations_path(project) + end + + shared_examples 'external issue tracker activation' do |tracker:| + describe 'user sets and activates the Service' do + context 'when the connection test succeeds' do + before do + stub_request(:head, url).to_return(headers: { 'Content-Type' => 'application/json' }) + + click_link(tracker) + fill_form + click_button('Test settings and save changes') + wait_for_requests + end + + it 'activates the service' do + expect(page).to have_content("#{tracker} activated.") + expect(current_path).to eq(project_settings_integrations_path(project)) + end + + it 'shows the link in the menu' do + page.within('.nav-sidebar') do + expect(page).to have_link(tracker, href: url) + end + end + end + + context 'when the connection test fails' do + it 'activates the service' do + stub_request(:head, url).to_raise(HTTParty::Error) + + click_link(tracker) + fill_form + click_button('Test settings and save changes') + wait_for_requests + + expect(find('.flash-container-page')).to have_content 'Test failed.' + expect(find('.flash-container-page')).to have_content 'Save anyway' + + find('.flash-alert .flash-action').click + wait_for_requests + + expect(page).to have_content("#{tracker} activated.") + expect(current_path).to eq(project_settings_integrations_path(project)) + end + end + end + + describe 'user sets the service but keeps it disabled' do + before do + click_link(tracker) + fill_form(false) + click_button('Save changes') + end + + it 'saves but does not activate the service' do + expect(page).to have_content("#{tracker} settings saved, but not activated.") + expect(current_path).to eq(project_settings_integrations_path(project)) + end + + it 'does not show the external tracker link in the menu' do + page.within('.nav-sidebar') do + expect(page).not_to have_link(tracker, href: url) + end + end + end + end + + it_behaves_like 'external issue tracker activation', tracker: 'YouTrack' +end diff --git a/spec/models/project_services/youtrack_service_spec.rb b/spec/models/project_services/youtrack_service_spec.rb index b1e74a6f877..9524b526a46 100644 --- a/spec/models/project_services/youtrack_service_spec.rb +++ b/spec/models/project_services/youtrack_service_spec.rb @@ -14,10 +14,8 @@ describe YoutrackService do it { is_expected.to validate_presence_of(:project_url) } it { is_expected.to validate_presence_of(:issues_url) } - it { is_expected.to validate_presence_of(:new_issue_url) } it_behaves_like 'issue tracker service URL attribute', :project_url it_behaves_like 'issue tracker service URL attribute', :issues_url - it_behaves_like 'issue tracker service URL attribute', :new_issue_url end context 'when service is inactive' do @@ -27,7 +25,6 @@ describe YoutrackService do it { is_expected.not_to validate_presence_of(:project_url) } it { is_expected.not_to validate_presence_of(:issues_url) } - it { is_expected.not_to validate_presence_of(:new_issue_url) } end end