Commit 74ea508e authored by Imre Farkas's avatar Imre Farkas

Merge branch 'remove_jira_connect_app_feature_flag' into 'master'

Remove jira_connect_app feature flag

See merge request gitlab-org/gitlab!24818
parents e8f51a55 a4806fa0
......@@ -15,8 +15,6 @@ The following are required to install and test the app:
or [ngrok](https://ngrok.com). These also take care of SSL for you because Jira
requires all connections to the app host to be over SSL.
> This feature is currently behind the `:jira_connect_app` feature flag
## Installing the app in Jira
1. Enable Jira development mode to install apps that are not from the Atlassian Marketplace
......
......@@ -5,17 +5,12 @@ class JiraConnect::ApplicationController < ApplicationController
skip_before_action :authenticate_user!
skip_before_action :verify_authenticity_token
before_action :check_feature_flag_enabled!
before_action :verify_atlassian_jwt!
attr_reader :current_jira_installation
private
def check_feature_flag_enabled!
render_404 unless Feature.enabled?(:jira_connect_app)
end
def verify_atlassian_jwt!
return render_403 unless atlassian_jwt_valid?
......
......@@ -748,6 +748,10 @@ module EE
::Project.with_groups_level_repos_templates.exists?(id)
end
def jira_subscription_exists?
feature_available?(:jira_dev_panel_integration) && JiraConnectSubscription.for_project(self).exists?
end
private
def group_hooks
......
......@@ -11,7 +11,7 @@ module EE
def branch_change_hooks
super
return unless jira_subscription_exists?
return unless project.jira_subscription_exists?
branch_to_sync = branch_name if Atlassian::JiraIssueKeyExtractor.has_keys?(branch_name)
commits_to_sync = limited_commits.select { |commit| Atlassian::JiraIssueKeyExtractor.has_keys?(commit.safe_message) }.map(&:sha)
......@@ -21,12 +21,6 @@ module EE
end
end
def jira_subscription_exists?
::Feature.enabled?(:jira_connect_app) &&
project.feature_available?(:jira_dev_panel_integration) &&
JiraConnectSubscription.for_project(project).exists?
end
override :pipeline_options
def pipeline_options
mirror_update = project.mirror? &&
......
......@@ -9,7 +9,7 @@ module EE
def execute_hooks(merge_request, action = 'open', old_rev: nil, old_associations: {})
super
return unless jira_subscription_exists?
return unless project.jira_subscription_exists?
if Atlassian::JiraIssueKeyExtractor.has_keys?(merge_request.title, merge_request.description)
JiraConnect::SyncMergeRequestWorker.perform_async(merge_request.id)
......@@ -20,12 +20,6 @@ module EE
attr_accessor :blocking_merge_requests_params
def jira_subscription_exists?
::Feature.enabled?(:jira_connect_app) &&
project.feature_available?(:jira_dev_panel_integration) &&
JiraConnectSubscription.for_project(project).exists?
end
override :filter_params
def filter_params(merge_request)
unless current_user.can?(:update_approvers, merge_request)
......
---
title: Release Jira connect feature
merge_request: 24818
author:
type: added
......@@ -4,35 +4,17 @@ require 'spec_helper'
describe JiraConnect::AppDescriptorController do
describe '#show' do
context 'feature disabled' do
before do
stub_feature_flags(jira_connect_app: false)
end
it 'returns JSON app descriptor' do
get :show
it 'returns 404' do
get :show
expect(response).to have_gitlab_http_status(:not_found)
end
end
context 'feature enabled' do
before do
stub_feature_flags(jira_connect_app: true)
end
it 'returns JSON app descriptor' do
get :show
expect(response).to have_gitlab_http_status(:ok)
expect(json_response).to include(
'baseUrl' => 'https://test.host/-/jira_connect',
'lifecycle' => {
'installed' => '/events/installed',
'uninstalled' => '/events/uninstalled'
}
)
end
expect(response).to have_gitlab_http_status(:ok)
expect(json_response).to include(
'baseUrl' => 'https://test.host/-/jira_connect',
'lifecycle' => {
'installed' => '/events/installed',
'uninstalled' => '/events/uninstalled'
}
)
end
end
end
......@@ -3,63 +3,35 @@
require 'spec_helper'
describe JiraConnect::EventsController do
context 'feature disabled' do
before do
stub_feature_flags(jira_connect_app: false)
describe '#installed' do
subject do
post :installed, params: {
clientKey: '1234',
sharedSecret: 'secret',
baseUrl: 'https://test.atlassian.net'
}
end
describe '#installed' do
it 'returns 404' do
post :installed
expect(response).to have_gitlab_http_status(:not_found)
end
it 'saves the jira installation data' do
expect { subject }.to change { JiraConnectInstallation.count }.by(1)
end
describe '#uninstalled' do
it 'returns 404' do
post :uninstalled
it 'saves the correct values' do
subject
expect(response).to have_gitlab_http_status(:not_found)
end
end
end
installation = JiraConnectInstallation.find_by_client_key('1234')
context 'feature enabled' do
before do
stub_feature_flags(jira_connect_app: true)
expect(installation.shared_secret).to eq('secret')
expect(installation.base_url).to eq('https://test.atlassian.net')
end
describe '#installed' do
subject do
post :installed, params: {
clientKey: '1234',
sharedSecret: 'secret',
baseUrl: 'https://test.atlassian.net'
}
end
it 'saves the jira installation data' do
expect { subject }.to change { JiraConnectInstallation.count }.by(1)
end
context 'client key already exists' do
it 'returns 422' do
create(:jira_connect_installation, client_key: '1234')
it 'saves the correct values' do
subject
installation = JiraConnectInstallation.find_by_client_key('1234')
expect(installation.shared_secret).to eq('secret')
expect(installation.base_url).to eq('https://test.atlassian.net')
end
context 'client key already exists' do
it 'returns 422' do
create(:jira_connect_installation, client_key: '1234')
subject
expect(response).to have_gitlab_http_status(:unprocessable_entity)
end
expect(response).to have_gitlab_http_status(:unprocessable_entity)
end
end
......
......@@ -3,164 +3,126 @@
require 'spec_helper'
describe JiraConnect::SubscriptionsController do
context 'feature disabled' do
let_it_be(:installation) { create(:jira_connect_installation) }
describe '#index' do
before do
stub_feature_flags(jira_connect_app: false)
get :index, params: { jwt: jwt }
end
describe '#index' do
it 'returns 404' do
get :index
context 'without JWT' do
let(:jwt) { nil }
expect(response).to have_gitlab_http_status(:not_found)
it 'returns 403' do
expect(response).to have_gitlab_http_status(:forbidden)
end
end
describe '#create' do
it '#create returns 404' do
post :create
context 'with valid JWT' do
let(:qsh) { Atlassian::Jwt.create_query_string_hash('https://gitlab.test/subscriptions', 'GET', 'https://gitlab.test') }
let(:jwt) { Atlassian::Jwt.encode({ iss: installation.client_key, qsh: qsh }, installation.shared_secret) }
expect(response).to have_gitlab_http_status(:not_found)
it 'returns 200' do
expect(response).to have_gitlab_http_status(:ok)
end
end
describe '#destroy' do
let(:subscription) { create(:jira_connect_subscription) }
it '#destroy returns 404' do
delete :destroy, params: { id: subscription.id }
expect(response).to have_gitlab_http_status(:not_found)
it 'removes X-Frame-Options to allow rendering in iframe' do
expect(response.headers['X-Frame-Options']).to be_nil
end
end
end
context 'feature enabled' do
describe '#create' do
let(:group) { create(:group) }
let(:user) { create(:user) }
let(:current_user) { user }
before do
stub_feature_flags(jira_connect_app: true)
group.add_maintainer(user)
end
let(:installation) { create(:jira_connect_installation) }
subject { post :create, params: { jwt: jwt, namespace_path: group.path, format: :json } }
describe '#index' do
before do
get :index, params: { jwt: jwt }
end
context 'without JWT' do
let(:jwt) { nil }
it 'returns 403' do
expect(response).to have_gitlab_http_status(:forbidden)
end
end
context 'without JWT' do
let(:jwt) { nil }
context 'with valid JWT' do
let(:qsh) { Atlassian::Jwt.create_query_string_hash('https://gitlab.test/subscriptions', 'GET', 'https://gitlab.test') }
let(:jwt) { Atlassian::Jwt.encode({ iss: installation.client_key, qsh: qsh }, installation.shared_secret) }
it 'returns 403' do
sign_in(user)
it 'returns 200' do
expect(response).to have_gitlab_http_status(:ok)
end
subject
it 'removes X-Frame-Options to allow rendering in iframe' do
expect(response.headers['X-Frame-Options']).to be_nil
end
expect(response).to have_gitlab_http_status(:forbidden)
end
end
describe '#create' do
let(:group) { create(:group) }
let(:user) { create(:user) }
let(:current_user) { user }
before do
group.add_maintainer(user)
end
subject { post :create, params: { jwt: jwt, namespace_path: group.path, format: :json } }
context 'with valid JWT' do
let(:jwt) { Atlassian::Jwt.encode({ iss: installation.client_key }, installation.shared_secret) }
context 'without JWT' do
let(:jwt) { nil }
it 'returns 403' do
context 'signed in to GitLab' do
before do
sign_in(user)
subject
expect(response).to have_gitlab_http_status(:forbidden)
end
end
context 'with valid JWT' do
let(:jwt) { Atlassian::Jwt.encode({ iss: installation.client_key }, installation.shared_secret) }
context 'signed in to GitLab' do
context 'dev panel integration is available' do
before do
sign_in(user)
stub_licensed_features(jira_dev_panel_integration: true)
end
context 'dev panel integration is available' do
before do
stub_licensed_features(jira_dev_panel_integration: true)
end
it 'creates a subscription' do
expect { subject }.to change { installation.subscriptions.count }.from(0).to(1)
end
it 'creates a subscription' do
expect { subject }.to change { installation.subscriptions.count }.from(0).to(1)
end
it 'returns 200' do
subject
it 'returns 200' do
subject
expect(response).to have_gitlab_http_status(:ok)
end
expect(response).to have_gitlab_http_status(:ok)
end
end
context 'dev panel integration is not available' do
before do
stub_licensed_features(jira_dev_panel_integration: false)
end
context 'dev panel integration is not available' do
before do
stub_licensed_features(jira_dev_panel_integration: false)
end
it 'returns 422' do
subject
it 'returns 422' do
subject
expect(response).to have_gitlab_http_status(:unprocessable_entity)
end
expect(response).to have_gitlab_http_status(:unprocessable_entity)
end
end
end
context 'not signed in to GitLab' do
it 'returns 401' do
subject
context 'not signed in to GitLab' do
it 'returns 401' do
subject
expect(response).to have_gitlab_http_status(:unauthorized)
end
expect(response).to have_gitlab_http_status(:unauthorized)
end
end
end
end
describe '#destroy' do
let(:subscription) { create(:jira_connect_subscription, installation: installation) }
describe '#destroy' do
let(:subscription) { create(:jira_connect_subscription, installation: installation) }
before do
delete :destroy, params: { jwt: jwt, id: subscription.id }
end
before do
delete :destroy, params: { jwt: jwt, id: subscription.id }
end
context 'without JWT' do
let(:jwt) { nil }
context 'without JWT' do
let(:jwt) { nil }
it 'returns 403' do
expect(response).to have_gitlab_http_status(:forbidden)
end
it 'returns 403' do
expect(response).to have_gitlab_http_status(:forbidden)
end
end
context 'with valid JWT' do
let(:jwt) { Atlassian::Jwt.encode({ iss: installation.client_key }, installation.shared_secret) }
context 'with valid JWT' do
let(:jwt) { Atlassian::Jwt.encode({ iss: installation.client_key }, installation.shared_secret) }
it 'deletes the subscription' do
expect { subscription.reload }.to raise_error ActiveRecord::RecordNotFound
expect(response).to have_gitlab_http_status(:ok)
end
it 'deletes the subscription' do
expect { subscription.reload }.to raise_error ActiveRecord::RecordNotFound
expect(response).to have_gitlab_http_status(:ok)
end
end
end
......
......@@ -3,10 +3,6 @@
require 'spec_helper'
describe 'Subscriptions Content Security Policy' do
before do
stub_feature_flags(jira_connect_app: true)
end
let(:installation) { create(:jira_connect_installation) }
let(:qsh) { Atlassian::Jwt.create_query_string_hash('https://gitlab.test/subscriptions', 'GET', 'https://gitlab.test') }
let(:jwt) { Atlassian::Jwt.encode({ iss: installation.client_key, qsh: qsh }, installation.shared_secret) }
......
......@@ -2640,4 +2640,22 @@ describe Project do
end
end
end
describe '#jira_subscription_exists?' do
subject { project.jira_subscription_exists? }
context 'jira connect subscription exists' do
let!(:jira_connect_subscription) { create(:jira_connect_subscription, namespace: project.namespace) }
it { is_expected.to eq(false) }
context 'dev panel integration is available' do
before do
stub_licensed_features(jira_dev_panel_integration: true)
end
it { is_expected.to eq(true) }
end
end
end
end
......@@ -176,60 +176,46 @@ describe Git::BranchPushService do
end
end
context 'when feature is enabled' do
context 'has Jira dev panel integration license' do
before do
stub_feature_flags(jira_connect_app: true)
stub_licensed_features(jira_dev_panel_integration: true)
end
context 'has Jira dev panel integration license' do
context 'with a Jira subscription' do
before do
stub_licensed_features(jira_dev_panel_integration: true)
create(:jira_connect_subscription, namespace: project.namespace)
end
context 'with a Jira subscription' do
before do
create(:jira_connect_subscription, namespace: project.namespace)
end
context 'branch name contains Jira issue key' do
let(:branch_to_sync) { 'branch-JIRA-123' }
let(:ref) { "refs/heads/#{branch_to_sync}" }
context 'branch name contains Jira issue key' do
let(:branch_to_sync) { 'branch-JIRA-123' }
let(:ref) { "refs/heads/#{branch_to_sync}" }
it_behaves_like 'enqueues Jira sync worker'
end
context 'commit message contains Jira issue key' do
let(:commits_to_sync) { [newrev] }
it_behaves_like 'enqueues Jira sync worker'
end
before do
allow_any_instance_of(Commit).to receive(:safe_message).and_return('Commit with key JIRA-123')
end
context 'commit message contains Jira issue key' do
let(:commits_to_sync) { [newrev] }
it_behaves_like 'enqueues Jira sync worker'
before do
allow_any_instance_of(Commit).to receive(:safe_message).and_return('Commit with key JIRA-123')
end
context 'branch name and commit message does not contain Jira issue key' do
it_behaves_like 'does not enqueue Jira sync worker'
end
it_behaves_like 'enqueues Jira sync worker'
end
context 'without a Jira subscription' do
context 'branch name and commit message does not contain Jira issue key' do
it_behaves_like 'does not enqueue Jira sync worker'
end
end
context 'does not have Jira dev panel integration license' do
before do
stub_licensed_features(jira_dev_panel_integration: false)
end
context 'without a Jira subscription' do
it_behaves_like 'does not enqueue Jira sync worker'
end
end
context 'when feature is disabled' do
context 'does not have Jira dev panel integration license' do
before do
stub_feature_flags(jira_connect_app: false)
stub_licensed_features(jira_dev_panel_integration: false)
end
it_behaves_like 'does not enqueue Jira sync worker'
......
......@@ -35,49 +35,35 @@ describe MergeRequests::BaseService do
end
end
context 'when feature is enabled' do
context 'has Jira dev panel integration license' do
before do
stub_feature_flags(jira_connect_app: true)
stub_licensed_features(jira_dev_panel_integration: true)
end
context 'has Jira dev panel integration license' do
context 'with a Jira subscription' do
before do
stub_licensed_features(jira_dev_panel_integration: true)
create(:jira_connect_subscription, namespace: project.namespace)
end
context 'with a Jira subscription' do
before do
create(:jira_connect_subscription, namespace: project.namespace)
end
context 'MR contains Jira issue key' do
let(:title) { 'Awesome merge_request with issue JIRA-123' }
context 'MR contains Jira issue key' do
let(:title) { 'Awesome merge_request with issue JIRA-123' }
it_behaves_like 'enqueues Jira sync worker'
end
context 'MR does not contain Jira issue key' do
it_behaves_like 'does not enqueue Jira sync worker'
end
it_behaves_like 'enqueues Jira sync worker'
end
context 'without a Jira subscription' do
context 'MR does not contain Jira issue key' do
it_behaves_like 'does not enqueue Jira sync worker'
end
end
context 'does not have Jira dev panel integration license' do
before do
stub_licensed_features(jira_dev_panel_integration: false)
end
context 'without a Jira subscription' do
it_behaves_like 'does not enqueue Jira sync worker'
end
end
context 'when feature is disabled' do
context 'does not have Jira dev panel integration license' do
before do
stub_feature_flags(jira_connect_app: false)
stub_licensed_features(jira_dev_panel_integration: false)
end
it_behaves_like 'does not enqueue Jira sync worker'
......
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