Download as
Browse Code »

Commit 5f25cdfe19c7c0a8c1ada592307e9017e2a754e1

Authored by Drew Blessing
1 parent 172ad962
Exists in spb-stable and in 2 other branches 7-0-stable, 7-0-stable-spb

Implement Merge Request Labels

Showing 15 changed files with 142 additions and 17 deletions   Show diff stats
app/assets/stylesheets/sections/merge_requests.scss
  Diff comments   View file @5f25cdf
@@ -74,6 +74,10 @@ @@ -74,6 +74,10 @@
74 74
75 .merge-request-info { 75 .merge-request-info {
76 color: #999; 76 color: #999;
  77 +
  78 + .merge-request-labels {
  79 + display: inline-block;
  80 + }
77 } 81 }
78 } 82 }
79 } 83 }
@@ -112,3 +116,7 @@ @@ -112,3 +116,7 @@
112 } 116 }
113 } 117 }
114 } 118 }
  119 +
  120 +.merge-request-show-labels .label {
  121 + padding: 6px 10px;
  122 +}
app/controllers/application_controller.rb
  Diff comments   View file @5f25cdf
@@ -117,6 +117,11 @@ class ApplicationController < ActionController::Base @@ -117,6 +117,11 @@ class ApplicationController < ActionController::Base
117 return access_denied! unless can?(current_user, :push_code, project) 117 return access_denied! unless can?(current_user, :push_code, project)
118 end 118 end
119 119
  120 + def authorize_labels!
  121 + # Labels should be accessible for issues and/or merge requests
  122 + authorize_read_issue! || authorize_read_merge_request!
  123 + end
  124 +
120 def access_denied! 125 def access_denied!
121 render "errors/access_denied", layout: "errors", status: 404 126 render "errors/access_denied", layout: "errors", status: 404
122 end 127 end
app/controllers/projects/labels_controller.rb
  Diff comments   View file @5f25cdf
1 class Projects::LabelsController < Projects::ApplicationController 1 class Projects::LabelsController < Projects::ApplicationController
2 before_filter :module_enabled 2 before_filter :module_enabled
3 3
4 - # Allow read any issue  
5 - before_filter :authorize_read_issue! 4 + before_filter :authorize_labels!
6 5
7 respond_to :js, :html 6 respond_to :js, :html
8 7
@@ -13,12 +12,18 @@ class Projects::LabelsController &lt; Projects::ApplicationController @@ -13,12 +12,18 @@ class Projects::LabelsController &lt; Projects::ApplicationController
13 def generate 12 def generate
14 Gitlab::IssuesLabels.generate(@project) 13 Gitlab::IssuesLabels.generate(@project)
15 14
16 - redirect_to project_issues_path(@project) 15 + if params[:redirect] == 'issues'
  16 + redirect_to project_issues_path(@project)
  17 + elsif params[:redirect] == 'merge_requests'
  18 + redirect_to project_merge_requests_path(@project)
  19 + end
17 end 20 end
18 21
19 protected 22 protected
20 23
21 def module_enabled 24 def module_enabled
22 - return render_404 unless @project.issues_enabled 25 + unless @project.issues_enabled || @project.merge_requests_enabled
  26 + return render_404
  27 + end
23 end 28 end
24 end 29 end
app/models/merge_request.rb
  Diff comments   View file @5f25cdf
@@ -36,7 +36,9 @@ class MergeRequest &lt; ActiveRecord::Base @@ -36,7 +36,9 @@ class MergeRequest &lt; ActiveRecord::Base
36 36
37 delegate :commits, :diffs, :last_commit, :last_commit_short_sha, to: :merge_request_diff, prefix: nil 37 delegate :commits, :diffs, :last_commit, :last_commit_short_sha, to: :merge_request_diff, prefix: nil
38 38
39 - attr_accessible :title, :assignee_id, :source_project_id, :source_branch, :target_project_id, :target_branch, :milestone_id, :state_event, :description 39 + attr_accessible :title, :assignee_id, :source_project_id, :source_branch,
  40 + :target_project_id, :target_branch, :milestone_id,
  41 + :state_event, :description, :label_list
