Commit 0ef055a4aef76d20668a40717fb33bfe41599cfb

Authored by Izaak Alpert
1 parent 09112dbb

MR on fork: Email fixes, style fixes

-Removed many extra spaces I added
-Fixed email templates to be consistent/better looking

GITLAB-894, GITLAB-895, (GITLAB-858)

Change-Id: I35c1a8e0e22af7de26d54e5c3da987fa7bb3921e
app/models/merge_request.rb
... ... @@ -194,7 +194,6 @@ class MergeRequest < ActiveRecord::Base
194 194 commits
195 195 end
196 196  
197   -
198 197 def merge!(user_id)
199 198 self.author_id_of_changes = user_id
200 199 self.merge
... ... @@ -222,7 +221,6 @@ class MergeRequest < ActiveRecord::Base
222 221 Gitlab::Satellite::MergeAction.new(current_user, self).diff_in_satellite
223 222 end
224 223  
225   -
226 224 # Returns the commit as a series of email patches.
227 225 #
228 226 # see "git format-patch"
... ...
app/models/note.rb
... ... @@ -52,11 +52,11 @@ class Note < ActiveRecord::Base
52 52  
53 53 def self.create_status_change_note(noteable, project, author, status)
54 54 create({
55   - noteable: noteable,
56   - project: project,
57   - author: author,
58   - note: "_Status changed to #{status}_"
59   - }, without_protection: true)
  55 + noteable: noteable,
  56 + project: project,
  57 + author: author,
  58 + note: "_Status changed to #{status}_"
  59 + }, without_protection: true)
60 60 end
61 61  
62 62 def commit_author
... ... @@ -97,7 +97,7 @@ class Note < ActiveRecord::Base
97 97 # otherwise false is returned
98 98 def downvote?
99 99 votable? && (note.start_with?('-1') ||
100   - note.start_with?(':-1:')
  100 + note.start_with?(':-1:')
101 101 )
102 102 end
103 103  
... ... @@ -136,8 +136,8 @@ class Note < ActiveRecord::Base
136 136 else
137 137 super
138 138 end
139   - # Temp fix to prevent app crash
140   - # if note commit id doesn't exist
  139 + # Temp fix to prevent app crash
  140 + # if note commit id doesn't exist
141 141 rescue
142 142 nil
143 143 end
... ... @@ -146,7 +146,7 @@ class Note < ActiveRecord::Base
146 146 # otherwise false is returned
147 147 def upvote?
148 148 votable? && (note.start_with?('+1') ||
149   - note.start_with?(':+1:')
  149 + note.start_with?(':+1:')
150 150 )
151 151 end
152 152  
... ...
app/views/merge_requests/update_branches.js.haml
1 1 :plain
2 2 $(".target_branch").html("#{escape_javascript(options_for_select(@target_branches))}");
3 3 $(".target_branch").trigger("liszt:updated");
4   - $(".mr_target_commit").html("");
5   -
6   -
7   -
8   -
  4 + $(".mr_target_commit").html("");
9 5 \ No newline at end of file
... ...
app/views/notify/closed_merge_request_email.text.haml
... ... @@ -2,7 +2,7 @@
2 2  
3 3 Merge Request url: #{project_merge_request_url(@merge_request.target_project, @merge_request)}
4 4  
5   -Project:Branches: #{@merge_request.source_project.path_with_namespace}/#{@merge_request.source_branch} - #{@merge_request.target_project.path_with_namespace}#{@merge_request.target_branch}
  5 +Project:Branches: #{@merge_request.source_project.path_with_namespace}:#{@merge_request.source_branch} to #{@merge_request.target_project.path_with_namespace}:#{@merge_request.target_branch}
6 6  
7 7 Author: #{@merge_request.author_name}
8 8 Assignee: #{@merge_request.assignee_name}
... ...
app/views/notify/merged_merge_request_email.text.haml
... ... @@ -2,7 +2,7 @@
2 2  
3 3 Merge Request Url: #{project_merge_request_url(@merge_request.target_project, @merge_request)}
4 4  
5   -Project:Branches: #{@merge_request.source_project.path_with_namespace}/#{@merge_request.source_branch} - #{@merge_request.target_project.path_with_namespace}#{@merge_request.target_branch}
  5 +Project:Branches: #{@merge_request.source_project.path_with_namespace}:#{@merge_request.source_branch} to #{@merge_request.target_project.path_with_namespace}:#{@merge_request.target_branch}
