From 79fb993a6598df4836e5c0ed4e27a72e844429fc Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Sat, 14 Nov 2015 20:19:38 +0100 Subject: Enable rubocop metrics This enables rubocop metrics like CyclomaticComplexity and ABCSize. Initial threshold values are high, should be probably decreased. --- .rubocop.yml | 43 ++++++++++++++++++++++++------------------- 1 file changed, 24 insertions(+), 19 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index d59edbc8b17..fd382182abe 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -735,23 +735,39 @@ Metrics/AbcSize: Description: >- A calculated magnitude based on number of assignments, branches, and conditions. - Enabled: false + Enabled: true + Max: 70 + +Metrics/CyclomaticComplexity: + Description: >- + A complexity metric that is strongly correlated to the number + of test cases needed to validate a method. + Enabled: true + Max: 16 + +Metrics/PerceivedComplexity: + Description: >- + A complexity metric geared towards measuring complexity for a + human reader. + Enabled: true + Max: 16 + +Metrics/ParameterLists: + Description: 'Avoid parameter lists longer than three or four parameters.' + StyleGuide: 'https://github.com/bbatsov/ruby-style-guide#too-many-params' + Enabled: true + Max: 8 Metrics/BlockNesting: Description: 'Avoid excessive block nesting' StyleGuide: 'https://github.com/bbatsov/ruby-style-guide#three-is-the-number-thou-shalt-count' - Enabled: false + Enabled: true + Max: 4 Metrics/ClassLength: Description: 'Avoid classes longer than 100 lines of code.' Enabled: false -Metrics/CyclomaticComplexity: - Description: >- - A complexity metric that is strongly correlated to the number - of test cases needed to validate a method. - Enabled: false - Metrics/LineLength: Description: 'Limit lines to 80 characters.' StyleGuide: 'https://github.com/bbatsov/ruby-style-guide#80-character-limits' @@ -762,17 +778,6 @@ Metrics/MethodLength: StyleGuide: 'https://github.com/bbatsov/ruby-style-guide#short-methods' Enabled: false -Metrics/ParameterLists: - Description: 'Avoid parameter lists longer than three or four parameters.' - StyleGuide: 'https://github.com/bbatsov/ruby-style-guide#too-many-params' - Enabled: false - -Metrics/PerceivedComplexity: - Description: >- - A complexity metric geared towards measuring complexity for a - human reader. - Enabled: false - #################### Lint ################################ ### Warnings -- cgit v1.2.1 From 8c6db54e1283348f64b46733875db7ffe08993a6 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 17 Nov 2015 13:08:42 +0100 Subject: Extract repository_push_email to separate class --- app/mailers/emails/projects.rb | 96 ++++++----------------------- lib/gitlab/email/repository_push.rb | 116 ++++++++++++++++++++++++++++++++++++ 2 files changed, 135 insertions(+), 77 deletions(-) create mode 100644 lib/gitlab/email/repository_push.rb diff --git a/app/mailers/emails/projects.rb b/app/mailers/emails/projects.rb index caba63006da..92bca4e6181 100644 --- a/app/mailers/emails/projects.rb +++ b/app/mailers/emails/projects.rb @@ -59,85 +59,27 @@ module Emails subject: subject("Project was moved")) end - def repository_push_email(project_id, recipient, author_id: nil, - ref: nil, - action: nil, - compare: nil, - reverse_compare: false, - send_from_committer_email: false, - disable_diffs: false) - unless author_id && ref && action - raise ArgumentError, "missing keywords: author_id, ref, action" - end - - @project = Project.find(project_id) - @current_user = @author = User.find(author_id) - @reverse_compare = reverse_compare - @compare = compare - @ref_name = Gitlab::Git.ref_name(ref) - @ref_type = Gitlab::Git.tag_ref?(ref) ? "tag" : "branch" - @action = action - @disable_diffs = disable_diffs - - if @compare - @commits = Commit.decorate(compare.commits, @project) - @diffs = compare.diffs - end - - @action_name = - case action - when :create - "pushed new" - when :delete - "deleted" - else - "pushed to" - end - - @subject = "[Git]" - @subject << "[#{@project.path_with_namespace}]" - @subject << "[#{@ref_name}]" if action == :push - @subject << " " - - if action == :push - if @commits.length > 1 - @target_url = namespace_project_compare_url(@project.namespace, - @project, - from: Commit.new(@compare.base, @project), - to: Commit.new(@compare.head, @project)) - @subject << "Deleted " if @reverse_compare - @subject << "#{@commits.length} commits: #{@commits.first.title}" - else - @target_url = namespace_project_commit_url(@project.namespace, - @project, @commits.first) - - @subject << "Deleted 1 commit: " if @reverse_compare - @subject << @commits.first.title - end - else - unless action == :delete - @target_url = namespace_project_tree_url(@project.namespace, - @project, @ref_name) - end - - subject_action = @action_name.dup - subject_action[0] = subject_action[0].capitalize - @subject << "#{subject_action} #{@ref_type} #{@ref_name}" - end - + def repository_push_email(project_id, recipient, opts = {}) + email = Gitlab::Email::RepositoryPush.new(project_id, recipient, opts) + + @project = email.project + @current_user = @author = email.author + @reverse_compare = email.reverse_compare + @compare = email.compare + @ref_name = email.ref_name + @ref_type = email.ref_type + @action = email.action + @disable_diffs = email.disable_diffs + @commits = email.commits + @diffs = email.diffs + @action_name = email.action_name + @target_url = email.target_url @disable_footer = true - reply_to = - if send_from_committer_email && can_send_from_user_email?(@author) - @author.email - else - Gitlab.config.gitlab.email_reply_to - end - - mail(from: sender(author_id, send_from_committer_email), - reply_to: reply_to, - to: recipient, - subject: @subject) + mail(from: sender(email.author_id, email.send_from_committer_email), + reply_to: email.reply_to, + to: email.recipient, + subject: email.subject) end end end diff --git a/lib/gitlab/email/repository_push.rb b/lib/gitlab/email/repository_push.rb new file mode 100644 index 00000000000..f484f3cb76d --- /dev/null +++ b/lib/gitlab/email/repository_push.rb @@ -0,0 +1,116 @@ +module Gitlab + module Email + class RepositoryPush + attr_reader :compare, :reverse_compare, :send_from_cmmitter_email, :disable_diffs, + :action, :ref, :author_id + + def initialize(project_id, recipient, opts = {}) + raise ArgumentError, 'Missing arguments: author_id, ref, action' unless + opts[:author_id] && opts[:ref] && opts[:action] + + @project_id = project_id + @recipient = recipient + + @author_id = opts[:author_id] + @ref = opts[:ref] + @action = opts[:action] + + @compare = opts[:compare] || nil + @reverse_compare = opts[:reverse_compare] || false + @send_from_committer_email = opts[:send_from_committer_email] || false + @disable_diffs = opts[:disable_diffs] || false + + @author = author + @project = project + @commits = commits + @diffs = diffs + @ref_name = ref_name + @ref_type = ref_type + @action_name = action_name + end + + def project + Project.find(@project_id) + end + + def author + User.find(@author_id) + end + + def commits + Commit.decorate(@compare.commits, @project) if @compare + end + + def diffs + @compare.diffs if @compare + end + + def action_name + case @action + when :create + 'pushed new' + when :delete + 'deleted' + else + 'pushed to' + end + end + + def subject + subject_text = '[Git]' + subject_text << "[#{@project.path_with_namespace}]" + subject_text << "[#{@ref_name}]" if @action == :push + subject_text << ' ' + + if @action == :push + if @commits.length > 1 + subject_text << "Deleted " if @reverse_compare + subject_text << "#{@commits.length} commits: #{@commits.first.title}" + else + subject_text << "Deleted 1 commit: " if @reverse_compare + subject_text << @commits.first.title + end + end + + subject_action = @action_name.dup + subject_action[0] = subject_action[0].capitalize + subject_text << "#{subject_action} #{@ref_type} #{@ref_name}" + end + + def ref_name + Gitlab::Git.ref_name(@ref) + end + + def ref_type + Gitlab::Git.tag_ref?(@ref) ? 'tag' : 'branch' + end + + def target_url + if action == :push + if @commits.length > 1 + namespace_project_compare_url(@project.namespace, + @project, + from: Commit.new(@compare.base, @project), + to: Commit.new(@compare.head, @project)) + else + namespace_project_commit_url(@project.namespace, + @project, @commits.first) + end + end + + if action != :delete && action != :push + namespace_project_tree_url(@project.namespace, + @project, @ref_name) + end + end + + def reply_to + if @send_from_committer_email && can_send_from_user_email?(@author) + @author.email + else + Gitlab.config.gitlab.email_reply_to + end + end + end + end +end -- cgit v1.2.1 From 45f7f01f19a6c5f977d71b094cbe5fedb44dc9e2 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Wed, 18 Nov 2015 10:12:09 +0100 Subject: Make `can_send_from_user_email?` public in Notify --- app/mailers/notify.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/mailers/notify.rb b/app/mailers/notify.rb index 50a409c3754..0534eb025cd 100644 --- a/app/mailers/notify.rb +++ b/app/mailers/notify.rb @@ -33,13 +33,13 @@ class Notify < BaseMailer allowed_domains end - private - def can_send_from_user_email?(sender) sender_domain = sender.email.split("@").last self.class.allowed_email_domains.include?(sender_domain) end + private + # Return an email address that displays the name of the sender. # Only the displayed name changes; the actual email address is always the same. def sender(sender_id, send_from_user_email = false) -- cgit v1.2.1 From e2f937ce22e6b0eb458bbdb3fa93b06d80ecfd21 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Wed, 18 Nov 2015 10:13:34 +0100 Subject: Refactor RepositoryPush, move to Message namespace --- app/mailers/emails/projects.rb | 38 ++++----- lib/gitlab/email/message/repository_push.rb | 120 ++++++++++++++++++++++++++++ lib/gitlab/email/repository_push.rb | 116 --------------------------- 3 files changed, 140 insertions(+), 134 deletions(-) create mode 100644 lib/gitlab/email/message/repository_push.rb delete mode 100644 lib/gitlab/email/repository_push.rb diff --git a/app/mailers/emails/projects.rb b/app/mailers/emails/projects.rb index 92bca4e6181..c303e5a1a9c 100644 --- a/app/mailers/emails/projects.rb +++ b/app/mailers/emails/projects.rb @@ -60,26 +60,28 @@ module Emails end def repository_push_email(project_id, recipient, opts = {}) - email = Gitlab::Email::RepositoryPush.new(project_id, recipient, opts) - - @project = email.project - @current_user = @author = email.author - @reverse_compare = email.reverse_compare - @compare = email.compare - @ref_name = email.ref_name - @ref_type = email.ref_type - @action = email.action - @disable_diffs = email.disable_diffs - @commits = email.commits - @diffs = email.diffs - @action_name = email.action_name - @target_url = email.target_url + repository_push = + Gitlab::Email::Message::RepositoryPush.new(self, project_id, recipient, opts) + + @project = repository_push.project + @current_user = @author = repository_push.author + @reverse_compare = repository_push.reverse_compare + @compare = repository_push.compare + @ref_name = repository_push.ref_name + @ref_type = repository_push.ref_type + @action = repository_push.action + @action_name = repository_push.action_name + @disable_diffs = repository_push.disable_diffs + @commits = repository_push.commits + @diffs = repository_push.diffs + @target_url = repository_push.target_url @disable_footer = true - mail(from: sender(email.author_id, email.send_from_committer_email), - reply_to: email.reply_to, - to: email.recipient, - subject: email.subject) + mail(from: sender(repository_push.author_id, + repository_push.send_from_committer_email), + reply_to: repository_push.reply_to, + to: repository_push.recipient, + subject: repository_push.subject) end end end diff --git a/lib/gitlab/email/message/repository_push.rb b/lib/gitlab/email/message/repository_push.rb new file mode 100644 index 00000000000..bb92df9e1bb --- /dev/null +++ b/lib/gitlab/email/message/repository_push.rb @@ -0,0 +1,120 @@ +module Gitlab + module Email + module Message + class RepositoryPush + attr_reader :compare, :reverse_compare, :send_from_committer_email, + :disable_diffs, :action, :ref, :author_id + attr_accessor :recipient + + def initialize(notify, project_id, recipient, opts = {}) + raise ArgumentError, 'Missing arguments: author_id, ref, action' unless + opts[:author_id] && opts[:ref] && opts[:action] + + @notify = notify + @project_id = project_id + @recipient = recipient + + @author_id = opts[:author_id] + @ref = opts[:ref] + @action = opts[:action] + + @compare = opts[:compare] || nil + @reverse_compare = opts[:reverse_compare] || false + @send_from_committer_email = opts[:send_from_committer_email] || false + @disable_diffs = opts[:disable_diffs] || false + + @author = author + @project = project + @commits = commits + @diffs = diffs + @ref_name = ref_name + @ref_type = ref_type + @action_name = action_name + end + + def project + Project.find(@project_id) + end + + def author + User.find(@author_id) + end + + def commits + Commit.decorate(@compare.commits, @project) if @compare + end + + def diffs + @compare.diffs if @compare + end + + def action_name + case @action + when :create + 'pushed new' + when :delete + 'deleted' + else + 'pushed to' + end + end + + def subject + subject_text = '[Git]' + subject_text << "[#{@project.path_with_namespace}]" + subject_text << "[#{@ref_name}]" if @action == :push + subject_text << ' ' + + if @action == :push + if @commits.length > 1 + subject_text << "Deleted " if @reverse_compare + subject_text << "#{@commits.length} commits: #{@commits.first.title}" + else + subject_text << "Deleted 1 commit: " if @reverse_compare + subject_text << @commits.first.title + end + else + subject_action = @action_name.dup + subject_action[0] = subject_action[0].capitalize + subject_text << "#{subject_action} #{@ref_type} #{@ref_name}" + end + end + + def ref_name + Gitlab::Git.ref_name(@ref) + end + + def ref_type + Gitlab::Git.tag_ref?(@ref) ? 'tag' : 'branch' + end + + def target_url + if @action == :push + if @commits.length > 1 + @notify.namespace_project_compare_url(@project.namespace, + @project, + from: Commit.new(@compare.base, @project), + to: Commit.new(@compare.head, @project)) + else + @notify.namespace_project_commit_url(@project.namespace, + @project, @commits.first) + end + else + unless @action == :delete + @notify.namespace_project_tree_url(@project.namespace, + @project, @ref_name) + end + end + end + + def reply_to + if @send_from_committer_email && @notify.can_send_from_user_email?(@author) + @author.email + else + Gitlab.config.gitlab.email_reply_to + end + end + end + end + end +end diff --git a/lib/gitlab/email/repository_push.rb b/lib/gitlab/email/repository_push.rb deleted file mode 100644 index f484f3cb76d..00000000000 --- a/lib/gitlab/email/repository_push.rb +++ /dev/null @@ -1,116 +0,0 @@ -module Gitlab - module Email - class RepositoryPush - attr_reader :compare, :reverse_compare, :send_from_cmmitter_email, :disable_diffs, - :action, :ref, :author_id - - def initialize(project_id, recipient, opts = {}) - raise ArgumentError, 'Missing arguments: author_id, ref, action' unless - opts[:author_id] && opts[:ref] && opts[:action] - - @project_id = project_id - @recipient = recipient - - @author_id = opts[:author_id] - @ref = opts[:ref] - @action = opts[:action] - - @compare = opts[:compare] || nil - @reverse_compare = opts[:reverse_compare] || false - @send_from_committer_email = opts[:send_from_committer_email] || false - @disable_diffs = opts[:disable_diffs] || false - - @author = author - @project = project - @commits = commits - @diffs = diffs - @ref_name = ref_name - @ref_type = ref_type - @action_name = action_name - end - - def project - Project.find(@project_id) - end - - def author - User.find(@author_id) - end - - def commits - Commit.decorate(@compare.commits, @project) if @compare - end - - def diffs - @compare.diffs if @compare - end - - def action_name - case @action - when :create - 'pushed new' - when :delete - 'deleted' - else - 'pushed to' - end - end - - def subject - subject_text = '[Git]' - subject_text << "[#{@project.path_with_namespace}]" - subject_text << "[#{@ref_name}]" if @action == :push - subject_text << ' ' - - if @action == :push - if @commits.length > 1 - subject_text << "Deleted " if @reverse_compare - subject_text << "#{@commits.length} commits: #{@commits.first.title}" - else - subject_text << "Deleted 1 commit: " if @reverse_compare - subject_text << @commits.first.title - end - end - - subject_action = @action_name.dup - subject_action[0] = subject_action[0].capitalize - subject_text << "#{subject_action} #{@ref_type} #{@ref_name}" - end - - def ref_name - Gitlab::Git.ref_name(@ref) - end - - def ref_type - Gitlab::Git.tag_ref?(@ref) ? 'tag' : 'branch' - end - - def target_url - if action == :push - if @commits.length > 1 - namespace_project_compare_url(@project.namespace, - @project, - from: Commit.new(@compare.base, @project), - to: Commit.new(@compare.head, @project)) - else - namespace_project_commit_url(@project.namespace, - @project, @commits.first) - end - end - - if action != :delete && action != :push - namespace_project_tree_url(@project.namespace, - @project, @ref_name) - end - end - - def reply_to - if @send_from_committer_email && can_send_from_user_email?(@author) - @author.email - else - Gitlab.config.gitlab.email_reply_to - end - end - end - end -end -- cgit v1.2.1 From 4beba7494b096f2540b19017bb7c1c8e91679135 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Fri, 20 Nov 2015 15:29:47 +0100 Subject: Improve Messagee::RepositoryPush --- app/mailers/emails/projects.rb | 6 +- lib/gitlab/email/message/repository_push.rb | 126 ++++++++++++++-------------- 2 files changed, 68 insertions(+), 64 deletions(-) diff --git a/app/mailers/emails/projects.rb b/app/mailers/emails/projects.rb index c303e5a1a9c..bedadb583c7 100644 --- a/app/mailers/emails/projects.rb +++ b/app/mailers/emails/projects.rb @@ -65,20 +65,20 @@ module Emails @project = repository_push.project @current_user = @author = repository_push.author - @reverse_compare = repository_push.reverse_compare @compare = repository_push.compare @ref_name = repository_push.ref_name @ref_type = repository_push.ref_type @action = repository_push.action @action_name = repository_push.action_name - @disable_diffs = repository_push.disable_diffs @commits = repository_push.commits @diffs = repository_push.diffs @target_url = repository_push.target_url + @disable_diffs = repository_push.disable_diffs? + @reverse_compare = repository_push.reverse_compare? @disable_footer = true mail(from: sender(repository_push.author_id, - repository_push.send_from_committer_email), + repository_push.send_from_committer_email?), reply_to: repository_push.reply_to, to: repository_push.recipient, subject: repository_push.subject) diff --git a/lib/gitlab/email/message/repository_push.rb b/lib/gitlab/email/message/repository_push.rb index bb92df9e1bb..61962abf822 100644 --- a/lib/gitlab/email/message/repository_push.rb +++ b/lib/gitlab/email/message/repository_push.rb @@ -2,118 +2,122 @@ module Gitlab module Email module Message class RepositoryPush - attr_reader :compare, :reverse_compare, :send_from_committer_email, - :disable_diffs, :action, :ref, :author_id attr_accessor :recipient + attr_reader :author_id, :ref, :action def initialize(notify, project_id, recipient, opts = {}) - raise ArgumentError, 'Missing arguments: author_id, ref, action' unless + raise ArgumentError, 'Missing options: author_id, ref, action' unless opts[:author_id] && opts[:ref] && opts[:action] @notify = notify @project_id = project_id @recipient = recipient + @opts = opts - @author_id = opts[:author_id] - @ref = opts[:ref] - @action = opts[:action] - - @compare = opts[:compare] || nil - @reverse_compare = opts[:reverse_compare] || false - @send_from_committer_email = opts[:send_from_committer_email] || false - @disable_diffs = opts[:disable_diffs] || false - - @author = author - @project = project - @commits = commits - @diffs = diffs - @ref_name = ref_name - @ref_type = ref_type - @action_name = action_name + @author_id = opts.delete(:author_id) + @ref = opts.delete(:ref) + @action = opts.delete(:action) end def project - Project.find(@project_id) + @project ||= Project.find(@project_id) end def author - User.find(@author_id) + @author ||= User.find(@author_id) end def commits - Commit.decorate(@compare.commits, @project) if @compare + @commits ||= (Commit.decorate(compare.commits, project) if compare) end def diffs - @compare.diffs if @compare + @diffs ||= (compare.diffs if compare) end - def action_name - case @action - when :create - 'pushed new' - when :delete - 'deleted' - else - 'pushed to' - end + def compare + @opts[:compare] end - def subject - subject_text = '[Git]' - subject_text << "[#{@project.path_with_namespace}]" - subject_text << "[#{@ref_name}]" if @action == :push - subject_text << ' ' + def reverse_compare? + @opts[:reverse_compare] || false + end - if @action == :push - if @commits.length > 1 - subject_text << "Deleted " if @reverse_compare - subject_text << "#{@commits.length} commits: #{@commits.first.title}" + def disable_diffs? + @opts[:disable_diffs] || false + end + + def send_from_committer_email? + @opts[:send_from_committer_email] || false + end + + def action_name + @action_name ||= + case @action + when :create + 'pushed new' + when :delete + 'deleted' else - subject_text << "Deleted 1 commit: " if @reverse_compare - subject_text << @commits.first.title + 'pushed to' end - else - subject_action = @action_name.dup - subject_action[0] = subject_action[0].capitalize - subject_text << "#{subject_action} #{@ref_type} #{@ref_name}" - end end def ref_name - Gitlab::Git.ref_name(@ref) + @ref_name ||= Gitlab::Git.ref_name(@ref) end def ref_type - Gitlab::Git.tag_ref?(@ref) ? 'tag' : 'branch' + @ref_type ||= Gitlab::Git.tag_ref?(@ref) ? 'tag' : 'branch' end def target_url if @action == :push - if @commits.length > 1 - @notify.namespace_project_compare_url(@project.namespace, - @project, - from: Commit.new(@compare.base, @project), - to: Commit.new(@compare.head, @project)) + if commits.length > 1 && compare + @notify.namespace_project_compare_url(project.namespace, + project, + from: Commit.new(compare.base, project), + to: Commit.new(compare.head, project)) else - @notify.namespace_project_commit_url(@project.namespace, - @project, @commits.first) + @notify.namespace_project_commit_url(project.namespace, + project, commits.first) end else unless @action == :delete - @notify.namespace_project_tree_url(@project.namespace, - @project, @ref_name) + @notify.namespace_project_tree_url(project.namespace, + project, ref_name) end end end def reply_to - if @send_from_committer_email && @notify.can_send_from_user_email?(@author) - @author.email + if send_from_committer_email? && @notify.can_send_from_user_email?(author) + author.email else Gitlab.config.gitlab.email_reply_to end end + + def subject + subject_text = '[Git]' + subject_text << "[#{project.path_with_namespace}]" + subject_text << "[#{ref_name}]" if @action == :push + subject_text << ' ' + + if @action == :push + if commits.length > 1 + subject_text << "Deleted " if reverse_compare? + subject_text << "#{commits.length} commits: #{commits.first.title}" + else + subject_text << "Deleted 1 commit: " if reverse_compare? + subject_text << commits.first.title + end + else + subject_action = action_name.dup + subject_action[0] = subject_action[0].capitalize + subject_text << "#{subject_action} #{ref_type} #{ref_name}" + end + end end end end -- cgit v1.2.1 From 9f2752e5dcc31dc4e9d91ee18caf1d36f1b7684e Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Sat, 21 Nov 2015 20:54:19 +0100 Subject: Remove obsolete variables in `repository_push_email` --- app/mailers/emails/projects.rb | 25 +++++---------------- app/views/notify/repository_push_email.html.haml | 28 +++++++++++++----------- app/views/notify/repository_push_email.text.haml | 24 ++++++++++---------- lib/gitlab/email/message/repository_push.rb | 17 +++++++++++--- 4 files changed, 46 insertions(+), 48 deletions(-) diff --git a/app/mailers/emails/projects.rb b/app/mailers/emails/projects.rb index bedadb583c7..35015ca34f8 100644 --- a/app/mailers/emails/projects.rb +++ b/app/mailers/emails/projects.rb @@ -60,28 +60,13 @@ module Emails end def repository_push_email(project_id, recipient, opts = {}) - repository_push = + @message = Gitlab::Email::Message::RepositoryPush.new(self, project_id, recipient, opts) - @project = repository_push.project - @current_user = @author = repository_push.author - @compare = repository_push.compare - @ref_name = repository_push.ref_name - @ref_type = repository_push.ref_type - @action = repository_push.action - @action_name = repository_push.action_name - @commits = repository_push.commits - @diffs = repository_push.diffs - @target_url = repository_push.target_url - @disable_diffs = repository_push.disable_diffs? - @reverse_compare = repository_push.reverse_compare? - @disable_footer = true - - mail(from: sender(repository_push.author_id, - repository_push.send_from_committer_email?), - reply_to: repository_push.reply_to, - to: repository_push.recipient, - subject: repository_push.subject) + mail(from: sender(@message.author_id, @message.send_from_committer_email?), + reply_to: @message.reply_to, + to: @message.recipient, + subject: @message.subject) end end end diff --git a/app/views/notify/repository_push_email.html.haml b/app/views/notify/repository_push_email.html.haml index 12f83aae04b..4361f67a74d 100644 --- a/app/views/notify/repository_push_email.html.haml +++ b/app/views/notify/repository_push_email.html.haml @@ -1,30 +1,32 @@ -%h3 #{@author.name} #{@action_name} #{@ref_type} #{@ref_name} at #{link_to @project.name_with_namespace, namespace_project_url(@project.namespace, @project)} +%h3 + #{@message.author_name} #{@message.action_name} #{@message.ref_type} #{@message.ref_name} + at #{link_to(@message.project_name_with_namespace, namespace_project_url(@message.project_namespace, @message.project))} -- if @compare - - if @reverse_compare +- if @message.compare + - if @message.reverse_compare? %p %strong WARNING: The push did not contain any new commits, but force pushed to delete the commits and changes below. %h4 - = @reverse_compare ? "Deleted commits:" : "Commits:" + = @message.reverse_compare? ? "Deleted commits:" : "Commits:" %ul - - @commits.each do |commit| + - @message.commits.each do |commit| %li - %strong #{link_to commit.short_id, namespace_project_commit_url(@project.namespace, @project, commit)} + %strong #{link_to(commit.short_id, namespace_project_commit_url(@message.project_namespace, @message.project, commit))} %div %span by #{commit.author_name} %i at #{commit.committed_date.strftime("%Y-%m-%dT%H:%M:%SZ")} %pre.commit-message = commit.safe_message - %h4 #{pluralize @diffs.count, "changed file"}: + %h4 #{pluralize @message.diffs_count, "changed file"}: %ul - - @diffs.each_with_index do |diff, i| + - @message.diffs.each_with_index do |diff, i| %li.file-stats - %a{href: "#{@target_url if @disable_diffs}#diff-#{i}" } + %a{href: "#{@message.target_url if @message.disable_diffs?}#diff-#{i}" } - if diff.deleted_file %span.deleted-file − @@ -40,11 +42,11 @@ - else = diff.new_path - - unless @disable_diffs + - unless @message.disable_diffs? %h4 Changes: - - @diffs.each_with_index do |diff, i| + - @message.diffs.each_with_index do |diff, i| %li{id: "diff-#{i}"} - %a{href: @target_url + "#diff-#{i}"} + %a{href: @message.target_url + "#diff-#{i}"} - if diff.deleted_file %strong = diff.old_path @@ -62,5 +64,5 @@ = color_email_diff(diff.diff) %br - - if @compare.timeout + - if @message.compare_timeout %h5 Huge diff. To prevent performance issues changes are hidden diff --git a/app/views/notify/repository_push_email.text.haml b/app/views/notify/repository_push_email.text.haml index 97a176ed2a3..aa0e263b6df 100644 --- a/app/views/notify/repository_push_email.text.haml +++ b/app/views/notify/repository_push_email.text.haml @@ -1,21 +1,21 @@ -#{@author.name} #{@action_name} #{@ref_type} #{@ref_name} at #{@project.name_with_namespace} -- if @compare +#{@message.author_name} #{@message.action_name} #{@message.ref_type} #{@message.ref_name} at #{@message.project_name_with_namespace} +- if @message.compare \ \ - - if @reverse_compare + - if @message.reverse_compare? WARNING: The push did not contain any new commits, but force pushed to delete the commits and changes below. \ \ - = @reverse_compare ? "Deleted commits:" : "Commits:" - - @commits.each do |commit| + = @message.reverse_compare? ? "Deleted commits:" : "Commits:" + - @message.commits.each do |commit| #{commit.short_id} by #{commit.author_name} at #{commit.committed_date.strftime("%Y-%m-%dT%H:%M:%SZ")} #{commit.safe_message} \- - - - - \ \ - #{pluralize @diffs.count, "changed file"}: + #{pluralize @message.diffs_count, "changed file"}: \ - - @diffs.each do |diff| + - @message.diffs.each do |diff| - if diff.deleted_file \- − #{diff.old_path} - elsif diff.renamed_file @@ -24,11 +24,11 @@ \- + #{diff.new_path} - else \- #{diff.new_path} - - unless @disable_diffs + - unless @message.disable_diffs? \ \ Changes: - - @diffs.each do |diff| + - @message.diffs.each do |diff| \ \===================================== - if diff.deleted_file @@ -39,11 +39,11 @@ = diff.new_path \===================================== != diff.diff - - if @compare.timeout + - if @message.compare_timeout \ \ Huge diff. To prevent performance issues it was hidden - - if @target_url + - if @message.target_url \ \ - View it on GitLab: #{@target_url} + View it on GitLab: #{@message.target_url} diff --git a/lib/gitlab/email/message/repository_push.rb b/lib/gitlab/email/message/repository_push.rb index 61962abf822..b0af3feee9e 100644 --- a/lib/gitlab/email/message/repository_push.rb +++ b/lib/gitlab/email/message/repository_push.rb @@ -5,6 +5,9 @@ module Gitlab attr_accessor :recipient attr_reader :author_id, :ref, :action + delegate :namespace, :name_with_namespace, to: :project, prefix: :project + delegate :name, to: :author, prefix: :author + def initialize(notify, project_id, recipient, opts = {}) raise ArgumentError, 'Missing options: author_id, ref, action' unless opts[:author_id] && opts[:ref] && opts[:action] @@ -35,10 +38,18 @@ module Gitlab @diffs ||= (compare.diffs if compare) end + def diffs_count + diffs.count if diffs + end + def compare @opts[:compare] end + def compare_timeout + compare.timeout if compare + end + def reverse_compare? @opts[:reverse_compare] || false end @@ -74,17 +85,17 @@ module Gitlab def target_url if @action == :push if commits.length > 1 && compare - @notify.namespace_project_compare_url(project.namespace, + @notify.namespace_project_compare_url(project_namespace, project, from: Commit.new(compare.base, project), to: Commit.new(compare.head, project)) else - @notify.namespace_project_commit_url(project.namespace, + @notify.namespace_project_commit_url(project_namespace, project, commits.first) end else unless @action == :delete - @notify.namespace_project_tree_url(project.namespace, + @notify.namespace_project_tree_url(project_namespace, project, ref_name) end end -- cgit v1.2.1 From d835fbc79f6cd6f17fc7472af48074805622a573 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Sun, 22 Nov 2015 20:59:31 +0100 Subject: Fix url helpers in RepositoryPush --- lib/gitlab/email/message/repository_push.rb | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/gitlab/email/message/repository_push.rb b/lib/gitlab/email/message/repository_push.rb index b0af3feee9e..bc8aece733f 100644 --- a/lib/gitlab/email/message/repository_push.rb +++ b/lib/gitlab/email/message/repository_push.rb @@ -16,6 +16,7 @@ module Gitlab @project_id = project_id @recipient = recipient @opts = opts + @urls = Gitlab::Application.routes.url_helpers @author_id = opts.delete(:author_id) @ref = opts.delete(:ref) @@ -85,17 +86,17 @@ module Gitlab def target_url if @action == :push if commits.length > 1 && compare - @notify.namespace_project_compare_url(project_namespace, + @urls.namespace_project_compare_url(project_namespace, project, from: Commit.new(compare.base, project), to: Commit.new(compare.head, project)) else - @notify.namespace_project_commit_url(project_namespace, + @urls.namespace_project_commit_url(project_namespace, project, commits.first) end else unless @action == :delete - @notify.namespace_project_tree_url(project_namespace, + @urls.namespace_project_tree_url(project_namespace, project, ref_name) end end -- cgit v1.2.1 From 360a96a299397cbb5f8b15f916a9efd5962ff6be Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Mon, 23 Nov 2015 20:25:49 +0000 Subject: Fix specs by adding forgotten instance variable --- app/mailers/emails/projects.rb | 3 +++ 1 file changed, 3 insertions(+) diff --git a/app/mailers/emails/projects.rb b/app/mailers/emails/projects.rb index 35015ca34f8..b96418679bd 100644 --- a/app/mailers/emails/projects.rb +++ b/app/mailers/emails/projects.rb @@ -63,6 +63,9 @@ module Emails @message = Gitlab::Email::Message::RepositoryPush.new(self, project_id, recipient, opts) + # used in notify layout + @target_url = @message.target_url + mail(from: sender(@message.author_id, @message.send_from_committer_email?), reply_to: @message.reply_to, to: @message.recipient, -- cgit v1.2.1 From 75c6b29f6b15e164717c27e6c3d7eecb84c923f8 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Thu, 3 Dec 2015 14:16:16 +0100 Subject: Add `RepositoryPush` specs --- lib/gitlab/email/message/repository_push.rb | 10 +- .../gitlab/email/message/repository_push_spec.rb | 122 +++++++++++++++++++++ 2 files changed, 127 insertions(+), 5 deletions(-) create mode 100644 spec/lib/gitlab/email/message/repository_push_spec.rb diff --git a/lib/gitlab/email/message/repository_push.rb b/lib/gitlab/email/message/repository_push.rb index bc8aece733f..3526eb2cad9 100644 --- a/lib/gitlab/email/message/repository_push.rb +++ b/lib/gitlab/email/message/repository_push.rb @@ -87,17 +87,17 @@ module Gitlab if @action == :push if commits.length > 1 && compare @urls.namespace_project_compare_url(project_namespace, - project, - from: Commit.new(compare.base, project), - to: Commit.new(compare.head, project)) + project, + from: Commit.new(compare.base, project), + to: Commit.new(compare.head, project)) else @urls.namespace_project_commit_url(project_namespace, - project, commits.first) + project, commits.first) end else unless @action == :delete @urls.namespace_project_tree_url(project_namespace, - project, ref_name) + project, ref_name) end end end diff --git a/spec/lib/gitlab/email/message/repository_push_spec.rb b/spec/lib/gitlab/email/message/repository_push_spec.rb new file mode 100644 index 00000000000..56ae2a8d121 --- /dev/null +++ b/spec/lib/gitlab/email/message/repository_push_spec.rb @@ -0,0 +1,122 @@ +require 'spec_helper' + +describe Gitlab::Email::Message::RepositoryPush do + include RepoHelpers + + let!(:group) { create(:group, name: 'my_group') } + let!(:project) { create(:project, name: 'my_project', namespace: group) } + let!(:author) { create(:author, name: 'Author') } + + let(:message) do + described_class.new(Notify, project.id, 'recipient@example.com', opts) + end + + context 'new commits have been pushed to repository' do + let(:opts) do + { author_id: author.id, ref: 'master', action: :push, compare: compare, + send_from_committer_email: true } + end + let(:compare) do + Gitlab::Git::Compare.new(project.repository.raw_repository, + sample_image_commit.id, sample_commit.id) + end + + describe '#project' do + subject { message.project } + it { is_expected.to eq project } + it { is_expected.to be_an_instance_of Project } + end + + describe '#project_namespace' do + subject { message.project_namespace } + it { is_expected.to eq group } + it { is_expected.to be_kind_of Namespace } + end + + describe '#project_name_with_namespace' do + subject { message.project_name_with_namespace } + it { is_expected.to eq 'my_group / my_project' } + end + + describe '#author' do + subject { message.author } + it { is_expected.to eq author } + it { is_expected.to be_an_instance_of User } + end + + describe '#author_name' do + subject { message.author_name } + it { is_expected.to eq 'Author' } + end + + describe '#commits' do + subject { message.commits } + it { is_expected.to be_kind_of Array } + it { is_expected.to all(be_instance_of Commit) } + end + + describe '#diffs' do + subject { message.diffs } + it { is_expected.to all(be_an_instance_of Gitlab::Git::Diff) } + end + + describe '#diffs_count' do + subject { message.diffs_count } + it { is_expected.to eq compare.diffs.count } + end + + describe '#compare' do + subject { message.compare } + it { is_expected.to be_an_instance_of Gitlab::Git::Compare } + end + + describe '#compare_timeout' do + subject { message.compare_timeout } + it { is_expected.to eq compare.timeout } + end + + describe '#reverse_compare?' do + subject { message.reverse_compare? } + it { is_expected.to eq false } + end + + describe '#disable_diffs?' do + subject { message.disable_diffs? } + it { is_expected.to eq false } + end + + describe '#send_from_committer_email?' do + subject { message.send_from_committer_email? } + it { is_expected.to eq true } + end + + describe '#action_name' do + subject { message.action_name } + it { is_expected.to eq 'pushed to' } + end + + describe '#ref_name' do + subject { message.ref_name } + it { is_expected.to eq 'master' } + end + + describe '#ref_type' do + subject { message.ref_type } + it { is_expected.to eq 'branch' } + end + + describe '#target_url' do + subject { message.target_url } + it { is_expected.to include 'compare' } + it { is_expected.to include compare.commits.first.parents.first.id } + it { is_expected.to include compare.commits.last.id } + end + + describe '#subject' do + subject { message.subject } + it { is_expected.to include "[Git][#{project.path_with_namespace}]" } + it { is_expected.to include "#{compare.commits.length} commits" } + it { is_expected.to include compare.commits.first.message.split("\n").first } + end + end +end -- cgit v1.2.1 From 7e9109c43bdaedb79b06ba8948cd729ac5f68deb Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Fri, 4 Dec 2015 13:07:42 +0100 Subject: Move `common_mentionable_setup` to shared context in specs --- spec/support/mentionable_shared_examples.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/support/mentionable_shared_examples.rb b/spec/support/mentionable_shared_examples.rb index 33d2b14583c..fce91015fd4 100644 --- a/spec/support/mentionable_shared_examples.rb +++ b/spec/support/mentionable_shared_examples.rb @@ -4,7 +4,7 @@ # - let(:backref_text) { "the way that +subject+ should refer to itself in backreferences " } # - let(:set_mentionable_text) { lambda { |txt| "block that assigns txt to the subject's mentionable_text" } } -def common_mentionable_setup +shared_context 'mentionable context' do let(:project) { subject.project } let(:author) { subject.author } @@ -56,7 +56,7 @@ def common_mentionable_setup end shared_examples 'a mentionable' do - common_mentionable_setup + include_context 'mentionable context' it 'generates a descriptive back-reference' do expect(subject.gfm_reference).to eq(backref_text) @@ -88,7 +88,7 @@ shared_examples 'a mentionable' do end shared_examples 'an editable mentionable' do - common_mentionable_setup + include_context 'mentionable context' it_behaves_like 'a mentionable' -- cgit v1.2.1 From 591035968dc96acd27155ced4c0ae04649fcd113 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Fri, 4 Dec 2015 12:55:19 +0000 Subject: Duplicate options in `RepositoryPush` --- lib/gitlab/email/message/repository_push.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/gitlab/email/message/repository_push.rb b/lib/gitlab/email/message/repository_push.rb index 3526eb2cad9..ca89b67e580 100644 --- a/lib/gitlab/email/message/repository_push.rb +++ b/lib/gitlab/email/message/repository_push.rb @@ -15,12 +15,12 @@ module Gitlab @notify = notify @project_id = project_id @recipient = recipient - @opts = opts + @opts = opts.dup @urls = Gitlab::Application.routes.url_helpers - @author_id = opts.delete(:author_id) - @ref = opts.delete(:ref) - @action = opts.delete(:action) + @author_id = @opts.delete(:author_id) + @ref = @opts.delete(:ref) + @action = @opts.delete(:action) end def project -- cgit v1.2.1 From 66f658a9b543b1493f625b2f44f3f845d64b749d Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Fri, 4 Dec 2015 14:11:17 +0100 Subject: Check if commits are available in `RepositoryPush` --- lib/gitlab/email/message/repository_push.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/gitlab/email/message/repository_push.rb b/lib/gitlab/email/message/repository_push.rb index ca89b67e580..c53148d2ed8 100644 --- a/lib/gitlab/email/message/repository_push.rb +++ b/lib/gitlab/email/message/repository_push.rb @@ -84,8 +84,8 @@ module Gitlab end def target_url - if @action == :push - if commits.length > 1 && compare + if @action == :push && commits + if commits.length > 1 @urls.namespace_project_compare_url(project_namespace, project, from: Commit.new(compare.base, project), @@ -116,7 +116,7 @@ module Gitlab subject_text << "[#{ref_name}]" if @action == :push subject_text << ' ' - if @action == :push + if @action == :push && commits if commits.length > 1 subject_text << "Deleted " if reverse_compare? subject_text << "#{commits.length} commits: #{commits.first.title}" -- cgit v1.2.1 From 652de0b820587983e0af76186db4570b536d7ce3 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Mon, 7 Dec 2015 10:43:07 +0100 Subject: Refactor CI YAML processor's validators --- lib/ci/gitlab_ci_yaml_processor.rb | 66 +++++++++++++++++----------- spec/lib/ci/gitlab_ci_yaml_processor_spec.rb | 6 +-- 2 files changed, 43 insertions(+), 29 deletions(-) diff --git a/lib/ci/gitlab_ci_yaml_processor.rb b/lib/ci/gitlab_ci_yaml_processor.rb index 3beafcad117..9c11fc3c81d 100644 --- a/lib/ci/gitlab_ci_yaml_processor.rb +++ b/lib/ci/gitlab_ci_yaml_processor.rb @@ -132,26 +132,36 @@ module Ci end def validate_job!(name, job) + validate_job_name!(name) + validate_job_keys!(name, job) + validate_job_types!(name, job) + + validate_job_stage!(name, job) if job[:stage] + validate_job_cache!(name, job) if job[:cache] + validate_job_artifacts!(name, job) if job[:artifacts] + end + + private + + def validate_job_name!(name) if name.blank? || !validate_string(name) raise ValidationError, "job name should be non-empty string" end + end + def validate_job_keys!(name, job) job.keys.each do |key| unless ALLOWED_JOB_KEYS.include? key raise ValidationError, "#{name} job: unknown parameter #{key}" end end + end + def validate_job_types!(name, job) if !validate_string(job[:script]) && !validate_array_of_strings(job[:script]) raise ValidationError, "#{name} job: script should be a string or an array of a strings" end - if job[:stage] - unless job[:stage].is_a?(String) && job[:stage].in?(stages) - raise ValidationError, "#{name} job: stage parameter should be #{stages.join(", ")}" - end - end - if job[:image] && !validate_string(job[:image]) raise ValidationError, "#{name} job: image should be a string" end @@ -172,36 +182,40 @@ module Ci raise ValidationError, "#{name} job: except parameter should be an array of strings" end - if job[:cache] - if job[:cache][:untracked] && !validate_boolean(job[:cache][:untracked]) - raise ValidationError, "#{name} job: cache:untracked parameter should be an boolean" - end - - if job[:cache][:paths] && !validate_array_of_strings(job[:cache][:paths]) - raise ValidationError, "#{name} job: cache:paths parameter should be an array of strings" - end + if job[:allow_failure] && !validate_boolean(job[:allow_failure]) + raise ValidationError, "#{name} job: allow_failure parameter should be an boolean" end - if job[:artifacts] - if job[:artifacts][:untracked] && !validate_boolean(job[:artifacts][:untracked]) - raise ValidationError, "#{name} job: artifacts:untracked parameter should be an boolean" - end + if job[:when] && !job[:when].in?(%w(on_success on_failure always)) + raise ValidationError, "#{name} job: when parameter should be on_success, on_failure or always" + end + end - if job[:artifacts][:paths] && !validate_array_of_strings(job[:artifacts][:paths]) - raise ValidationError, "#{name} job: artifacts:paths parameter should be an array of strings" - end + def validate_job_stage!(name, job) + unless job[:stage].is_a?(String) && job[:stage].in?(stages) + raise ValidationError, "#{name} job: stage parameter should be #{stages.join(", ")}" end + end - if job[:allow_failure] && !validate_boolean(job[:allow_failure]) - raise ValidationError, "#{name} job: allow_failure parameter should be an boolean" + def validate_job_cache!(name, job) + if job[:cache][:untracked] && !validate_boolean(job[:cache][:untracked]) + raise ValidationError, "#{name} job: cache:untracked parameter should be an boolean" end - if job[:when] && !job[:when].in?(%w(on_success on_failure always)) - raise ValidationError, "#{name} job: when parameter should be on_success, on_failure or always" + if job[:cache][:paths] && !validate_array_of_strings(job[:cache][:paths]) + raise ValidationError, "#{name} job: cache:paths parameter should be an array of strings" end end - private + def validate_job_artifacts!(name, job) + if job[:artifacts][:untracked] && !validate_boolean(job[:artifacts][:untracked]) + raise ValidationError, "#{name} job: artifacts:untracked parameter should be an boolean" + end + + if job[:artifacts][:paths] && !validate_array_of_strings(job[:artifacts][:paths]) + raise ValidationError, "#{name} job: artifacts:paths parameter should be an array of strings" + end + end def validate_array_of_strings(values) values.is_a?(Array) && values.all? { |value| validate_string(value) } diff --git a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb index 6f287719ba6..0d50f20ae34 100644 --- a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb +++ b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb @@ -532,21 +532,21 @@ module Ci end it "returns errors if job stage is not a string" do - config = YAML.dump({ rspec: { script: "test", type: 1, allow_failure: "string" } }) + config = YAML.dump({ rspec: { script: "test", type: 1 } }) expect do GitlabCiYamlProcessor.new(config, path) end.to raise_error(GitlabCiYamlProcessor::ValidationError, "rspec job: stage parameter should be build, test, deploy") end it "returns errors if job stage is not a pre-defined stage" do - config = YAML.dump({ rspec: { script: "test", type: "acceptance", allow_failure: "string" } }) + config = YAML.dump({ rspec: { script: "test", type: "acceptance" } }) expect do GitlabCiYamlProcessor.new(config, path) end.to raise_error(GitlabCiYamlProcessor::ValidationError, "rspec job: stage parameter should be build, test, deploy") end it "returns errors if job stage is not a defined stage" do - config = YAML.dump({ types: ["build", "test"], rspec: { script: "test", type: "acceptance", allow_failure: "string" } }) + config = YAML.dump({ types: ["build", "test"], rspec: { script: "test", type: "acceptance" } }) expect do GitlabCiYamlProcessor.new(config, path) end.to raise_error(GitlabCiYamlProcessor::ValidationError, "rspec job: stage parameter should be build, test") -- cgit v1.2.1 From 6ffe8a06fdf1ffc40e60a487a730b50c4699907d Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Mon, 7 Dec 2015 10:52:22 +0100 Subject: Bump cyclomatic and perceived complexity threshold by one --- .rubocop.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index fd382182abe..b4ca11c8343 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -743,14 +743,14 @@ Metrics/CyclomaticComplexity: A complexity metric that is strongly correlated to the number of test cases needed to validate a method. Enabled: true - Max: 16 + Max: 17 Metrics/PerceivedComplexity: Description: >- A complexity metric geared towards measuring complexity for a human reader. Enabled: true - Max: 16 + Max: 17 Metrics/ParameterLists: Description: 'Avoid parameter lists longer than three or four parameters.' -- cgit v1.2.1 From cbeb06eb420f294b3a406f869f11de554048e93d Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Fri, 11 Dec 2015 13:00:24 +0000 Subject: Mix url helpers in into `RepositoryPush` --- lib/gitlab/email/message/repository_push.rb | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/lib/gitlab/email/message/repository_push.rb b/lib/gitlab/email/message/repository_push.rb index c53148d2ed8..a2eb7a70bd2 100644 --- a/lib/gitlab/email/message/repository_push.rb +++ b/lib/gitlab/email/message/repository_push.rb @@ -5,6 +5,8 @@ module Gitlab attr_accessor :recipient attr_reader :author_id, :ref, :action + include Gitlab::Application.routes.url_helpers + delegate :namespace, :name_with_namespace, to: :project, prefix: :project delegate :name, to: :author, prefix: :author @@ -16,7 +18,6 @@ module Gitlab @project_id = project_id @recipient = recipient @opts = opts.dup - @urls = Gitlab::Application.routes.url_helpers @author_id = @opts.delete(:author_id) @ref = @opts.delete(:ref) @@ -86,18 +87,18 @@ module Gitlab def target_url if @action == :push && commits if commits.length > 1 - @urls.namespace_project_compare_url(project_namespace, - project, - from: Commit.new(compare.base, project), - to: Commit.new(compare.head, project)) + namespace_project_compare_url(project_namespace, + project, + from: Commit.new(compare.base, project), + to: Commit.new(compare.head, project)) else - @urls.namespace_project_commit_url(project_namespace, - project, commits.first) + namespace_project_commit_url(project_namespace, + project, commits.first) end else unless @action == :delete - @urls.namespace_project_tree_url(project_namespace, - project, ref_name) + namespace_project_tree_url(project_namespace, + project, ref_name) end end end -- cgit v1.2.1