Commit cbeb875c authored by Aishwarya Subramanian's avatar Aishwarya Subramanian

Backend review suggestions

Updated api attribute to issue_keys
instead of is_present as it is more verbose
Updated spec descriptions
Using params block in api
Updated use case when Jira enforcement is
not required for MR merge
parent c33bbe00
......@@ -3,8 +3,8 @@
module API
module Entities
class MergeRequestJiraIssue < Grape::Entity
expose :is_present do |merge_request|
Atlassian::JiraIssueKeyExtractor.has_keys?(merge_request.title, merge_request.description)
expose :issue_keys, if: -> (merge_request, _) { merge_request.project&.project_setting&.prevent_merge_without_jira_issue } do |merge_request|
Atlassian::JiraIssueKeyExtractor.new(merge_request.title, merge_request.description).issue_keys
end
end
end
......
......@@ -27,11 +27,14 @@ module API
# Examples:
# GET /projects/:id/merge_requests/:merge_request_iid/jira_issue
desc 'Get Jira Issue association for merge request'
params do
requires :id, type: String, desc: 'The ID of a project'
requires :merge_request_iid, types: [Integer, String], desc: 'The IID of a merge request'
end
get 'jira_issue' do
forbidden! unless can?(current_user, :read_merge_request, user_project)
not_found! unless user_project.project_setting.prevent_merge_without_jira_issue
merge_request = find_merge_request_with_access(params[:merge_request_iid])
merge_request = find_merge_request_with_access(declared(params)[:merge_request_iid])
present_jira_issue(merge_request)
end
......
......@@ -12,14 +12,6 @@ RSpec.describe API::MergeRequestJiraIssue do
subject { get api(url, current_user) }
shared_examples 'not found' do
specify do
subject
expect(response).to have_gitlab_http_status(:not_found)
end
end
context 'when feature is available' do
before do
stub_licensed_features(jira_issue_association_enforcement: true)
......@@ -49,7 +41,11 @@ RSpec.describe API::MergeRequestJiraIssue do
project.create_project_setting(prevent_merge_without_jira_issue: false)
end
it_behaves_like 'not found'
it 'does not include attribute issue_keys' do
subject
expect(json_response).not_to have_key(:issue_keys)
end
end
context 'when jira issue is required for merge' do
......@@ -57,17 +53,17 @@ RSpec.describe API::MergeRequestJiraIssue do
project.create_project_setting(prevent_merge_without_jira_issue: true)
end
it 'responds with not found' do
it 'responds with status :ok' do
subject
expect(response).to have_gitlab_http_status(:ok)
end
context 'when Jira issue is not provided in MR title/description' do
it 'responds with not found' do
it 'responds with empty issue key' do
subject
expect(json_response["is_present"]).to eq(false)
expect(json_response["issue_keys"]).to be_empty
end
end
......@@ -76,22 +72,22 @@ RSpec.describe API::MergeRequestJiraIssue do
merge_request.update!(title: 'Fix PRODUCT-1234')
end
it 'responds with not found' do
it 'responds with the issue key in MR title' do
subject
expect(json_response["is_present"]).to eq(true)
expect(json_response["issue_keys"]).to eq(["PRODUCT-1234"])
end
end
context 'when Jira issue is provided in MR description' do
before do
merge_request.update!(description: 'Jira issue associated: PRODUCT-1234')
merge_request.update!(description: 'Jira issue associated: FEATURE-1234')
end
it 'responds with not found' do
it 'responds with the issue key in MR description' do
subject
expect(json_response["is_present"]).to eq(true)
expect(json_response["issue_keys"]).to eq(["FEATURE-1234"])
end
end
end
......@@ -103,6 +99,14 @@ RSpec.describe API::MergeRequestJiraIssue do
let(:current_user) { user }
shared_examples 'not found' do
specify do
subject
expect(response).to have_gitlab_http_status(:not_found)
end
end
where(:licensed, :feature_flag) do
false | true
true | false
......
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