40 42
41 attr_accessor :should_remove_source_branch 43 attr_accessor :should_remove_source_branch
42 44
@@ -44,6 +46,9 @@ class MergeRequest &lt; ActiveRecord::Base @@ -44,6 +46,9 @@ class MergeRequest &lt; ActiveRecord::Base
44 # It allows us to close or modify broken merge requests 46 # It allows us to close or modify broken merge requests
45 attr_accessor :allow_broken 47 attr_accessor :allow_broken
46 48
  49 + ActsAsTaggableOn.strict_case_match = true
  50 + acts_as_taggable_on :labels
  51 +
47 state_machine :state, initial: :opened do 52 state_machine :state, initial: :opened do
48 event :close do 53 event :close do
49 transition [:reopened, :opened] => :closed 54 transition [:reopened, :opened] => :closed
app/models/project.rb
  Diff comments   View file @5f25cdf
@@ -281,8 +281,11 @@ class Project &lt; ActiveRecord::Base @@ -281,8 +281,11 @@ class Project &lt; ActiveRecord::Base
281 self.id 281 self.id
282 end 282 end
283 283
  284 + # Tags are shared by issues and merge requests
284 def issues_labels 285 def issues_labels
285 - @issues_labels ||= (issues_default_labels + issues.tags_on(:labels)).uniq.sort_by(&:name) 286 + @issues_labels ||= (issues_default_labels +
  287 + merge_requests.tags_on(:labels) +
  288 + issues.tags_on(:labels)).uniq.sort_by(&:name)
286 end 289 end
287 290
288 def issue_exists?(issue_id) 291 def issue_exists?(issue_id)
app/views/projects/issues/index.html.haml
  Diff comments   View file @5f25cdf
1 = render "head" 1 = render "head"
2 .row 2 .row
3 .col-md-3 3 .col-md-3
4 - = render 'shared/project_filter', project_entities_path: project_issues_path(@project), labels: true 4 + = render 'shared/project_filter', project_entities_path: project_issues_path(@project),
  5 + labels: true, redirect: 'issues'
5 .col-md-9.issues-holder 6 .col-md-9.issues-holder
6 = render "issues" 7 = render "issues"
app/views/projects/merge_requests/_form.html.haml
  Diff comments   View file @5f25cdf
@@ -52,6 +52,15 @@ @@ -52,6 +52,15 @@
52 = f.text_area :description, class: "form-control js-gfm-input", rows: 14 52 = f.text_area :description, class: "form-control js-gfm-input", rows: 14
53 %p.hint Description is parsed with #{link_to "GitLab Flavored Markdown", help_markdown_path, target: '_blank'}. 53 %p.hint Description is parsed with #{link_to "GitLab Flavored Markdown", help_markdown_path, target: '_blank'}.
54 54
  55 + - if @merge_request.persisted? # Only allow labels on edit to avoid fork vs upstream repo labels issue
  56 + .form-group
  57 + = f.label :label_list, class: 'control-label' do
  58 + %i.icon-tag
  59 + Labels
  60 + .col-sm-10
  61 + = f.text_field :label_list, maxlength: 2000, class: "form-control"
  62 + %p.hint Separate labels with commas.
  63 +
