Enable CacheMarkdownField for the remaining models
This commit alters views for the following models to use the markdown cache if present: * AbuseReport * Appearance * ApplicationSetting * BroadcastMessage * Group * Issue * Label * MergeRequest * Milestone * Project At the same time, calls to `escape_once` have been moved into the `single_line` Banzai pipeline, so they can't be missed out by accident and the work is done at save, rather than render, time.
This commit is contained in:
		
							parent
							
								
									dd159a750b
								
							
						
					
					
						commit
						9920551536
					
				|  | @ -37,7 +37,7 @@ class Admin::BroadcastMessagesController < Admin::ApplicationController | |||
|   end | ||||
| 
 | ||||
|   def preview | ||||
|     @message = broadcast_message_params[:message] | ||||
|     @broadcast_message = BroadcastMessage.new(broadcast_message_params) | ||||
|   end | ||||
| 
 | ||||
|   protected | ||||
|  |  | |||
|  | @ -16,7 +16,7 @@ module AppearancesHelper | |||
|   end | ||||
| 
 | ||||
|   def brand_text | ||||
|     markdown(brand_item.description) | ||||
|     markdown_field(brand_item, :description) | ||||
|   end | ||||
| 
 | ||||
|   def brand_item | ||||
|  |  | |||
|  | @ -11,18 +11,6 @@ module ApplicationSettingsHelper | |||
|     current_application_settings.signin_enabled? | ||||
|   end | ||||
| 
 | ||||
|   def extra_sign_in_text | ||||
|     current_application_settings.sign_in_text | ||||
|   end | ||||
| 
 | ||||
|   def after_sign_up_text | ||||
|     current_application_settings.after_sign_up_text | ||||
|   end | ||||
| 
 | ||||
|   def shared_runners_text | ||||
|     current_application_settings.shared_runners_text | ||||
|   end | ||||
| 
 | ||||
|   def user_oauth_applications? | ||||
|     current_application_settings.user_oauth_applications | ||||
|   end | ||||
|  |  | |||
|  | @ -3,7 +3,7 @@ module BroadcastMessagesHelper | |||
|     return unless message.present? | ||||
| 
 | ||||
|     content_tag :div, class: 'broadcast-message', style: broadcast_message_style(message) do | ||||
|       icon('bullhorn') << ' ' << render_broadcast_message(message.message) | ||||
|       icon('bullhorn') << ' ' << render_broadcast_message(message) | ||||
|     end | ||||
|   end | ||||
| 
 | ||||
|  | @ -32,7 +32,7 @@ module BroadcastMessagesHelper | |||
|     end | ||||
|   end | ||||
| 
 | ||||
|   def render_broadcast_message(message) | ||||
|     Banzai.render(message, pipeline: :broadcast_message).html_safe | ||||
|   def render_broadcast_message(broadcast_message) | ||||
|     Banzai.render_field(broadcast_message, :message).html_safe | ||||
|   end | ||||
| end | ||||
|  |  | |||
|  | @ -13,14 +13,12 @@ module GitlabMarkdownHelper | |||
|   def link_to_gfm(body, url, html_options = {}) | ||||
|     return "" if body.blank? | ||||
| 
 | ||||
|     escaped_body = if body.start_with?('<img') | ||||
|                      body | ||||
|                    else | ||||
|                      escape_once(body) | ||||
|                    end | ||||
| 
 | ||||
|     user = current_user if defined?(current_user) | ||||
|     gfm_body = Banzai.render(escaped_body, project: @project, current_user: user, pipeline: :single_line) | ||||
|     context = { | ||||
|       project: @project, | ||||
|       current_user: (current_user if defined?(current_user)), | ||||
|       pipeline: :single_line, | ||||
|     } | ||||
|     gfm_body = Banzai.render(body, context) | ||||
| 
 | ||||
