From cd52cef1c0898230cb684ee294bd7a6c5b2f226a Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Thu, 14 May 2015 13:05:33 +0200 Subject: [PATCH 1/5] Fix reference links in dashboard activity and ATOM feeds. --- CHANGELOG | 1 + app/helpers/events_helper.rb | 4 ++-- app/helpers/gitlab_markdown_helper.rb | 14 ++++++++------ app/views/events/_event_issue.atom.haml | 2 +- app/views/events/_event_merge_request.atom.haml | 2 +- app/views/events/_event_note.atom.haml | 2 +- app/views/events/_event_push.atom.haml | 2 +- app/views/events/event/_note.html.haml | 2 +- lib/gitlab/markdown.rb | 16 +++++++++------- lib/redcarpet/render/gitlab_html.rb | 5 ++++- spec/helpers/gitlab_markdown_helper_spec.rb | 2 +- 11 files changed, 30 insertions(+), 22 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index a06509c7c79..890c154fede 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -58,6 +58,7 @@ v 7.11.0 (unreleased) - Fix automatic blocking of auto-created users from Active Directory. - Call merge request web hook for each new commits (Arthur Gautier) - Use SIGKILL by default in Sidekiq::MemoryKiller + - Fix reference links in dashboard activity and ATOM feeds. v 7.10.2 - Fix CI links on MR page diff --git a/app/helpers/events_helper.rb b/app/helpers/events_helper.rb index 18c75a8726b..e55d729fa4f 100644 --- a/app/helpers/events_helper.rb +++ b/app/helpers/events_helper.rb @@ -168,8 +168,8 @@ module EventsHelper end end - def event_note(text) - text = first_line_in_markdown(text, 150) + def event_note(text, options = {}) + text = first_line_in_markdown(text, 150, options) sanitize(text, tags: %w(a img b pre code p span)) end diff --git a/app/helpers/gitlab_markdown_helper.rb b/app/helpers/gitlab_markdown_helper.rb index c309f890d96..846aded4bda 100644 --- a/app/helpers/gitlab_markdown_helper.rb +++ b/app/helpers/gitlab_markdown_helper.rb @@ -19,7 +19,7 @@ module GitlabMarkdownHelper escape_once(body) end - gfm_body = gfm(escaped_body, @project, html_options) + gfm_body = gfm(escaped_body, {}, html_options) gfm_body.gsub!(%r{.*?}m) do |match| "#{match}#{link_to("", url, html_options)[0..-5]}" # "".length +1 @@ -32,11 +32,13 @@ module GitlabMarkdownHelper unless @markdown && options == @options @options = options - # see https://github.com/vmg/redcarpet#darling-i-packed-you-a-couple-renderers-for-lunch - rend = Redcarpet::Render::GitlabHTML.new(self, user_color_scheme_class, { + options.merge!( # Handled further down the line by Gitlab::Markdown::SanitizationFilter escape_html: false - }.merge(options)) + ) + + # see https://github.com/vmg/redcarpet#darling-i-packed-you-a-couple-renderers-for-lunch + rend = Redcarpet::Render::GitlabHTML.new(self, user_color_scheme_class, options) # see https://github.com/vmg/redcarpet#and-its-like-really-simple-to-use @markdown = Redcarpet::Markdown.new(rend, @@ -58,8 +60,8 @@ module GitlabMarkdownHelper # as Markdown. HTML tags in the parsed output are not counted toward the # +max_chars+ limit. If the length limit falls within a tag's contents, then # the tag contents are truncated without removing the closing tag. - def first_line_in_markdown(text, max_chars = nil) - md = markdown(text).strip + def first_line_in_markdown(text, max_chars = nil, options = {}) + md = markdown(text, options).strip truncate_visible(md, max_chars || md.length) if md.present? end diff --git a/app/views/events/_event_issue.atom.haml b/app/views/events/_event_issue.atom.haml index 0edb61ea246..4259f64c191 100644 --- a/app/views/events/_event_issue.atom.haml +++ b/app/views/events/_event_issue.atom.haml @@ -1,3 +1,3 @@ %div{xmlns: "http://www.w3.org/1999/xhtml"} - if issue.description.present? - = markdown(issue.description, xhtml: true) + = markdown(issue.description, xhtml: true, reference_only_path: false, project: issue.project) diff --git a/app/views/events/_event_merge_request.atom.haml b/app/views/events/_event_merge_request.atom.haml index 1a8b62abeab..e8ed13df783 100644 --- a/app/views/events/_event_merge_request.atom.haml +++ b/app/views/events/_event_merge_request.atom.haml @@ -1,3 +1,3 @@ %div{xmlns: "http://www.w3.org/1999/xhtml"} - if merge_request.description.present? - = markdown(merge_request.description, xhtml: true) + = markdown(merge_request.description, xhtml: true, reference_only_path: false, project: merge_request.project) diff --git a/app/views/events/_event_note.atom.haml b/app/views/events/_event_note.atom.haml index b49c331ccf2..cfbfba50202 100644 --- a/app/views/events/_event_note.atom.haml +++ b/app/views/events/_event_note.atom.haml @@ -1,2 +1,2 @@ %div{xmlns: "http://www.w3.org/1999/xhtml"} - = markdown(note.note, xhtml: true) + = markdown(note.note, xhtml: true, reference_only_path: false, project: note.project) diff --git a/app/views/events/_event_push.atom.haml b/app/views/events/_event_push.atom.haml index 5d14def8f75..42762e04b51 100644 --- a/app/views/events/_event_push.atom.haml +++ b/app/views/events/_event_push.atom.haml @@ -6,7 +6,7 @@ %i at = commit[:timestamp].to_time.to_s(:short) - %blockquote= markdown(escape_once(commit[:message]), xhtml: true) + %blockquote= markdown(escape_once(commit[:message]), xhtml: true, reference_only_path: false, project: note.project) - if event.commits_count > 15 %p %i diff --git a/app/views/events/event/_note.html.haml b/app/views/events/event/_note.html.haml index 4ef18c09060..07bec1697f5 100644 --- a/app/views/events/event/_note.html.haml +++ b/app/views/events/event/_note.html.haml @@ -14,7 +14,7 @@ .event-note .md %i.fa.fa-comment-o.event-note-icon - = event_note(event.target.note) + = event_note(event.target.note, project: event.project) - note = event.target - if note.attachment.url - if note.attachment.image? diff --git a/lib/gitlab/markdown.rb b/lib/gitlab/markdown.rb index cc68416d5fc..4b45f239ce5 100644 --- a/lib/gitlab/markdown.rb +++ b/lib/gitlab/markdown.rb @@ -24,10 +24,10 @@ module Gitlab # Public: Parse the provided text with GitLab-Flavored Markdown # # text - the source text - # project - the project + # options - options # html_options - extra options for the reference links as given to link_to - def gfm(text, project = @project, html_options = {}) - gfm_with_options(text, {}, project, html_options) + def gfm(text, options = {}, html_options = {}) + gfm_with_options(text, options, html_options) end # Public: Parse the provided text with GitLab-Flavored Markdown @@ -38,7 +38,7 @@ module Gitlab # :reference_only_path - Use relative path for reference links # project - the project # html_options - extra options for the reference links as given to link_to - def gfm_with_options(text, options = {}, project = @project, html_options = {}) + def gfm_with_options(text, options = {}, html_options = {}) return text if text.nil? # Duplicate the string so we don't alter the original, then call to_str @@ -48,7 +48,9 @@ module Gitlab options.reverse_merge!( xhtml: false, - reference_only_path: true + reference_only_path: true, + project: @project, + current_user: current_user ) pipeline = HTML::Pipeline.new(filters) @@ -62,9 +64,9 @@ module Gitlab no_header_anchors: options[:no_header_anchors], # ReferenceFilter - current_user: current_user, + current_user: options[:current_user], only_path: options[:reference_only_path], - project: project, + project: options[:project], reference_class: html_options[:class], # RelativeLinkFilter diff --git a/lib/redcarpet/render/gitlab_html.rb b/lib/redcarpet/render/gitlab_html.rb index 15eb6effe08..7dcecc2ecf6 100644 --- a/lib/redcarpet/render/gitlab_html.rb +++ b/lib/redcarpet/render/gitlab_html.rb @@ -7,9 +7,12 @@ class Redcarpet::Render::GitlabHTML < Redcarpet::Render::HTML def initialize(template, color_scheme, options = {}) @template = template @color_scheme = color_scheme - @project = @template.instance_variable_get("@project") @options = options.dup + @options.reverse_merge!( + project: @template.instance_variable_get("@project") + ) + super(options) end diff --git a/spec/helpers/gitlab_markdown_helper_spec.rb b/spec/helpers/gitlab_markdown_helper_spec.rb index 7d0335c2320..9f3e8cf585e 100644 --- a/spec/helpers/gitlab_markdown_helper_spec.rb +++ b/spec/helpers/gitlab_markdown_helper_spec.rb @@ -21,7 +21,7 @@ describe GitlabMarkdownHelper do describe "#gfm" do it "should forward HTML options to links" do - expect(gfm("Fixed in #{commit.id}", @project, class: 'foo')). + expect(gfm("Fixed in #{commit.id}", { project: @project }, class: 'foo')). to have_selector('a.gfm.foo') end From 0b098060cc2d11ec1350ace79dc111972a511990 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Thu, 14 May 2015 13:26:58 +0200 Subject: [PATCH 2/5] Spin icon next to "Checking for CI status..." --- CHANGELOG | 1 + .../projects/merge_requests/show/_mr_ci.html.haml | 12 ++++++------ 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index a06509c7c79..dabaa7377e2 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -58,6 +58,7 @@ v 7.11.0 (unreleased) - Fix automatic blocking of auto-created users from Active Directory. - Call merge request web hook for each new commits (Arthur Gautier) - Use SIGKILL by default in Sidekiq::MemoryKiller + - Spin spinner icon next to "Checking for CI status..." on MR page. v 7.10.2 - Fix CI links on MR page diff --git a/app/views/projects/merge_requests/show/_mr_ci.html.haml b/app/views/projects/merge_requests/show/_mr_ci.html.haml index ffa3f7b0e36..3b1cd53df37 100644 --- a/app/views/projects/merge_requests/show/_mr_ci.html.haml +++ b/app/views/projects/merge_requests/show/_mr_ci.html.haml @@ -1,34 +1,34 @@ - if @commits.any? .ci_widget.ci-success{style: "display:none"} - %i.fa.fa-check + = icon("check") %span CI build passed for #{@merge_request.last_commit_short_sha}. = link_to "View build page", ci_build_details_path(@merge_request), :"data-no-turbolink" => "data-no-turbolink" .ci_widget.ci-failed{style: "display:none"} - %i.fa.fa-times + = icon("times") %span CI build failed for #{@merge_request.last_commit_short_sha}. = link_to "View build page", ci_build_details_path(@merge_request), :"data-no-turbolink" => "data-no-turbolink" - [:running, :pending].each do |status| .ci_widget{class: "ci-#{status}", style: "display:none"} - %i.fa.fa-clock-o + = icon("clock-o") %span CI build #{status} for #{@merge_request.last_commit_short_sha}. = link_to "View build page", ci_build_details_path(@merge_request), :"data-no-turbolink" => "data-no-turbolink" .ci_widget - %i.fa.fa-spinner + = icon("spinner spin") Checking for CI status for #{@merge_request.last_commit_short_sha} .ci_widget.ci-canceled{style: "display:none"} - %i.fa.fa-times + = icon("times") %span CI build canceled for #{@merge_request.last_commit_short_sha}. = link_to "View build page", ci_build_details_path(@merge_request), :"data-no-turbolink" => "data-no-turbolink" .ci_widget.ci-error{style: "display:none"} - %i.fa.fa-times + = icon("times") %span Cannot connect to the CI server. Please check your settings and try again. From 3d4d89f060f01b7993ef8e1839744c81b53293bf Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Thu, 14 May 2015 18:30:23 +0300 Subject: [PATCH 3/5] Use count badges with number_with_delimiter helper for issue, commit count --- app/helpers/application_helper.rb | 17 +++++++++++++---- app/views/projects/commits/_head.html.haml | 2 +- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 672be54e66f..26bb3c66fd6 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -324,12 +324,21 @@ module ApplicationHelper count = if project.nil? - "" + nil elsif current_controller?(:issues) - " (#{project.issues.send(entity).count})" + project.issues.send(entity).count elsif current_controller?(:merge_requests) - " (#{project.merge_requests.send(entity).count})" + project.merge_requests.send(entity).count end - "#{entity_title}#{count}" + + html = "" + html += content_tag :span, entity_title + html += " " + + if count.present? + html += content_tag :span, number_with_delimiter(count), class: 'badge' + end + + html.html_safe end end diff --git a/app/views/projects/commits/_head.html.haml b/app/views/projects/commits/_head.html.haml index 66261c7336d..e3d8cd0fdd5 100644 --- a/app/views/projects/commits/_head.html.haml +++ b/app/views/projects/commits/_head.html.haml @@ -3,7 +3,7 @@ = link_to namespace_project_commits_path(@project.namespace, @project, @ref || @repository.root_ref) do = icon("history") Commits - %span.badge= number_with_precision(@repository.commit_count, precision: 0, delimiter: ',') + %span.badge= number_with_delimiter(@repository.commit_count) = nav_link(controller: :compare) do = link_to namespace_project_compare_index_path(@project.namespace, @project, from: @repository.root_ref, to: @ref || @repository.root_ref) do = icon("exchange") From 40f381e6527f6eac3f4ccbbd7cfde690f2b40be8 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Thu, 14 May 2015 18:32:07 +0300 Subject: [PATCH 4/5] append empty space only if count exists --- app/helpers/application_helper.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 26bb3c66fd6..3d4588a3b5d 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -333,9 +333,9 @@ module ApplicationHelper html = "" html += content_tag :span, entity_title - html += " " if count.present? + html += " " html += content_tag :span, number_with_delimiter(count), class: 'badge' end From ded82ed4c9fde4e439a0da01138c6a3d9c1c991e Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Thu, 14 May 2015 18:34:06 +0300 Subject: [PATCH 5/5] Small refactoring of state_filters_text_for helper --- app/helpers/application_helper.rb | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 3d4588a3b5d..ea9722b9bef 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -331,11 +331,10 @@ module ApplicationHelper project.merge_requests.send(entity).count end - html = "" - html += content_tag :span, entity_title + html = content_tag :span, entity_title if count.present? - html += " " + html += " " html += content_tag :span, number_with_delimiter(count), class: 'badge' end