diff options
38 files changed, 423 insertions, 472 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/CHANGELOG b/CHANGELOG index f7c6b09e7b2..cc9fcf1992b 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,6 +1,8 @@ Please view this file on the master branch, on stable branches it's out of date. -v 8.2.0 (unreleased) +v 8.3.0 (unreleased) + +v 8.2.0 - Remove CSS property preventing hard tabs from rendering in Chromium 45 (Stan Hu) - Fix Drone CI service template not saving properly (Stan Hu) - Fix avatars not showing in Atom feeds and project issues when Gravatar disabled (Stan Hu) @@ -1 +1 @@ -8.1.0.pre +8.3.0.pre diff --git a/app/assets/javascripts/notes.js.coffee b/app/assets/javascripts/notes.js.coffee index b0682f16845..ea75c656bcc 100644 --- a/app/assets/javascripts/notes.js.coffee +++ b/app/assets/javascripts/notes.js.coffee @@ -29,6 +29,7 @@ class @Notes $(document).on "ajax:success", "form.edit_note", @updateNote # Edit note link + $(document).on "click", ".js-note-edit", @showEditForm $(document).on "click", ".note-edit-cancel", @cancelEdit # Reopen and close actions for Issue/MR combined with note form submit @@ -66,6 +67,7 @@ class @Notes $(document).off "ajax:success", ".js-main-target-form" $(document).off "ajax:success", ".js-discussion-note-form" $(document).off "ajax:success", "form.edit_note" + $(document).off "click", ".js-note-edit" $(document).off "click", ".note-edit-cancel" $(document).off "click", ".js-note-delete" $(document).off "click", ".js-note-attachment-delete" @@ -285,14 +287,13 @@ class @Notes Adds a hidden div with the original content of the note to fill the edit note form with if the user cancels ### - showEditForm: (note, formHTML) -> - nodeText = note.find(".note-text"); - nodeText.hide() - note.find('.note-edit-form').remove() - nodeText.after(formHTML) + showEditForm: (e) -> + e.preventDefault() + note = $(this).closest(".note") note.find(".note-body > .note-text").hide() note.find(".note-header").hide() - form = note.find(".note-edit-form") + base_form = note.find(".note-edit-form") + form = base_form.clone().insertAfter(base_form) form.addClass('current-note-edit-form gfm-form') form.find('.div-dropzone').remove() 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/controllers/projects/notes_controller.rb b/app/controllers/projects/notes_controller.rb index 0c98e2f1bfd..41cd08c93c6 100644 --- a/app/controllers/projects/notes_controller.rb +++ b/app/controllers/projects/notes_controller.rb @@ -3,7 +3,7 @@ class Projects::NotesController < Projects::ApplicationController before_action :authorize_read_note! before_action :authorize_create_note!, only: [:create] before_action :authorize_admin_note!, only: [:update, :destroy] - before_action :find_current_user_notes, except: [:destroy, :edit, :delete_attachment] + before_action :find_current_user_notes, except: [:destroy, :delete_attachment] def index current_fetched_at = Time.now.to_i @@ -29,11 +29,6 @@ class Projects::NotesController < Projects::ApplicationController end end - def edit - @note = note - render layout: false - end - def update @note = Notes::UpdateService.new(project, current_user, note_params).execute(note) diff --git a/app/helpers/diff_helper.rb b/app/helpers/diff_helper.rb index b889fb28973..b715ecd85d2 100644 --- a/app/helpers/diff_helper.rb +++ b/app/helpers/diff_helper.rb @@ -1,4 +1,8 @@ module DiffHelper + def diff_view + params[:view] == 'parallel' ? 'parallel' : 'inline' + end + def allowed_diff_size if diff_hard_limit_enabled? Commit::DIFF_HARD_LIMIT_FILES @@ -132,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: (params[:view] != 'parallel' ? '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: (params[: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) @@ -187,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/app/views/projects/commit/show.html.haml b/app/views/projects/commit/show.html.haml index 30a3973828f..85e203cbe57 100644 --- a/app/views/projects/commit/show.html.haml +++ b/app/views/projects/commit/show.html.haml @@ -3,4 +3,4 @@ = render "commit_box" = render "ci_menu" if @ci_commit = render "projects/diffs/diffs", diffs: @diffs, project: @project -= render "projects/notes/notes_with_form", view: params[:view] += render "projects/notes/notes_with_form" diff --git a/app/views/projects/diffs/_diffs.html.haml b/app/views/projects/diffs/_diffs.html.haml index e46bf1ab1e7..416fb4da071 100644 --- a/app/views/projects/diffs/_diffs.html.haml +++ b/app/views/projects/diffs/_diffs.html.haml @@ -1,4 +1,4 @@ -- if params[:view] == 'parallel' +- if diff_view == 'parallel' - fluid_layout true - diff_files = safe_diff_files(diffs) diff --git a/app/views/projects/diffs/_file.html.haml b/app/views/projects/diffs/_file.html.haml index 410ff6abb43..c745b4e69bf 100644 --- a/app/views/projects/diffs/_file.html.haml +++ b/app/views/projects/diffs/_file.html.haml @@ -33,7 +33,7 @@ -# Skipp all non non-supported blobs - return unless blob.respond_to?('text?') - if blob.text? - - if params[:view] == 'parallel' + - if diff_view == 'parallel' = render "projects/diffs/parallel_view", diff_file: diff_file, project: project, blob: blob, index: i - else = render "projects/diffs/text_file", diff_file: diff_file, index: i @@ -42,4 +42,3 @@ = render "projects/diffs/image", diff_file: diff_file, old_file: old_file, file: blob, index: i - else .nothing-here-block No preview for this file type - diff --git a/app/views/projects/notes/_form.html.haml b/app/views/projects/notes/_form.html.haml index 13dfa0a1bb3..5dd84317e3b 100644 --- a/app/views/projects/notes/_form.html.haml +++ b/app/views/projects/notes/_form.html.haml @@ -1,5 +1,5 @@ = form_for [@project.namespace.becomes(Namespace), @project, @note], remote: true, html: { :'data-type' => 'json', multipart: true, id: nil, class: "new_note js-new-note-form common-note-form gfm-form" }, authenticity_token: true do |f| - = hidden_field_tag :view, params[:view] + = hidden_field_tag :view, diff_view = hidden_field_tag :line_type = note_target_fields(@note) = f.hidden_field :commit_id diff --git a/app/views/projects/notes/_note.html.haml b/app/views/projects/notes/_note.html.haml index 8a3292f7daf..efa7dd01cc2 100644 --- a/app/views/projects/notes/_note.html.haml +++ b/app/views/projects/notes/_note.html.haml @@ -7,7 +7,7 @@ .note-header - if note_editable?(note) .note-actions - = link_to edit_namespace_project_note_path(note.project.namespace, note.project, note), title: 'Edit comment', remote: true, class: 'js-note-edit' do + = link_to '#', title: 'Edit comment', class: 'js-note-edit' do = icon('pencil-square-o') = link_to namespace_project_note_path(note.project.namespace, note.project, note), title: 'Remove comment', method: :delete, data: { confirm: 'Are you sure you want to remove this comment?' }, remote: true, class: 'js-note-delete danger' do @@ -59,6 +59,8 @@ .note-text = preserve do = markdown(note.note, {no_header_anchors: true}) + - if note_editable?(note) + = render 'projects/notes/edit_form', note: note - if note.attachment.url .note-attachment diff --git a/app/views/projects/notes/_notes_with_form.html.haml b/app/views/projects/notes/_notes_with_form.html.haml index 91cefa6d14d..99c1b0fa43e 100644 --- a/app/views/projects/notes/_notes_with_form.html.haml +++ b/app/views/projects/notes/_notes_with_form.html.haml @@ -4,7 +4,7 @@ .js-main-target-form - if can? current_user, :create_note, @project - = render "projects/notes/form", view: params[:view] + = render "projects/notes/form", view: diff_view :javascript - window._notes = new Notes("#{namespace_project_notes_path(namespace_id: @project.namespace, target_id: @noteable.id, target_type: @noteable.class.name.underscore)}", #{@notes.map(&:id).to_json}, #{Time.now.to_i}, "#{params[:view]}") + new Notes("#{namespace_project_notes_path(namespace_id: @project.namespace, target_id: @noteable.id, target_type: @noteable.class.name.underscore)}", #{@notes.map(&:id).to_json}, #{Time.now.to_i}, "#{diff_view}") diff --git a/app/views/projects/notes/edit.js.erb b/app/views/projects/notes/edit.js.erb deleted file mode 100644 index 2599bad5d6e..00000000000 --- a/app/views/projects/notes/edit.js.erb +++ /dev/null @@ -1,2 +0,0 @@ -$note = $('.note-row-<%= @note.id %>:visible'); -_notes.showEditForm($note, '<%= escape_javascript(render('edit_form', note: @note)) %>'); diff --git a/config/routes.rb b/config/routes.rb index c0077ab1a99..0bc2c173453 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -660,7 +660,7 @@ Gitlab::Application.routes.draw do end end - resources :notes, constraints: { id: /\d+/ } do + resources :notes, only: [:index, :create, :destroy, :update], constraints: { id: /\d+/ } do member do delete :delete_attachment end diff --git a/doc/release/monthly.md b/doc/release/monthly.md index d347f58ba0f..c9ab87671d2 100644 --- a/doc/release/monthly.md +++ b/doc/release/monthly.md @@ -54,21 +54,25 @@ template are explained below: - [ ] Update GitLab.com with RC1 - [ ] Create the regression issue in the CE issue tracker: - > This is a meta issue to index possible regressions in this monthly release - > and any patch versions. - > - > Please do not raise or discuss issues directly in this issue but link to - > issues that might warrant a patch release. If there is a Merge Request - > that fixes the issue, please link to that as well. - > - > Please only post one regression issue and/or merge request per comment. - > Comments will be updated by the release manager as they are addressed. + ``` + This is a meta issue to index possible regressions in this monthly release + and any patch versions. + + Please do not raise or discuss issues directly in this issue but link to + issues that might warrant a patch release. If there is a Merge Request + that fixes the issue, please link to that as well. + + Please only post one regression issue and/or merge request per comment. + Comments will be updated by the release manager as they are addressed. + ``` - [ ] Tweet about RC1 release: - > GitLab x.y.0.rc1 is available: https://packages.gitlab.com/gitlab/unstable - > Use at your own risk. Please link regressions issues from - > LINK_TO_REGRESSION_ISSUE + ``` + GitLab x.y.0.rc1 is available: https://packages.gitlab.com/gitlab/unstable + Use at your own risk. Please link regressions issues from + LINK_TO_REGRESSION_ISSUE + ``` ### Xth: (3 working days before the 22nd) diff --git a/features/steps/project/issues/issues.rb b/features/steps/project/issues/issues.rb index be134b9c2bb..af2da41badb 100644 --- a/features/steps/project/issues/issues.rb +++ b/features/steps/project/issues/issues.rb @@ -219,7 +219,7 @@ class Spinach::Features::ProjectIssues < Spinach::FeatureSteps end step 'The code block should be unchanged' do - expect(page).to have_content("Command [1]: /usr/local/bin/git , see [text](doc/text)") + expect(page).to have_content("```\nCommand [1]: /usr/local/bin/git , see [text](doc/text)\n```") end step 'project \'Shop\' has issue \'Bugfix1\' with description: \'Description for issue1\'' do 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 diff --git a/spec/features/notes_on_merge_requests_spec.rb b/spec/features/notes_on_merge_requests_spec.rb index 16d5a03e88c..d7cb3b2e86e 100644 --- a/spec/features/notes_on_merge_requests_spec.rb +++ b/spec/features/notes_on_merge_requests_spec.rb @@ -65,6 +65,12 @@ describe 'Comments', feature: true do end describe 'when editing a note', js: true do + it 'should contain the hidden edit form' do + page.within("#note_#{note.id}") do + is_expected.to have_css('.note-edit-form', visible: false) + end + end + describe 'editing the note' do before do find('.note').hover diff --git a/spec/features/task_lists_spec.rb b/spec/features/task_lists_spec.rb index 6cbe685a93b..fca3c77fc64 100644 --- a/spec/features/task_lists_spec.rb +++ b/spec/features/task_lists_spec.rb @@ -51,6 +51,7 @@ feature 'Task Lists', feature: true do expect(page).to have_selector(container) expect(page).to have_selector("#{container} .wiki .task-list .task-list-item .task-list-item-checkbox") + expect(page).to have_selector("#{container} .js-task-list-field") expect(page).to have_selector('form.js-issuable-update') expect(page).to have_selector('a.btn-close') end @@ -89,6 +90,7 @@ feature 'Task Lists', feature: true do expect(page).to have_selector('.note .js-task-list-container') expect(page).to have_selector('.note .js-task-list-container .task-list .task-list-item .task-list-item-checkbox') + expect(page).to have_selector('.note .js-task-list-container .js-task-list-field') end it 'is only editable by author' do @@ -125,6 +127,7 @@ feature 'Task Lists', feature: true do expect(page).to have_selector(container) expect(page).to have_selector("#{container} .wiki .task-list .task-list-item .task-list-item-checkbox") + expect(page).to have_selector("#{container} .js-task-list-field") expect(page).to have_selector('form.js-issuable-update') expect(page).to have_selector('a.btn-close') end |