|     fragment = Nokogiri::HTML::DocumentFragment.parse(gfm_body) | ||||
|     if fragment.children.size == 1 && fragment.children[0].name == 'a' | ||||
|  |  | |||
|  | @ -21,7 +21,7 @@ | |||
|   %td | ||||
|     %strong.subheading.visible-xs-block.visible-sm-block Message | ||||
|     .message | ||||
|       = markdown(abuse_report.message.squish!, pipeline: :single_line, author: reporter) | ||||
|       = markdown_field(abuse_report, :message) | ||||
|   %td | ||||
|     - if user | ||||
|       = link_to 'Remove user & report', admin_abuse_report_path(abuse_report, remove_user: true), | ||||
|  |  | |||
|  | @ -1,7 +1,10 @@ | |||
| .broadcast-message-preview{ style: broadcast_message_style(@broadcast_message) } | ||||
|   = icon('bullhorn') | ||||
|   .js-broadcast-message-preview | ||||
|     = render_broadcast_message(@broadcast_message.message.presence || "Your message here") | ||||
|     - if @broadcast_message.message.present? | ||||
|       = render_broadcast_message(@broadcast_message) | ||||
|     - else | ||||
|       = "Your message here" | ||||
| 
 | ||||
| = form_for [:admin, @broadcast_message], html: { class: 'broadcast-message-form form-horizontal js-quick-submit js-requires-input'} do |f| | ||||
|   = form_errors(@broadcast_message) | ||||
|  |  | |||
|  | @ -1 +1 @@ | |||
| $('.js-broadcast-message-preview').html("#{j(render_broadcast_message(@message))}"); | ||||
| $('.js-broadcast-message-preview').html("#{j(render_broadcast_message(@broadcast_message))}"); | ||||
|  |  | |||
|  | @ -23,4 +23,4 @@ | |||
| 
 | ||||
|   - if group.description.present? | ||||
|     .description | ||||
|       = markdown(group.description, pipeline: :description) | ||||
|       = markdown_field(group, :description) | ||||
|  |  | |||
|  | @ -1,7 +1,7 @@ | |||
| %li{id: dom_id(label)} | ||||
|   .label-row | ||||
|     = render_colored_label(label, tooltip: false) | ||||
|     = markdown(label.description, pipeline: :single_line) | ||||
|     = markdown_field(label, :description) | ||||
|     .pull-right | ||||
|       = link_to 'Edit', edit_admin_label_path(label), class: 'btn btn-sm' | ||||
|       = link_to 'Delete', admin_label_path(label), class: 'btn btn-sm btn-remove remove-row', method: :delete, remote: true, data: {confirm: "Delete this label? Are you sure?"} | ||||
|  |  | |||
|  | @ -87,7 +87,7 @@ | |||
| 
 | ||||
|             - if project.description.present? | ||||
|               .description | ||||
|                 = markdown(project.description, pipeline: :description) | ||||
|                 = markdown_field(project, :description) | ||||
| 
 | ||||
|       = paginate @projects, theme: 'gitlab' | ||||
|     - else | ||||
|  |  | |||
|  | @ -3,9 +3,9 @@ | |||
|     Almost there... | ||||
|   %p.lead | ||||
|     Please check your email to confirm your account | ||||
| - if after_sign_up_text.present? | ||||
| - if current_application_settings.after_sign_up_text.present? | ||||
|   .well-confirmation.text-center | ||||
|     = markdown(after_sign_up_text) | ||||
|     = markdown_field(current_application_settings, :after_sign_up_text) | ||||
| %p.confirmation-content.text-center | ||||
|   No confirmation email received? Please check your spam folder or | ||||
| .append-bottom-20.prepend-top-20.text-center | ||||
|  |  | |||
|  | @ -21,7 +21,7 @@ | |||
| 
 | ||||
|       - if @group.description.present? | ||||
|         .cover-desc.description | ||||
|           = markdown(@group.description, pipeline: :description) | ||||
|           = markdown_field(@group, :description) | ||||
| 
 | ||||
| %div.groups-header{ class: container_class } | ||||
|   .top-area | ||||
|  |  | |||
|  | @ -20,7 +20,7 @@ | |||
|     Read more about GitLab at #{link_to promo_host, promo_url, target: '_blank'}. | ||||
|     - if current_application_settings.help_page_text.present? | ||||
|       %hr | ||||
|       = markdown(current_application_settings.help_page_text) | ||||
|       = markdown_field(current_application_settings, :help_page_text) | ||||
| 
 | ||||
| %hr | ||||
| 
 | ||||
|  |  | |||
|  | @ -25,8 +25,8 @@ | |||
|                 Perform code reviews and enhance collaboration with merge requests. | ||||
|                 Each project can also have an issue tracker and a wiki. | ||||
| 
 | ||||
