diff options
author | Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com> | 2015-11-17 19:17:15 +0100 |
---|---|---|
committer | Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com> | 2015-11-17 19:17:15 +0100 |
commit | 5098ad9d3c892b2ba117950eac8770fcfad9e53b (patch) | |
tree | 0e83c08032778ad503a8f305a98d692d583ddcbe | |
parent | 6f6c75cb85195984c6e610ad109464dcb952a5fe (diff) | |
parent | fab04fac13c9f9c116e1e4ed744244c5041e198d (diff) | |
download | gitlab-ce-5098ad9d3c892b2ba117950eac8770fcfad9e53b.tar.gz |
Merge branch 'remove-code-duplication'
Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
22 files changed, 371 insertions, 434 deletions
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 141e7ba41de..94753093540 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -87,4 +87,3 @@ flay: tags: - ruby - mysql - allow_failure: true diff --git a/app/controllers/concerns/issues_action.rb b/app/controllers/concerns/issues_action.rb new file mode 100644 index 00000000000..effd4721949 --- /dev/null +++ b/app/controllers/concerns/issues_action.rb @@ -0,0 +1,14 @@ +module IssuesAction + extend ActiveSupport::Concern + + def issues + @issues = get_issues_collection + @issues = @issues.page(params[:page]).per(ApplicationController::PER_PAGE) + @issues = @issues.preload(:author, :project) + + respond_to do |format| + format.html + format.atom { render layout: false } + end + end +end diff --git a/app/controllers/concerns/merge_requests_action.rb b/app/controllers/concerns/merge_requests_action.rb new file mode 100644 index 00000000000..f7a25111db9 --- /dev/null +++ b/app/controllers/concerns/merge_requests_action.rb @@ -0,0 +1,9 @@ +module MergeRequestsAction + extend ActiveSupport::Concern + + def merge_requests + @merge_requests = get_merge_requests_collection + @merge_requests = @merge_requests.page(params[:page]).per(ApplicationController::PER_PAGE) + @merge_requests = @merge_requests.preload(:author, :target_project) + end +end diff --git a/app/controllers/dashboard_controller.rb b/app/controllers/dashboard_controller.rb index b2c1fa4230c..087da935087 100644 --- a/app/controllers/dashboard_controller.rb +++ b/app/controllers/dashboard_controller.rb @@ -1,26 +1,12 @@ class DashboardController < Dashboard::ApplicationController + include IssuesAction + include MergeRequestsAction + before_action :event_filter, only: :activity before_action :projects, only: [:issues, :merge_requests] respond_to :html - def merge_requests - @merge_requests = get_merge_requests_collection - @merge_requests = @merge_requests.page(params[:page]).per(PER_PAGE) - @merge_requests = @merge_requests.preload(:author, :target_project) - end - - def issues - @issues = get_issues_collection - @issues = @issues.page(params[:page]).per(PER_PAGE) - @issues = @issues.preload(:author, :project) - - respond_to do |format| - format.html - format.atom { render layout: false } - end - end - def activity @last_push = current_user.recent_push diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb index fb4eb094f27..fb26a4e6fc3 100644 --- a/app/controllers/groups_controller.rb +++ b/app/controllers/groups_controller.rb @@ -1,4 +1,7 @@ class GroupsController < Groups::ApplicationController + include IssuesAction + include MergeRequestsAction + skip_before_action :authenticate_user!, only: [:show, :issues, :merge_requests] respond_to :html before_action :group, except: [:new, :create] @@ -53,23 +56,6 @@ class GroupsController < Groups::ApplicationController end end - def merge_requests - @merge_requests = get_merge_requests_collection - @merge_requests = @merge_requests.page(params[:page]).per(PER_PAGE) - @merge_requests = @merge_requests.preload(:author, :target_project) - end - - def issues - @issues = get_issues_collection - @issues = @issues.page(params[:page]).per(PER_PAGE) - @issues = @issues.preload(:author, :project) - - respond_to do |format| - format.html - format.atom { render layout: false } - end - end - def edit end diff --git a/app/helpers/diff_helper.rb b/app/helpers/diff_helper.rb index f47d5c4c742..b715ecd85d2 100644 --- a/app/helpers/diff_helper.rb +++ b/app/helpers/diff_helper.rb @@ -136,25 +136,11 @@ module DiffHelper end def inline_diff_btn - params_copy = params.dup - params_copy[:view] = 'inline' - # Always use HTML to handle case where JSON diff rendered this button - params_copy.delete(:format) - - link_to url_for(params_copy), id: "inline-diff-btn", class: (diff_view == 'inline' ? 'btn active' : 'btn') do - 'Inline' - end + diff_btn('Inline', 'inline', diff_view == 'inline') end def parallel_diff_btn - params_copy = params.dup - params_copy[:view] = 'parallel' - # Always use HTML to handle case where JSON diff rendered this button - params_copy.delete(:format) - - link_to url_for(params_copy), id: "parallel-diff-btn", class: (diff_view == 'parallel' ? 'btn active' : 'btn') do - 'Side-by-side' - end + diff_btn('Side-by-side', 'parallel', diff_view == 'parallel') end def submodule_link(blob, ref, repository = @repository) @@ -191,4 +177,18 @@ module DiffHelper def editable_diff?(diff) !diff.deleted_file && @merge_request && @merge_request.source_project end + + private + + def diff_btn(title, name, selected) + params_copy = params.dup + params_copy[:view] = name + + # Always use HTML to handle case where JSON diff rendered this button + params_copy.delete(:format) + + link_to url_for(params_copy), id: "#{name}-diff-btn", class: (selected ? 'btn active' : 'btn') do + title + end + end end diff --git a/app/helpers/gitlab_markdown_helper.rb b/app/helpers/gitlab_markdown_helper.rb index 65813482120..98c6d9d5d2e 100644 --- a/app/helpers/gitlab_markdown_helper.rb +++ b/app/helpers/gitlab_markdown_helper.rb @@ -46,39 +46,13 @@ module GitlabMarkdownHelper end def markdown(text, context = {}) - return "" unless text.present? - - context.reverse_merge!( - path: @path, - pipeline: :default, - project: @project, - project_wiki: @project_wiki, - ref: @ref - ) - - user = current_user if defined?(current_user) - - html = Gitlab::Markdown.render(text, context) - Gitlab::Markdown.post_process(html, pipeline: context[:pipeline], project: @project, user: user) + process_markdown(text, context) end # TODO (rspeicher): Remove all usages of this helper and just call `markdown` # with a custom pipeline depending on the content being rendered def gfm(text, options = {}) - return "" unless text.present? - - options.reverse_merge!( - path: @path, - pipeline: :default, - project: @project, - project_wiki: @project_wiki, - ref: @ref - ) - - user = current_user if defined?(current_user) - - html = Gitlab::Markdown.gfm(text, options) - Gitlab::Markdown.post_process(html, pipeline: options[:pipeline], project: @project, user: user) + process_markdown(text, options, :gfm) end def asciidoc(text) @@ -204,4 +178,26 @@ module GitlabMarkdownHelper '' end end + + def process_markdown(text, options, method = :markdown) + return "" unless text.present? + + options.reverse_merge!( + path: @path, + pipeline: :default, + project: @project, + project_wiki: @project_wiki, + ref: @ref + ) + + user = current_user if defined?(current_user) + + html = if method == :gfm + Gitlab::Markdown.gfm(text, options) + else + Gitlab::Markdown.render(text, options) + end + + Gitlab::Markdown.post_process(html, pipeline: options[:pipeline], project: @project, user: user) + end end diff --git a/app/helpers/namespaces_helper.rb b/app/helpers/namespaces_helper.rb index b3132a1f3ba..e7f3cb21038 100644 --- a/app/helpers/namespaces_helper.rb +++ b/app/helpers/namespaces_helper.rb @@ -17,15 +17,6 @@ module NamespacesHelper grouped_options_for_select(options, selected) end - def namespace_select_tag(id, opts = {}) - css_class = "ajax-namespace-select " - css_class << "multiselect " if opts[:multiple] - css_class << (opts[:class] || '') - value = opts[:selected] || '' - - hidden_field_tag(id, value, class: css_class) - end - def namespace_icon(namespace, size = 40) if namespace.kind_of?(Group) group_icon(namespace) diff --git a/app/helpers/selects_helper.rb b/app/helpers/selects_helper.rb index 12fce8db701..7e54d4d1b5b 100644 --- a/app/helpers/selects_helper.rb +++ b/app/helpers/selects_helper.rb @@ -35,8 +35,20 @@ module SelectsHelper end def groups_select_tag(id, opts = {}) - css_class = "ajax-groups-select " - css_class << "multiselect " if opts[:multiple] + opts[:class] ||= '' + opts[:class] << ' ajax-groups-select' + select2_tag(id, opts) + end + + def namespace_select_tag(id, opts = {}) + opts[:class] ||= '' + opts[:class] << ' ajax-namespace-select' + select2_tag(id, opts) + end + + def select2_tag(id, opts = {}) + css_class = '' + css_class << 'multiselect ' if opts[:multiple] css_class << (opts[:class] || '') value = opts[:selected] || '' diff --git a/app/mailers/emails/issues.rb b/app/mailers/emails/issues.rb index 2c035fbb70b..abdeefed5ef 100644 --- a/app/mailers/emails/issues.rb +++ b/app/mailers/emails/issues.rb @@ -1,53 +1,49 @@ module Emails module Issues def new_issue_email(recipient_id, issue_id) - @issue = Issue.find(issue_id) - @project = @issue.project - @target_url = namespace_project_issue_url(@project.namespace, @project, @issue) - mail_new_thread(@issue, - from: sender(@issue.author_id), - to: recipient(recipient_id), - subject: subject("#{@issue.title} (##{@issue.iid})")) - - SentNotification.record(@issue, recipient_id, reply_key) + issue_mail_with_notification(issue_id, recipient_id) do + mail_new_thread(@issue, issue_thread_options(@issue.author_id, recipient_id)) + end end def reassigned_issue_email(recipient_id, issue_id, previous_assignee_id, updated_by_user_id) - @issue = Issue.find(issue_id) - @previous_assignee = User.find_by(id: previous_assignee_id) if previous_assignee_id - @project = @issue.project - @target_url = namespace_project_issue_url(@project.namespace, @project, @issue) - mail_answer_thread(@issue, - from: sender(updated_by_user_id), - to: recipient(recipient_id), - subject: subject("#{@issue.title} (##{@issue.iid})")) - - SentNotification.record(@issue, recipient_id, reply_key) + issue_mail_with_notification(issue_id, recipient_id) do + @previous_assignee = User.find_by(id: previous_assignee_id) if previous_assignee_id + mail_answer_thread(@issue, issue_thread_options(updated_by_user_id, recipient_id)) + end end def closed_issue_email(recipient_id, issue_id, updated_by_user_id) - @issue = Issue.find issue_id - @project = @issue.project - @updated_by = User.find updated_by_user_id - @target_url = namespace_project_issue_url(@project.namespace, @project, @issue) - mail_answer_thread(@issue, - from: sender(updated_by_user_id), - to: recipient(recipient_id), - subject: subject("#{@issue.title} (##{@issue.iid})")) - - SentNotification.record(@issue, recipient_id, reply_key) + issue_mail_with_notification(issue_id, recipient_id) do + @updated_by = User.find updated_by_user_id + mail_answer_thread(@issue, issue_thread_options(updated_by_user_id, recipient_id)) + end end def issue_status_changed_email(recipient_id, issue_id, status, updated_by_user_id) - @issue = Issue.find issue_id - @issue_status = status + issue_mail_with_notification(issue_id, recipient_id) do + @issue_status = status + @updated_by = User.find updated_by_user_id + mail_answer_thread(@issue, issue_thread_options(updated_by_user_id, recipient_id)) + end + end + + private + + def issue_thread_options(sender_id, recipient_id) + { + from: sender(sender_id), + to: recipient(recipient_id), + subject: subject("#{@issue.title} (##{@issue.iid})") + } + end + + def issue_mail_with_notification(issue_id, recipient_id) + @issue = Issue.find(issue_id) @project = @issue.project - @updated_by = User.find updated_by_user_id @target_url = namespace_project_issue_url(@project.namespace, @project, @issue) - mail_answer_thread(@issue, - from: sender(updated_by_user_id), - to: recipient(recipient_id), - subject: subject("#{@issue.title} (##{@issue.iid})")) + + yield SentNotification.record(@issue, recipient_id, reply_key) end diff --git a/app/mailers/emails/notes.rb b/app/mailers/emails/notes.rb index 87ba94a583d..65f37e92677 100644 --- a/app/mailers/emails/notes.rb +++ b/app/mailers/emails/notes.rb @@ -1,49 +1,54 @@ module Emails module Notes def note_commit_email(recipient_id, note_id) - @note = Note.find(note_id) - @commit = @note.noteable - @project = @note.project - @target_url = namespace_project_commit_url(@project.namespace, @project, - @commit, anchor: - "note_#{@note.id}") - mail_answer_thread(@commit, - from: sender(@note.author_id), - to: recipient(recipient_id), - subject: subject("#{@commit.title} (#{@commit.short_id})")) - - SentNotification.record_note(@note, recipient_id, reply_key) + note_mail_with_notification(note_id, recipient_id) do + @commit = @note.noteable + @target_url = namespace_project_commit_url(*note_target_url_options) + + mail_answer_thread(@commit, + from: sender(@note.author_id), + to: recipient(recipient_id), + subject: subject("#{@commit.title} (#{@commit.short_id})")) + end end def note_issue_email(recipient_id, note_id) - @note = Note.find(note_id) - @issue = @note.noteable - @project = @note.project - @target_url = namespace_project_issue_url(@project.namespace, @project, - @issue, anchor: - "note_#{@note.id}") - mail_answer_thread(@issue, - from: sender(@note.author_id), - to: recipient(recipient_id), - subject: subject("#{@issue.title} (##{@issue.iid})")) - - SentNotification.record_note(@note, recipient_id, reply_key) + note_mail_with_notification(note_id, recipient_id) do + @issue = @note.noteable + @target_url = namespace_project_issue_url(*note_target_url_options) + mail_answer_thread(@issue, note_thread_options(recipient_id)) + end end def note_merge_request_email(recipient_id, note_id) + note_mail_with_notification(note_id, recipient_id) do + @merge_request = @note.noteable + @target_url = namespace_project_merge_request_url(*note_target_url_options) + mail_answer_thread(@merge_request, note_thread_options(recipient_id)) + end + end + + private + + def note_target_url_options + [@project.namespace, @project, @note.noteable, anchor: "note_#{@note.id}"] + end + + def note_thread_options(recipient_id) + { + from: sender(@note.author_id), + to: recipient(recipient_id), + subject: subject("#{@note.noteable.title} (##{@note.noteable.iid})") + } + end + + def note_mail_with_notification(note_id, recipient_id) @note = Note.find(note_id) - @merge_request = @note.noteable @project = @note.project - @target_url = namespace_project_merge_request_url(@project.namespace, - @project, - @merge_request, anchor: - "note_#{@note.id}") - mail_answer_thread(@merge_request, - from: sender(@note.author_id), - to: recipient(recipient_id), - subject: subject("#{@merge_request.title} (##{@merge_request.iid})")) - - SentNotification.record_note(@note, recipient_id, reply_key) + + yield + + SentNotification.record(@note, recipient_id, reply_key) end end end diff --git a/app/models/project_services/slack_service/note_message.rb b/app/models/project_services/slack_service/note_message.rb index 074478b292d..b15d9a14677 100644 --- a/app/models/project_services/slack_service/note_message.rb +++ b/app/models/project_services/slack_service/note_message.rb @@ -45,30 +45,27 @@ class SlackService def create_commit_note(commit) commit_sha = commit[:id] commit_sha = Commit.truncate_sha(commit_sha) - commit_link = "[commit #{commit_sha}](#{@note_url})" - title = format_title(commit[:message]) - @message = "#{@user_name} commented on #{commit_link} in #{project_link}: *#{title}*" + commented_on_message( + "[commit #{commit_sha}](#{@note_url})", + format_title(commit[:message])) end def create_issue_note(issue) - issue_iid = issue[:iid] - note_link = "[issue ##{issue_iid}](#{@note_url})" - title = format_title(issue[:title]) - @message = "#{@user_name} commented on #{note_link} in #{project_link}: *#{title}*" + commented_on_message( + "[issue ##{issue[:iid]}](#{@note_url})", + format_title(issue[:title])) end def create_merge_note(merge_request) - merge_request_id = merge_request[:iid] - merge_request_link = "[merge request ##{merge_request_id}](#{@note_url})" - title = format_title(merge_request[:title]) - @message = "#{@user_name} commented on #{merge_request_link} in #{project_link}: *#{title}*" + commented_on_message( + "[merge request ##{merge_request[:iid]}](#{@note_url})", + format_title(merge_request[:title])) end def create_snippet_note(snippet) - snippet_id = snippet[:id] - snippet_link = "[snippet ##{snippet_id}](#{@note_url})" - title = format_title(snippet[:title]) - @message = "#{@user_name} commented on #{snippet_link} in #{project_link}: *#{title}*" + commented_on_message( + "[snippet ##{snippet[:id]}](#{@note_url})", + format_title(snippet[:title])) end def description_message @@ -78,5 +75,9 @@ class SlackService def project_link "[#{@project_name}](#{@project_url})" end + + def commented_on_message(target_link, title) + @message = "#{@user_name} commented on #{target_link} in #{project_link}: *#{title}*" + end end end diff --git a/app/services/issuable_base_service.rb b/app/services/issuable_base_service.rb index 15b3825f96a..11d2b08bba7 100644 --- a/app/services/issuable_base_service.rb +++ b/app/services/issuable_base_service.rb @@ -28,6 +28,9 @@ class IssuableBaseService < BaseService end def filter_params(issuable_ability_name = :issue) + params[:assignee_id] = "" if params[:assignee_id] == IssuableFinder::NONE + params[:milestone_id] = "" if params[:milestone_id] == IssuableFinder::NONE + ability = :"admin_#{issuable_ability_name}" unless can?(current_user, ability, project) @@ -36,4 +39,36 @@ class IssuableBaseService < BaseService params.delete(:assignee_id) end end + + def update(issuable) + change_state(issuable) + filter_params + old_labels = issuable.labels.to_a + + if params.present? && issuable.update_attributes(params.merge(updated_by: current_user)) + issuable.reset_events_cache + + if issuable.labels != old_labels + create_labels_note( + issuable, + issuable.labels - old_labels, + old_labels - issuable.labels) + end + + handle_changes(issuable) + issuable.create_new_cross_references!(current_user) + execute_hooks(issuable, 'update') + end + + issuable + end + + def change_state(issuable) + case params.delete(:state_event) + when 'reopen' + reopen_service.new(project, current_user, {}).execute(issuable) + when 'close' + close_service.new(project, current_user, {}).execute(issuable) + end + end end diff --git a/app/services/issues/update_service.rb b/app/services/issues/update_service.rb index aa1fd79d22d..7c112f731a7 100644 --- a/app/services/issues/update_service.rb +++ b/app/services/issues/update_service.rb @@ -1,33 +1,7 @@ module Issues class UpdateService < Issues::BaseService def execute(issue) - case params.delete(:state_event) - when 'reopen' - Issues::ReopenService.new(project, current_user, {}).execute(issue) - when 'close' - Issues::CloseService.new(project, current_user, {}).execute(issue) - end - - params[:assignee_id] = "" if params[:assignee_id] == IssuableFinder::NONE - params[:milestone_id] = "" if params[:milestone_id] == IssuableFinder::NONE - - filter_params - old_labels = issue.labels.to_a - - if params.present? && issue.update_attributes(params.merge(updated_by: current_user)) - issue.reset_events_cache - - if issue.labels != old_labels - create_labels_note( - issue, issue.labels - old_labels, old_labels - issue.labels) - end - - handle_changes(issue) - issue.create_new_cross_references!(current_user) - execute_hooks(issue, 'update') - end - - issue + update(issue) end def handle_changes(issue) @@ -44,5 +18,13 @@ module Issues create_title_change_note(issue, issue.previous_changes['title'].first) end end + + def reopen_service + Issues::ReopenService + end + + def close_service + Issues::CloseService + end end end diff --git a/app/services/merge_requests/update_service.rb b/app/services/merge_requests/update_service.rb index d2849e5193f..a5db3776eb6 100644 --- a/app/services/merge_requests/update_service.rb +++ b/app/services/merge_requests/update_service.rb @@ -11,36 +11,7 @@ module MergeRequests params.except!(:target_project_id) params.except!(:source_branch) - case params.delete(:state_event) - when 'reopen' - MergeRequests::ReopenService.new(project, current_user, {}).execute(merge_request) - when 'close' - MergeRequests::CloseService.new(project, current_user, {}).execute(merge_request) - end - - params[:assignee_id] = "" if params[:assignee_id] == IssuableFinder::NONE - params[:milestone_id] = "" if params[:milestone_id] == IssuableFinder::NONE - - filter_params - old_labels = merge_request.labels.to_a - - if params.present? && merge_request.update_attributes(params.merge(updated_by: current_user)) - merge_request.reset_events_cache - - if merge_request.labels != old_labels - create_labels_note( - merge_request, - merge_request.labels - old_labels, - old_labels - merge_request.labels - ) - end - - handle_changes(merge_request) - merge_request.create_new_cross_references!(current_user) - execute_hooks(merge_request, 'update') - end - - merge_request + update(merge_request) end def handle_changes(merge_request) @@ -68,5 +39,13 @@ module MergeRequests merge_request.mark_as_unchecked end end + + def reopen_service + MergeRequests::ReopenService + end + + def close_service + MergeRequests::CloseService + end end end diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb index a6b22348650..4b871f072d4 100644 --- a/app/services/notification_service.rb +++ b/app/services/notification_service.rb @@ -113,7 +113,7 @@ class NotificationService end # Add all users participating in the thread (author, assignee, comment authors) - participants = + participants = if target.respond_to?(:participants) target.participants(note.author) else @@ -276,35 +276,25 @@ class NotificationService # Remove users with disabled notifications from array # Also remove duplications and nil recipients def reject_muted_users(users, project = nil) - users = users.to_a.compact.uniq - users = users.reject(&:blocked?) - - users.reject do |user| - next user.notification.disabled? unless project - - member = project.project_members.find_by(user_id: user.id) - - if !member && project.group - member = project.group.group_members.find_by(user_id: user.id) - end - - # reject users who globally disabled notification and has no membership - next user.notification.disabled? unless member - - # reject users who disabled notification in project - next true if member.notification.disabled? - - # reject users who have N_GLOBAL in project and disabled in global settings - member.notification.global? && user.notification.disabled? - end + reject_users(users, :disabled?, project) end # Remove users with notification level 'Mentioned' def reject_mention_users(users, project = nil) + reject_users(users, :mention?, project) + end + + # Reject users which method_name from notification object returns true. + # + # Example: + # reject_users(users, :watch?, project) + # + def reject_users(users, method_name, project = nil) users = users.to_a.compact.uniq + users = users.reject(&:blocked?) users.reject do |user| - next user.notification.mention? unless project + next user.notification.send(method_name) unless project member = project.project_members.find_by(user_id: user.id) @@ -313,19 +303,19 @@ class NotificationService end # reject users who globally set mention notification and has no membership - next user.notification.mention? unless member + next user.notification.send(method_name) unless member # reject users who set mention notification in project - next true if member.notification.mention? + next true if member.notification.send(method_name) # reject users who have N_MENTION in project and disabled in global settings - member.notification.global? && user.notification.mention? + member.notification.global? && user.notification.send(method_name) end end def reject_unsubscribed_users(recipients, target) return recipients unless target.respond_to? :subscriptions - + recipients.reject do |user| subscription = target.subscriptions.find_by_user_id(user.id) subscription && !subscription.subscribed @@ -343,7 +333,7 @@ class NotificationService recipients end end - + def new_resource_email(target, project, method) recipients = build_recipients(target, project, target.author) diff --git a/lib/gitlab/markdown/abstract_reference_filter.rb b/lib/gitlab/markdown/abstract_reference_filter.rb new file mode 100644 index 00000000000..fd5b7eb9332 --- /dev/null +++ b/lib/gitlab/markdown/abstract_reference_filter.rb @@ -0,0 +1,100 @@ +require 'gitlab/markdown' + +module Gitlab + module Markdown + # Issues, Snippets and Merge Requests shares similar functionality in refernce filtering. + # All this functionality moved to this class + class AbstractReferenceFilter < ReferenceFilter + include CrossProjectReference + + def self.object_class + # Implement in child class + # Example: MergeRequest + end + + def self.object_name + object_class.name.underscore + end + + def self.object_sym + object_name.to_sym + end + + def self.data_reference + "data-#{object_name.dasherize}" + end + + # Public: Find references in text (like `!123` for merge requests) + # + # AnyReferenceFilter.references_in(text) do |match, object| + # "<a href=...>PREFIX#{object}</a>" + # end + # + # PREFIX - symbol that detects reference (like ! for merge requests) + # object - reference object (snippet, merget request etc) + # text - String text to search. + # + # Yields the String match, the Integer referenced object ID, and an optional String + # of the external project reference. + # + # Returns a String replaced with the return of the block. + def self.references_in(text) + text.gsub(object_class.reference_pattern) do |match| + yield match, $~[object_sym].to_i, $~[:project] + end + end + + def self.referenced_by(node) + { object_sym => LazyReference.new(object_class, node.attr(data_reference)) } + end + + delegate :object_class, :object_sym, :references_in, to: :class + + def find_object(project, id) + # Implement in child class + # Example: project.merge_requests.find + end + + def url_for_object(object, project) + # Implement in child class + # Example: project_merge_request_url + end + + def call + replace_text_nodes_matching(object_class.reference_pattern) do |content| + object_link_filter(content) + end + end + + # Replace references (like `!123` for merge requests) in text with links + # to the referenced object's details page. + # + # text - String text to replace references in. + # + # Returns a String with references replaced with links. All links + # have `gfm` and `gfm-OBJECT_NAME` class names attached for styling. + def object_link_filter(text) + references_in(text) do |match, id, project_ref| + project = project_from_ref(project_ref) + + if project && object = find_object(project, id) + title = escape_once("#{object_title}: #{object.title}") + klass = reference_class(object_sym) + data = data_attribute(project: project.id, object_sym => object.id) + url = url_for_object(object, project) + + %(<a href="#{url}" #{data} + title="#{title}" + class="#{klass}">#{match}</a>) + else + match + end + end + end + + def object_title + object_class.name.titleize + end + end + end +end diff --git a/lib/gitlab/markdown/issue_reference_filter.rb b/lib/gitlab/markdown/issue_reference_filter.rb index 481d282f7b1..1ed69e2f431 100644 --- a/lib/gitlab/markdown/issue_reference_filter.rb +++ b/lib/gitlab/markdown/issue_reference_filter.rb @@ -6,66 +6,17 @@ module Gitlab # issues that do not exist are ignored. # # This filter supports cross-project references. - class IssueReferenceFilter < ReferenceFilter - include CrossProjectReference - - # Public: Find `#123` issue references in text - # - # IssueReferenceFilter.references_in(text) do |match, issue, project_ref| - # "<a href=...>##{issue}</a>" - # end - # - # text - String text to search. - # - # Yields the String match, the Integer issue ID, and an optional String of - # the external project reference. - # - # Returns a String replaced with the return of the block. - def self.references_in(text) - text.gsub(Issue.reference_pattern) do |match| - yield match, $~[:issue].to_i, $~[:project] - end - end - - def self.referenced_by(node) - { issue: LazyReference.new(Issue, node.attr("data-issue")) } - end - - def call - replace_text_nodes_matching(Issue.reference_pattern) do |content| - issue_link_filter(content) - end + class IssueReferenceFilter < AbstractReferenceFilter + def self.object_class + Issue end - # Replace `#123` issue references in text with links to the referenced - # issue's details page. - # - # text - String text to replace references in. - # - # Returns a String with `#123` references replaced with links. All links - # have `gfm` and `gfm-issue` class names attached for styling. - def issue_link_filter(text) - self.class.references_in(text) do |match, id, project_ref| - project = self.project_from_ref(project_ref) - - if project && issue = project.get_issue(id) - url = url_for_issue(id, project, only_path: context[:only_path]) - - title = escape_once("Issue: #{issue.title}") - klass = reference_class(:issue) - data = data_attribute(project: project.id, issue: issue.id) - - %(<a href="#{url}" #{data} - title="#{title}" - class="#{klass}">#{match}</a>) - else - match - end - end + def find_object(project, id) + project.get_issue(id) end - def url_for_issue(*args) - IssuesHelper.url_for_issue(*args) + def url_for_object(issue, project) + IssuesHelper.url_for_issue(issue.iid, project, only_path: context[:only_path]) end end end diff --git a/lib/gitlab/markdown/merge_request_reference_filter.rb b/lib/gitlab/markdown/merge_request_reference_filter.rb index 5bc63269808..1f47f03c94e 100644 --- a/lib/gitlab/markdown/merge_request_reference_filter.rb +++ b/lib/gitlab/markdown/merge_request_reference_filter.rb @@ -6,65 +6,16 @@ module Gitlab # to merge requests that do not exist are ignored. # # This filter supports cross-project references. - class MergeRequestReferenceFilter < ReferenceFilter - include CrossProjectReference - - # Public: Find `!123` merge request references in text - # - # MergeRequestReferenceFilter.references_in(text) do |match, merge_request, project_ref| - # "<a href=...>##{merge_request}</a>" - # end - # - # text - String text to search. - # - # Yields the String match, the Integer merge request ID, and an optional - # String of the external project reference. - # - # Returns a String replaced with the return of the block. - def self.references_in(text) - text.gsub(MergeRequest.reference_pattern) do |match| - yield match, $~[:merge_request].to_i, $~[:project] - end - end - - def self.referenced_by(node) - { merge_request: LazyReference.new(MergeRequest, node.attr("data-merge-request")) } - end - - def call - replace_text_nodes_matching(MergeRequest.reference_pattern) do |content| - merge_request_link_filter(content) - end + class MergeRequestReferenceFilter < AbstractReferenceFilter + def self.object_class + MergeRequest end - # Replace `!123` merge request references in text with links to the - # referenced merge request's details page. - # - # text - String text to replace references in. - # - # Returns a String with `!123` references replaced with links. All links - # have `gfm` and `gfm-merge_request` class names attached for styling. - def merge_request_link_filter(text) - self.class.references_in(text) do |match, id, project_ref| - project = self.project_from_ref(project_ref) - - if project && merge_request = project.merge_requests.find_by(iid: id) - title = escape_once("Merge Request: #{merge_request.title}") - klass = reference_class(:merge_request) - data = data_attribute(project: project.id, merge_request: merge_request.id) - - url = url_for_merge_request(merge_request, project) - - %(<a href="#{url}" #{data} - title="#{title}" - class="#{klass}">#{match}</a>) - else - match - end - end + def find_object(project, id) + project.merge_requests.find_by(iid: id) end - def url_for_merge_request(mr, project) + def url_for_object(mr, project) h = Gitlab::Application.routes.url_helpers h.namespace_project_merge_request_url(project.namespace, project, mr, only_path: context[:only_path]) diff --git a/lib/gitlab/markdown/snippet_reference_filter.rb b/lib/gitlab/markdown/snippet_reference_filter.rb index f783f951711..f7bd07c2a34 100644 --- a/lib/gitlab/markdown/snippet_reference_filter.rb +++ b/lib/gitlab/markdown/snippet_reference_filter.rb @@ -6,65 +6,16 @@ module Gitlab # snippets that do not exist are ignored. # # This filter supports cross-project references. - class SnippetReferenceFilter < ReferenceFilter - include CrossProjectReference - - # Public: Find `$123` snippet references in text - # - # SnippetReferenceFilter.references_in(text) do |match, snippet| - # "<a href=...>$#{snippet}</a>" - # end - # - # text - String text to search. - # - # Yields the String match, the Integer snippet ID, and an optional String - # of the external project reference. - # - # Returns a String replaced with the return of the block. - def self.references_in(text) - text.gsub(Snippet.reference_pattern) do |match| - yield match, $~[:snippet].to_i, $~[:project] - end - end - - def self.referenced_by(node) - { snippet: LazyReference.new(Snippet, node.attr("data-snippet")) } - end - - def call - replace_text_nodes_matching(Snippet.reference_pattern) do |content| - snippet_link_filter(content) - end + class SnippetReferenceFilter < AbstractReferenceFilter + def self.object_class + Snippet end - # Replace `$123` snippet references in text with links to the referenced - # snippets's details page. - # - # text - String text to replace references in. - # - # Returns a String with `$123` references replaced with links. All links - # have `gfm` and `gfm-snippet` class names attached for styling. - def snippet_link_filter(text) - self.class.references_in(text) do |match, id, project_ref| - project = self.project_from_ref(project_ref) - - if project && snippet = project.snippets.find_by(id: id) - title = escape_once("Snippet: #{snippet.title}") - klass = reference_class(:snippet) - data = data_attribute(project: project.id, snippet: snippet.id) - - url = url_for_snippet(snippet, project) - - %(<a href="#{url}" #{data} - title="#{title}" - class="#{klass}">#{match}</a>) - else - match - end - end + def find_object(project, id) + project.snippets.find_by(id: id) end - def url_for_snippet(snippet, project) + def url_for_object(snippet, project) h = Gitlab::Application.routes.url_helpers h.namespace_project_snippet_url(project.namespace, project, snippet, only_path: context[:only_path]) diff --git a/lib/gitlab/markdown/user_reference_filter.rb b/lib/gitlab/markdown/user_reference_filter.rb index 2a594e1662e..ab5e1f6fe9e 100644 --- a/lib/gitlab/markdown/user_reference_filter.rb +++ b/lib/gitlab/markdown/user_reference_filter.rb @@ -85,13 +85,12 @@ module Gitlab def link_to_all project = context[:project] - url = urls.namespace_project_url(project.namespace, project, only_path: context[:only_path]) data = data_attribute(project: project.id) - text = User.reference_prefix + 'all' - %(<a href="#{url}" #{data} class="#{link_class}">#{text}</a>) + + link_tag(url, data, text) end def link_to_namespace(namespace) @@ -105,16 +104,20 @@ module Gitlab def link_to_group(group, namespace) url = urls.group_url(group, only_path: context[:only_path]) data = data_attribute(group: namespace.id) - text = Group.reference_prefix + group - %(<a href="#{url}" #{data} class="#{link_class}">#{text}</a>) + + link_tag(url, data, text) end def link_to_user(user, namespace) url = urls.user_url(user, only_path: context[:only_path]) data = data_attribute(user: namespace.owner_id) - text = User.reference_prefix + user + + link_tag(url, data, text) + end + + def link_tag(url, data, text) %(<a href="#{url}" #{data} class="#{link_class}">#{text}</a>) end end diff --git a/lib/tasks/flay.rake b/lib/tasks/flay.rake index dfb9df4772a..e9587595fef 100644 --- a/lib/tasks/flay.rake +++ b/lib/tasks/flay.rake @@ -1,6 +1,6 @@ desc 'Code duplication analyze via flay' task :flay do - output = %x(bundle exec flay --mass 30 app/ lib/gitlab/) + output = %x(bundle exec flay --mass 35 app/ lib/gitlab/) if output.include? "Similar code found" puts output |