6 6  
7 7 Author: #{@merge_request.author_name}
8 8 Assignee: #{@merge_request.assignee_name}
... ...
app/views/notify/new_merge_request_email.html.haml
... ... @@ -3,7 +3,7 @@
3 3 %p
4 4 = link_to_gfm truncate(@merge_request.title, length: 40), project_merge_request_url(@merge_request.target_project, @merge_request)
5 5 %p
6   - Project:Branches: #{@merge_request.source_project.path_with_namespace}/#{@merge_request.source_branch} - #{@merge_request.target_project.path_with_namespace}#{@merge_request.target_branch}
  6 + Project:Branches: #{@merge_request.source_project.path_with_namespace}:#{@merge_request.source_branch} → #{@merge_request.target_project.path_with_namespace}:#{@merge_request.target_branch}
7 7 %p
8 8 Assignee: #{@merge_request.author_name} → #{@merge_request.assignee_name}
9 9  
... ...
app/views/projects/merge_requests/_form.html.haml
... ... @@ -71,12 +71,12 @@
71 71 $.get("#{branch_from_project_merge_requests_path(@source_project)}", {ref: source_branch.val() });
72 72 $.get("#{branch_to_project_merge_requests_path(@source_project)}", {target_project_id: target_project.val(),ref: target_branch.val() });
73 73  
74   - target_project.live("change", function() {
  74 + target_project.on("change", function() {
75 75 $.get("#{update_branches_project_merge_requests_path(@source_project)}", {target_project_id: $(this).val() });
76 76 });
77   - source_branch.live("change", function() {
  77 + source_branch.on("change", function() {
78 78 $.get("#{branch_from_project_merge_requests_path(@source_project)}", {ref: $(this).val() });
79 79 });
80   - target_branch.live("change", function() {
  80 + target_branch.on("change", function() {
81 81 $.get("#{branch_to_project_merge_requests_path(@source_project)}", {target_project_id: target_project.val(),ref: $(this).val() });
82   - });
83 82 \ No newline at end of file
  83 + });
... ...
features/project/forked_merge_requests.feature
... ... @@ -4,7 +4,6 @@ Feature: Project Forked Merge Requests
4 4 And I am a member of project "Shop"
5 5 And I have a project forked off of "Shop" called "Forked Shop"
6 6  
7   -
8 7 @javascript
9 8 Scenario: I can visit the target projects commit for a forked merge request
10 9 Given I visit project "Forked Shop" merge requests page
... ...
features/steps/project/deploy_keys.rb
... ... @@ -3,13 +3,10 @@ class Spinach::Features::ProjectDeployKeys < Spinach::FeatureSteps
3 3 include SharedProject
4 4 include SharedPaths
5 5  
6   -
7   -
8 6 step 'project has deploy key' do
9 7 create(:deploy_keys_project, project: @project)
10 8 end
11 9  
12   -
13 10 step 'I should see project deploy keys' do
14 11 within '.enabled-keys' do
15 12 page.should have_content deploy_key.title
... ...
features/steps/project/project_fork.rb
... ... @@ -16,7 +16,6 @@ class ForkProject < Spinach::FeatureSteps
16 16 @project.team << [@user, :reporter]
17 17 end
18 18  
19   -
20 19 step 'I should see the forked project page' do
21 20 page.should have_content "Project was successfully forked."
22 21 current_path.should include current_user.namespace.path
... ...
features/steps/project/project_forked_merge_requests.rb
... ... @@ -4,8 +4,6 @@ class ProjectForkedMergeRequests &lt; Spinach::FeatureSteps
4 4 include SharedNote
5 5 include SharedPaths
6 6  
7   -
8   -
9 7 Given 'I am a member of project "Shop"' do
10 8 @project = Project.find_by_name "Shop"
11 9 @project ||= create(:project_with_code, name: "Shop")
... ... @@ -22,12 +20,10 @@ class ProjectForkedMergeRequests &lt; Spinach::FeatureSteps
22 20 forked_project_link.save!
23 21 end
24 22  
25   -
26 23 Given 'I click link "New Merge Request"' do
27 24 click_link "New Merge Request"
28 25 end
29 26  
30   -
31 27 Then 'I should see merge request "Merge Request On Forked Project"' do
32 28 page.should have_content "Merge Request On Forked Project"
33 29 @project.merge_requests.size.should >= 1
... ... @@ -151,7 +147,6 @@ class ProjectForkedMergeRequests &lt; Spinach::FeatureSteps
151 147 verify_commit_link(".mr_target_commit",@project)
152 148 end
153 149  
154   -
155 150 And 'I fill out an invalid "Merge Request On Forked Project" merge request' do
156 151 #If this isn't filled in the rest of the validations won't be triggered
157 152 fill_in "merge_request_title", with: "Merge Request On Forked Project"
... ... @@ -161,14 +156,12 @@ class ProjectForkedMergeRequests &lt; Spinach::FeatureSteps
161 156 find(:select, "merge_request_target_branch", {}).value.should == ""
162 157 end
163 158  
164   -
165 159 Then 'I should see validation errors' do
166 160 page.should have_content "Source branch can't be blank"
167 161 page.should have_content "Target branch can't be blank"
168 162 page.should have_content "Branch conflict You can not use same project/branch for source and target"
169 163 end
170 164  
171   -
172 165 def project
173 166 @project ||= Project.find_by_name!("Shop")
174 167 end
... ... @@ -179,5 +172,4 @@ class ProjectForkedMergeRequests &lt; Spinach::FeatureSteps
179 172 find(:div,container_div).should have_css ".browse_code_link_holder"
180 173 find(:div,container_div).find(".commit_short_id")['href'].should have_content "#{container_project.path_with_namespace}/commit"
181 174 end
182   -
183 175 end
... ...
features/steps/project/project_merge_requests.rb
... ... @@ -24,7 +24,6 @@ class ProjectMergeRequests &lt; Spinach::FeatureSteps
24 24 page.should have_content "Wiki Feature"
25 25 end
26 26  
27   -
28 27 Then 'I should see closed merge request "Bug NS-04"' do
29 28 merge_request = MergeRequest.find_by_title!("Bug NS-04")
30 29 merge_request.closed?.should be_true
... ...
lib/gitlab/satellite/merge_action.rb
... ... @@ -44,7 +44,6 @@ module Gitlab
44 44 handle_exception(ex)
45 45 end
46 46  
47   -
48 47 # Get a raw diff of the source to the target
49 48 def diff_in_satellite
50 49 in_locked_and_timed_satellite do |merge_repo|
... ... @@ -114,14 +113,12 @@ module Gitlab
114 113 end
115 114 commits = commits.map { |commit| Gitlab::Git::Commit.new(commit, nil) }
116 115 return commits
117   -
118 116 end
119 117 rescue Grit::Git::CommandFailed => ex
120 118 handle_exception(ex)
121 119 end
122 120  
123 121 private
124   -
125 122 # Merges the source_branch into the target_branch in the satellite.
126 123 #
127 124 # Note: it will clear out the satellite before doing anything
... ...
spec/factories.rb
... ... @@ -71,7 +71,6 @@ FactoryGirl.define do
71 71 end
72 72 end
73 73  
74   -
75 74 factory :group do
76 75 sequence(:name) { |n| "group#{n}" }
77 76 path { name.downcase.gsub(/\s/, '_') }
... ...
spec/factories_spec.rb
... ... @@ -5,7 +5,6 @@ INVALID_FACTORIES = [
5 5 :invalid_key,
6 6 ]
7 7  
8   -
9 8 FactoryGirl.factories.map(&:name).each do |factory_name|
10 9 next if INVALID_FACTORIES.include?(factory_name)
11 10  
... ...