|             - if extra_sign_in_text.present? | ||||
|               = markdown(extra_sign_in_text) | ||||
|             - if current_application_settings.sign_in_text.present? | ||||
|               = markdown_field(current_application_settings, :sign_in_text) | ||||
| 
 | ||||
|     %hr | ||||
|     .container | ||||
|  |  | |||
|  | @ -9,7 +9,7 @@ | |||
| 
 | ||||
|     .project-home-desc | ||||
|       - if @project.description.present? | ||||
|         = markdown(@project.description, pipeline: :description) | ||||
|         = markdown_field(@project, :description) | ||||
| 
 | ||||
|       - if forked_from_project = @project.forked_from_project | ||||
|         %p | ||||
|  |  | |||
|  | @ -65,10 +65,10 @@ | |||
| 
 | ||||
| .commit-box.content-block | ||||
|   %h3.commit-title | ||||
|     = markdown escape_once(@commit.title), pipeline: :single_line, author: @commit.author | ||||
|     = markdown(@commit.title, pipeline: :single_line, author: @commit.author) | ||||
|   - if @commit.description.present? | ||||
|     %pre.commit-description | ||||
|       = preserve(markdown(escape_once(@commit.description), pipeline: :single_line, author: @commit.author)) | ||||
|       = preserve(markdown(@commit.description, pipeline: :single_line, author: @commit.author)) | ||||
| 
 | ||||
| :javascript | ||||
|   $(".commit-info.branches").load("#{branches_namespace_project_commit_path(@project.namespace, @project, @commit.id)}"); | ||||
|  |  | |||
|  | @ -33,7 +33,7 @@ | |||
| 
 | ||||
|       - if commit.description? | ||||
|         %pre.commit-row-description.js-toggle-content | ||||
|           = preserve(markdown(escape_once(commit.description), pipeline: :single_line, author: commit.author)) | ||||
|           = preserve(markdown(commit.description, pipeline: :single_line, author: commit.author)) | ||||
| 
 | ||||
|       .commit-row-info | ||||
|         = commit_author_link(commit, avatar: false, size: 24) | ||||
|  |  | |||
|  | @ -55,12 +55,12 @@ | |||
| .issue-details.issuable-details | ||||
|   .detail-page-description.content-block | ||||
|     %h2.title | ||||
|       = markdown escape_once(@issue.title), pipeline: :single_line, author: @issue.author | ||||
|       = markdown_field(@issue, :title) | ||||
|     - if @issue.description.present? | ||||
|       .description{ class: can?(current_user, :update_issue, @issue) ? 'js-task-list-container' : '' } | ||||
|         .wiki | ||||
|           = preserve do | ||||
|             = markdown(@issue.description, cache_key: [@issue, "description"], author: @issue.author) | ||||
|             = markdown_field(@issue, :description) | ||||
|         %textarea.hidden.js-task-list-field | ||||
|           = @issue.description | ||||
|     = edited_time_ago_with_tooltip(@issue, placement: 'bottom', html_class: 'issue_edited_ago') | ||||
|  |  | |||
|  | @ -1,13 +1,13 @@ | |||
| .detail-page-description.content-block | ||||
|   %h2.title | ||||
|     = markdown escape_once(@merge_request.title), pipeline: :single_line, author: @merge_request.author | ||||
|     = markdown_field(@merge_request, :title) | ||||
| 
 | ||||
|   %div | ||||
|     - if @merge_request.description.present? | ||||
|       .description{class: can?(current_user, :update_merge_request, @merge_request) ? 'js-task-list-container' : ''} | ||||
|         .wiki | ||||
|           = preserve do | ||||
|             = markdown(@merge_request.description, cache_key: [@merge_request, "description"], author: @merge_request.author) | ||||
|             = markdown_field(@merge_request, :description) | ||||
|         %textarea.hidden.js-task-list-field | ||||
|           = @merge_request.description | ||||
| 
 | ||||
|  |  | |||
|  | @ -30,13 +30,13 @@ | |||
| 
 | ||||