55 .form-actions 64 .form-actions
56 - if @merge_request.new_record? 65 - if @merge_request.new_record?
57 = f.submit 'Submit merge request', class: "btn btn-create" 66 = f.submit 'Submit merge request', class: "btn btn-create"
@@ -83,3 +92,32 @@ @@ -83,3 +92,32 @@
83 target_branch.on("change", function() { 92 target_branch.on("change", function() {
84 $.get("#{branch_to_project_merge_requests_path(@source_project)}", {target_project_id: target_project.val(),ref: $(this).val() }); 93 $.get("#{branch_to_project_merge_requests_path(@source_project)}", {target_project_id: target_project.val(),ref: $(this).val() });
85 }); 94 });
  95 +
  96 + $("#merge_request_label_list")
  97 + .bind( "keydown", function( event ) {
  98 + if ( event.keyCode === $.ui.keyCode.TAB &&
  99 + $( this ).data( "autocomplete" ).menu.active ) {
  100 + event.preventDefault();
  101 + }
  102 + })
  103 + .bind("click", function(event) {
  104 + $(this).autocomplete("search", "");
  105 + })
  106 + .autocomplete({
  107 + minLength: 0,
  108 + source: function( request, response ) {
  109 + response( $.ui.autocomplete.filter(
  110 + #{raw labels_autocomplete_source}, extractLast( request.term ) ) );
  111 + },
  112 + focus: function() {
  113 + return false;
  114 + },
  115 + select: function(event, ui) {
  116 + var terms = split( this.value );
  117 + terms.pop();
  118 + terms.push( ui.item.value );
  119 + terms.push( "" );
  120 + this.value = terms.join( ", " );
  121 + return false;
  122 + }
  123 + });
app/views/projects/merge_requests/_merge_request.html.haml
  Diff comments   View file @5f25cdf
@@ -35,3 +35,9 @@ @@ -35,3 +35,9 @@
35 35
36 .pull-right 36 .pull-right
37 %small updated #{time_ago_with_tooltip(merge_request.updated_at, 'bottom', 'merge_request_updated_ago')} 37 %small updated #{time_ago_with_tooltip(merge_request.updated_at, 'bottom', 'merge_request_updated_ago')}
  38 +
  39 + .merge-request-labels
  40 + - merge_request.labels.each do |label|
  41 + %span{class: "label #{label_css_class(label.name)}"}
  42 + %i.icon-tag
  43 + = label.name
app/views/projects/merge_requests/_show.html.haml
  Diff comments   View file @5f25cdf
@@ -4,6 +4,7 @@ @@ -4,6 +4,7 @@
4 = render "projects/merge_requests/show/mr_box" 4 = render "projects/merge_requests/show/mr_box"
5 = render "projects/merge_requests/show/state_widget" 5 = render "projects/merge_requests/show/state_widget"
6 = render "projects/merge_requests/show/commits" 6 = render "projects/merge_requests/show/commits"
  7 + = render "projects/merge_requests/show/participants"
7 8
8 - if @commits.present? 9 - if @commits.present?
9 %ul.nav.nav-tabs 10 %ul.nav.nav-tabs
app/views/projects/merge_requests/index.html.haml
  Diff comments   View file @5f25cdf
@@ -8,7 +8,8 @@ @@ -8,7 +8,8 @@
8 %hr 8 %hr
9 .row 9 .row
10 .col-md-3 10 .col-md-3
11 - = render 'shared/project_filter', project_entities_path: project_merge_requests_path(@project) 11 + = render 'shared/project_filter', project_entities_path: project_merge_requests_path(@project),
  12 + labels: true, redirect: 'merge_requests'
12 .col-md-9 13 .col-md-9
13 .mr-filters.append-bottom-10 14 .mr-filters.append-bottom-10
14 .dropdown.inline 15 .dropdown.inline
app/views/projects/merge_requests/show/_participants.html.haml 0 → 100644
  Diff comments   View file @5f25cdf
@@ -0,0 +1,11 @@ @@ -0,0 +1,11 @@
  1 +.participants
  2 + %cite.cgray #{@merge_request.participants.count} participants
  3 + - @merge_request.participants.each do |participant|
  4 + = link_to_member(@project, participant, name: false, size: 24)
  5 +
  6 + .merge-request-show-labels.pull-right
  7 + - @merge_request.labels.each do |label|
  8 + %span{class: "label #{label_css_class(label.name)}"}
  9 + %i.icon-tag
  10 + = label.name
  11 + &nbsp;
app/views/shared/_project_filter.html.haml
  Diff comments   View file @5f25cdf
@@ -44,7 +44,7 @@ @@ -44,7 +44,7 @@
44 .light-well 44 .light-well
45 Add first label to your issues 45 Add first label to your issues
46 %br 46 %br
47 - or #{link_to 'generate', generate_project_labels_path(@project), method: :post} default set of labels 47 + or #{link_to 'generate', generate_project_labels_path(@project, redirect: redirect), method: :post} default set of labels
48 48
49 %fieldset 49 %fieldset
50 - if %w(state scope milestone_id assignee_id label_name).select { |k| params[k].present? }.any? 50 - if %w(state scope milestone_id assignee_id label_name).select { |k| params[k].present? }.any?
lib/api/entities.rb
  Diff comments   View file @5f25cdf
@@ -135,6 +135,7 @@ module API @@ -135,6 +135,7 @@ module API
135 expose :target_branch, :source_branch, :upvotes, :downvotes 135 expose :target_branch, :source_branch, :upvotes, :downvotes
136 expose :author, :assignee, using: Entities::UserBasic 136 expose :author, :assignee, using: Entities::UserBasic
137 expose :source_project_id, :target_project_id 137 expose :source_project_id, :target_project_id
  138 + expose :label_list, as: :labels
138 end 139 end
139 140
140 class SSHKey < Grape::Entity 141 class SSHKey < Grape::Entity
lib/api/merge_requests.rb
  Diff comments   View file @5f25cdf
@@ -67,6 +67,7 @@ module API @@ -67,6 +67,7 @@ module API
67 # assignee_id - Assignee user ID 67 # assignee_id - Assignee user ID
68 # title (required) - Title of MR 68 # title (required) - Title of MR
69 # description - Description of MR 69 # description - Description of MR
  70 + # labels (optional) - Labels for MR as a comma-separated list
70 # 71 #
71 # Example: 72 # Example:
72 # POST /projects/:id/merge_requests 73 # POST /projects/:id/merge_requests
@@ -75,6 +76,7 @@ module API @@ -75,6 +76,7 @@ module API
75 authorize! :write_merge_request, user_project 76 authorize! :write_merge_request, user_project
76 required_attributes! [:source_branch, :target_branch, :title] 77 required_attributes! [:source_branch, :target_branch, :title]
77 attrs = attributes_for_keys [:source_branch, :target_branch, :assignee_id, :title, :target_project_id, :description] 78 attrs = attributes_for_keys [:source_branch, :target_branch, :assignee_id, :title, :target_project_id, :description]
  79 + attrs[:label_list] = params[:labels] if params[:labels].present?
78 merge_request = ::MergeRequests::CreateService.new(user_project, current_user, attrs).execute 80 merge_request = ::MergeRequests::CreateService.new(user_project, current_user, attrs).execute
79 81
80 if merge_request.valid? 82 if merge_request.valid?
@@ -95,11 +97,13 @@ module API @@ -95,11 +97,13 @@ module API
95 # title - Title of MR 97 # title - Title of MR
96 # state_event - Status of MR. (close|reopen|merge) 98 # state_event - Status of MR. (close|reopen|merge)
97 # description - Description of MR 99 # description - Description of MR
  100 + # labels (optional) - Labels for a MR as a comma-separated list
98 # Example: 101 # Example:
99 # PUT /projects/:id/merge_request/:merge_request_id 102 # PUT /projects/:id/merge_request/:merge_request_id
100 # 103 #
101 put ":id/merge_request/:merge_request_id" do 104 put ":id/merge_request/:merge_request_id" do
102 attrs = attributes_for_keys [:source_branch, :target_branch, :assignee_id, :title, :state_event, :description] 105 attrs = attributes_for_keys [:source_branch, :target_branch, :assignee_id, :title, :state_event, :description]
  106 + attrs[:label_list] = params[:labels] if params[:labels].present?
103 merge_request = user_project.merge_requests.find(params[:merge_request_id]) 107 merge_request = user_project.merge_requests.find(params[:merge_request_id])
104 authorize! :modify_merge_request, merge_request 108 authorize! :modify_merge_request, merge_request
105 merge_request = ::MergeRequests::UpdateService.new(user_project, current_user, attrs).execute(merge_request) 109 merge_request = ::MergeRequests::UpdateService.new(user_project, current_user, attrs).execute(merge_request)
spec/requests/api/projects_spec.rb
  Diff comments   View file @5f25cdf
@@ -14,6 +14,12 @@ describe API::API, api: true do @@ -14,6 +14,12 @@ describe API::API, api: true do
14 let(:users_project) { create(:users_project, user: user, project: project, project_access: UsersProject::MASTER) } 14 let(:users_project) { create(:users_project, user: user, project: project, project_access: UsersProject::MASTER) }
15 let(:users_project2) { create(:users_project, user: user3, project: project, project_access: UsersProject::DEVELOPER) } 15 let(:users_project2) { create(:users_project, user: user3, project: project, project_access: UsersProject::DEVELOPER) }
16 let(:issue_with_labels) { create(:issue, author: user, assignee: user, project: project, :label_list => "label1, label2") } 16 let(:issue_with_labels) { create(:issue, author: user, assignee: user, project: project, :label_list => "label1, label2") }
  17 + let(:merge_request_with_labels) do
  18 + create(:merge_request, :simple, author: user, assignee: user,
  19 + source_project: project, target_project: project, title: 'Test',
  20 + label_list: 'label3, label4')
  21 + end
  22 +
17 23
18 describe "GET /projects" do 24 describe "GET /projects" do
19 before { project } 25 before { project }
@@ -634,15 +640,45 @@ describe API::API, api: true do @@ -634,15 +640,45 @@ describe API::API, api: true do
634 end 640 end
635 end 641 end
636 642
637 - describe "GET /projects/:id/labels" do  
638 - before { issue_with_labels } 643 + describe 'GET /projects/:id/labels' do
  644 + context 'with an issue' do
  645 + before { issue_with_labels }
639 646
640 - it "should return project labels" do  
641 - get api("/projects/#{project.id}/labels", user)  
642 - response.status.should == 200  
643 - json_response.should be_an Array  
644 - json_response.first['name'].should == issue_with_labels.labels.first.name  
645 - json_response.last['name'].should == issue_with_labels.labels.last.name 647 + it 'should return project labels' do
  648 + get api("/projects/#{project.id}/labels", user)
  649 + response.status.should == 200
  650 + json_response.should be_an Array
  651 + json_response.first['name'].should == issue_with_labels.labels.first.name
  652 + json_response.last['name'].should == issue_with_labels.labels.last.name
  653 + end
  654 + end
  655 +
  656 + context 'with a merge request' do
  657 + before { merge_request_with_labels }
  658 +
  659 + it 'should return project labels' do
  660 + get api("/projects/#{project.id}/labels", user)
  661 + response.status.should == 200
  662 + json_response.should be_an Array
  663 + json_response.first['name'].should == merge_request_with_labels.labels.first.name
  664 + json_response.last['name'].should == merge_request_with_labels.labels.last.name
  665 + end
  666 + end
  667 +
  668 + context 'with an issue and a merge request' do
  669 + before do
  670 + issue_with_labels
  671 + merge_request_with_labels
  672 + end
  673 +
  674 + it 'should return project labels from both' do
  675 + get api("/projects/#{project.id}/labels", user)
  676 + response.status.should == 200
  677 + json_response.should be_an Array
  678 + all_labels = issue_with_labels.labels.map(&:name).to_a
  679 + .concat(merge_request_with_labels.labels.map(&:name).to_a)
  680 + json_response.map { |e| e['name'] }.should =~ all_labels
  681 + end
646 end 682 end
647 end 683 end
648 end 684 end