| .detail-page-description.milestone-detail | ||||
|   %h2.title | ||||
|     = markdown escape_once(@milestone.title), pipeline: :single_line | ||||
|     = markdown_field(@milestone, :title) | ||||
|   %div | ||||
|     - if @milestone.description.present? | ||||
|       .description | ||||
|         .wiki | ||||
|           = preserve do | ||||
|             = markdown @milestone.description | ||||
|             = markdown_field(@milestone, :description) | ||||
| 
 | ||||
| - if @milestone.total_items_count(current_user).zero? | ||||
|   .alert.alert-success.prepend-top-default | ||||
|  |  | |||
|  | @ -33,7 +33,7 @@ | |||
| - if @commit | ||||
|   .commit-box.content-block | ||||
|     %h3.commit-title | ||||
|       = markdown escape_once(@commit.title), pipeline: :single_line | ||||
|       = markdown(@commit.title, pipeline: :single_line) | ||||
|     - if @commit.description.present? | ||||
|       %pre.commit-description | ||||
|         = preserve(markdown(escape_once(@commit.description), pipeline: :single_line)) | ||||
|         = preserve(markdown(@commit.description, pipeline: :single_line)) | ||||
|  |  | |||
|  | @ -12,7 +12,7 @@ | |||
|       = link_to namespace_project_commits_path(@project.namespace, @project, commit.id) do | ||||
|         %code= commit.short_id | ||||
|       = image_tag avatar_icon(commit.author_email), class: "", width: 16, alt: '' | ||||
|       = markdown escape_once(truncate(commit.title, length: 40)), pipeline: :single_line, author: commit.author | ||||
|       = markdown(truncate(commit.title, length: 40), pipeline: :single_line, author: commit.author) | ||||
|   %td | ||||
|     %span.pull-right.cgray | ||||
|       = time_ago_with_tooltip(commit.committed_date) | ||||
|  |  | |||
|  | @ -1,8 +1,8 @@ | |||
| %h3 Shared Runners | ||||
| 
 | ||||
| .bs-callout.bs-callout-warning.shared-runners-description | ||||
|   - if shared_runners_text.present? | ||||
|     = markdown(shared_runners_text, pipeline: 'plain_markdown') | ||||
|   - if current_application_settings.shared_runners_text.present? | ||||
|     = markdown_field(current_application_settings, :shared_runners_text) | ||||
|   - else | ||||
|     GitLab Shared Runners execute code of different projects on the same Runner | ||||
|     unless you configure GitLab Runner Autoscale with MaxBuilds 1 (which it is | ||||
|  |  | |||
|  | @ -30,4 +30,4 @@ | |||
|     .description.prepend-top-default | ||||
|       .wiki | ||||
|         = preserve do | ||||
|           = markdown release.description | ||||
|           = markdown_field(release, :description) | ||||
|  |  | |||
|  | @ -33,6 +33,6 @@ | |||
|       .description | ||||
|         .wiki | ||||
|           = preserve do | ||||
|             = markdown @release.description | ||||
|             = markdown_field(@release, :description) | ||||
|     - else | ||||
|       This tag has no release notes. | ||||
|  |  | |||
|  | @ -12,4 +12,4 @@ | |||
|     = link_to_label(label, tooltip: false) | ||||
|   - if label.description | ||||
|     %span.label-description | ||||
|       = markdown(label.description, pipeline: :single_line) | ||||
|       = markdown_field(label, :description) | ||||
|  |  | |||
|  | @ -35,4 +35,4 @@ | |||
| 
 | ||||
|   - if group.description.present? | ||||
|     .description | ||||
|       = markdown(group.description, pipeline: :description) | ||||
|       = markdown_field(group, :description) | ||||
|  |  | |||
|  | @ -8,7 +8,7 @@ | |||
|           = link_to milestones_label_path(options) do | ||||
|             - render_colored_label(label, tooltip: false) | ||||
|         %span.prepend-description-left | ||||
|           = markdown(label.description, pipeline: :single_line) | ||||
|           = markdown_field(label, :description) | ||||
| 
 | ||||
|       .pull-info-right | ||||
|         %span.append-right-20 | ||||
|  |  | |||
|  | @ -26,7 +26,7 @@ | |||
| 
 | ||||
| .detail-page-description.milestone-detail | ||||
|   %h2.title | ||||
|     = markdown escape_once(milestone.title), pipeline: :single_line | ||||
|     = markdown_field(milestone, :title) | ||||
| 
 | ||||
| - if milestone.complete?(current_user) && milestone.active? | ||||
|   .alert.alert-success.prepend-top-default | ||||
|  | @ -55,4 +55,3 @@ | |||
|             Open | ||||
|         %td | ||||
|           = ms.expires_at | ||||
| 
 | ||||
|  |  | |||
|  | @ -50,4 +50,4 @@ | |||
|           class: "commit-row-message" | ||||
|     - elsif project.description.present? | ||||
|       .description | ||||
|         = markdown(project.description, pipeline: :description) | ||||
|         = markdown_field(project, :description) | ||||
|  |  | |||
|  | @ -1,9 +1,12 @@ | |||
| - unless @snippet.content.empty? | ||||
|   - if markup?(@snippet.file_name) | ||||
|     %textarea.markdown-snippet-copy.blob-content{data: {blob_id: @snippet.id}} | ||||
|       = @snippet.data | ||||
|       = @snippet.content | ||||
|     .file-content.wiki | ||||
|       = render_markup(@snippet.file_name, @snippet.data) | ||||
|       - if gitlab_markdown?(@snippet.file_name) | ||||
|         = preserve(markdown_field(@snippet, :content)) | ||||
|       - else | ||||
|         = render_markup(@snippet.file_name, @snippet.content) | ||||
|   - else | ||||
|     = render 'shared/file_highlight', blob: @snippet | ||||
| - else | ||||
|  |  | |||
|  | @ -21,4 +21,4 @@ | |||
|       = render "snippets/actions" | ||||
| 
 | ||||
| %h2.snippet-title.prepend-top-0.append-bottom-0 | ||||
|   = markdown escape_once(@snippet.title), pipeline: :single_line, author: @snippet.author | ||||
|   = markdown_field(@snippet, :title) | ||||
|  |  | |||
|  | @ -0,0 +1,12 @@ | |||
| require 'erb' | ||||
| 
 | ||||
| module Banzai | ||||
|   module Filter | ||||
|     # Text filter that escapes these HTML entities: & " < > | ||||
|     class HTMLEntityFilter < HTML::Pipeline::TextFilter | ||||
|       def call | ||||
|         ERB::Util.html_escape(text) | ||||
|       end | ||||
|     end | ||||
|   end | ||||
| end | ||||
|  | @ -3,6 +3,7 @@ module Banzai | |||
|     class SingleLinePipeline < GfmPipeline | ||||
|       def self.filters | ||||
|         @filters ||= FilterArray[ | ||||
|           Filter::HTMLEntityFilter, | ||||
|           Filter::SanitizationFilter, | ||||
| 
 | ||||
|           Filter::EmojiFilter, | ||||
|  |  | |||
|  | @ -7,7 +7,7 @@ describe BroadcastMessagesHelper do | |||
|     end | ||||
| 
 | ||||
|     it 'includes the current message' do | ||||
|       current = double(message: 'Current Message') | ||||
|       current = BroadcastMessage.new(message: 'Current Message') | ||||
| 
 | ||||
|       allow(helper).to receive(:broadcast_message_style).and_return(nil) | ||||
| 
 | ||||
|  | @ -15,7 +15,7 @@ describe BroadcastMessagesHelper do | |||
|     end | ||||
| 
 | ||||
|     it 'includes custom style' do | ||||
|       current = double(message: 'Current Message') | ||||
|       current = BroadcastMessage.new(message: 'Current Message') | ||||
| 
 | ||||
|       allow(helper).to receive(:broadcast_message_style).and_return('foo') | ||||
| 
 | ||||
|  |  | |||
|  | @ -0,0 +1,14 @@ | |||
| require 'spec_helper' | ||||
| 
 | ||||
| describe Banzai::Filter::HTMLEntityFilter, lib: true do | ||||
|   include FilterSpecHelper | ||||
| 
 | ||||
|   let(:unescaped) { 'foo <strike attr="foo">&&&</strike>' } | ||||
|   let(:escaped) { 'foo <strike attr="foo">&&&</strike>' } | ||||
| 
 | ||||
|   it 'converts common entities to their HTML-escaped equivalents' do | ||||
|     output = filter(unescaped) | ||||
| 
 | ||||
|     expect(output).to eq(escaped) | ||||
|   end | ||||
| end | ||||
		Loading…
	
		Reference in New Issue