Commit 79193669 authored by Yorick Peterse's avatar Yorick Peterse Committed by Robert Speicher

Merge branch 'refactor-rendering-redacting' into 'master'

Support for rendering/redacting multiple documents

See merge request !4828
parent 12d9e057
...@@ -19,6 +19,7 @@ v 8.9.1 (unreleased) ...@@ -19,6 +19,7 @@ v 8.9.1 (unreleased)
- Set button label when picking an option from status dropdown. !4771 - Set button label when picking an option from status dropdown. !4771
- Prevent invalid URLs from raising exceptions in WikiLink Filter. !4775 - Prevent invalid URLs from raising exceptions in WikiLink Filter. !4775
- Handle external issues in IssueReferenceFilter. !4789 - Handle external issues in IssueReferenceFilter. !4789
- Support for rendering/redacting multiple documents. !4828
- Update Todos documentation and screenshots to include new functionality. !4840 - Update Todos documentation and screenshots to include new functionality. !4840
- Hide nav arrows by default. !4843 - Hide nav arrows by default. !4843
- Added bottom padding to label color suggestion link. !4845 - Added bottom padding to label color suggestion link. !4845
......
...@@ -62,8 +62,12 @@ class Projects::IssuesController < Projects::ApplicationController ...@@ -62,8 +62,12 @@ class Projects::IssuesController < Projects::ApplicationController
end end
def show def show
raw_notes = @issue.notes_with_associations.fresh
@notes = Banzai::NoteRenderer.
render(raw_notes, @project, current_user, @path, @project_wiki, @ref)
@note = @project.notes.new(noteable: @issue) @note = @project.notes.new(noteable: @issue)
@notes = @issue.notes.with_associations.fresh
@noteable = @issue @noteable = @issue
respond_to do |format| respond_to do |format|
......
...@@ -85,6 +85,15 @@ class Projects::MergeRequestsController < Projects::ApplicationController ...@@ -85,6 +85,15 @@ class Projects::MergeRequestsController < Projects::ApplicationController
@grouped_diff_notes = @merge_request.notes.grouped_diff_notes @grouped_diff_notes = @merge_request.notes.grouped_diff_notes
Banzai::NoteRenderer.render(
@grouped_diff_notes.values.flatten,
@project,
current_user,
@path,
@project_wiki,
@ref
)
respond_to do |format| respond_to do |format|
format.html format.html
format.json { render json: { html: view_to_html_string("projects/merge_requests/show/_diffs") } } format.json { render json: { html: view_to_html_string("projects/merge_requests/show/_diffs") } }
...@@ -311,8 +320,21 @@ class Projects::MergeRequestsController < Projects::ApplicationController ...@@ -311,8 +320,21 @@ class Projects::MergeRequestsController < Projects::ApplicationController
def define_show_vars def define_show_vars
# Build a note object for comment form # Build a note object for comment form
@note = @project.notes.new(noteable: @merge_request) @note = @project.notes.new(noteable: @merge_request)
@notes = @merge_request.mr_and_commit_notes.inc_author.fresh
@discussions = @notes.discussions @discussions = @merge_request.mr_and_commit_notes.
inc_author_project_award_emoji.
fresh.
discussions
@notes = Banzai::NoteRenderer.render(
@discussions.flatten,
@project,
current_user,
@path,
@project_wiki,
@ref
)
@noteable = @merge_request @noteable = @merge_request
# Get commits from repository # Get commits from repository
......
...@@ -24,6 +24,10 @@ class Projects::NotesController < Projects::ApplicationController ...@@ -24,6 +24,10 @@ class Projects::NotesController < Projects::ApplicationController
def create def create
@note = Notes::CreateService.new(project, current_user, note_params).execute @note = Notes::CreateService.new(project, current_user, note_params).execute
if @note.is_a?(Note)
Banzai::NoteRenderer.render([@note], @project, current_user)
end
respond_to do |format| respond_to do |format|
format.json { render json: note_json(@note) } format.json { render json: note_json(@note) }
format.html { redirect_back_or_default } format.html { redirect_back_or_default }
...@@ -33,6 +37,10 @@ class Projects::NotesController < Projects::ApplicationController ...@@ -33,6 +37,10 @@ class Projects::NotesController < Projects::ApplicationController
def update def update
@note = Notes::UpdateService.new(project, current_user, note_params).execute(note) @note = Notes::UpdateService.new(project, current_user, note_params).execute(note)
if @note.is_a?(Note)
Banzai::NoteRenderer.render([@note], @project, current_user)
end
respond_to do |format| respond_to do |format|
format.json { render json: note_json(@note) } format.json { render json: note_json(@note) }
format.html { redirect_back_or_default } format.html { redirect_back_or_default }
...@@ -118,6 +126,8 @@ class Projects::NotesController < Projects::ApplicationController ...@@ -118,6 +126,8 @@ class Projects::NotesController < Projects::ApplicationController
name: note.name name: note.name
} }
elsif note.valid? elsif note.valid?
Banzai::NoteRenderer.render([note], @project, current_user)
{ {
valid: true, valid: true,
id: note.id, id: note.id,
......
...@@ -6,6 +6,10 @@ class Note < ActiveRecord::Base ...@@ -6,6 +6,10 @@ class Note < ActiveRecord::Base
include Awardable include Awardable
include Importable include Importable
# Attribute containing rendered and redacted Markdown as generated by
# Banzai::ObjectRenderer.
attr_accessor :note_html
default_value_for :system, false default_value_for :system, false
attr_mentionable :note, pipeline: :note attr_mentionable :note, pipeline: :note
......
...@@ -32,7 +32,7 @@ ...@@ -32,7 +32,7 @@
.note-body{class: note_editable ? 'js-task-list-container' : ''} .note-body{class: note_editable ? 'js-task-list-container' : ''}
.note-text .note-text
= preserve do = preserve do
= markdown(note.note, pipeline: :note, cache_key: [note, "note"], author: note.author) = note.note_html
= edited_time_ago_with_tooltip(note, placement: 'bottom', html_class: 'note_edited_ago', include_author: true) = edited_time_ago_with_tooltip(note, placement: 'bottom', html_class: 'note_edited_ago', include_author: true)
- if note_editable - if note_editable
= render 'projects/notes/edit_form', note: note = render 'projects/notes/edit_form', note: note
......
...@@ -113,6 +113,10 @@ if Gitlab::Metrics.enabled? ...@@ -113,6 +113,10 @@ if Gitlab::Metrics.enabled?
config.instrument_methods(Banzai::Renderer) config.instrument_methods(Banzai::Renderer)
config.instrument_methods(Banzai::Querying) config.instrument_methods(Banzai::Querying)
config.instrument_instance_methods(Banzai::ObjectRenderer)
config.instrument_instance_methods(Banzai::Redactor)
config.instrument_methods(Banzai::NoteRenderer)
[Issuable, Mentionable, Participable].each do |klass| [Issuable, Mentionable, Participable].each do |klass|
config.instrument_instance_methods(klass) config.instrument_instance_methods(klass)
config.instrument_instance_methods(klass::ClassMethods) config.instrument_instance_methods(klass::ClassMethods)
......
...@@ -7,40 +7,13 @@ module Banzai ...@@ -7,40 +7,13 @@ module Banzai
# #
class RedactorFilter < HTML::Pipeline::Filter class RedactorFilter < HTML::Pipeline::Filter
def call def call
nodes = Querying.css(doc, 'a.gfm[data-reference-type]') Redactor.new(project, current_user).redact([doc])
visible = nodes_visible_to_user(nodes)
nodes.each do |node|
unless visible.include?(node)
# The reference should be replaced by the original text,
# which is not always the same as the rendered text.
text = node.attr('data-original') || node.text
node.replace(text)
end
end
doc doc
end end
private private
def nodes_visible_to_user(nodes)
per_type = Hash.new { |h, k| h[k] = [] }
visible = Set.new
nodes.each do |node|
per_type[node.attr('data-reference-type')] << node
end
per_type.each do |type, nodes|
parser = Banzai::ReferenceParser[type].new(project, current_user)
visible.merge(parser.nodes_visible_to_user(current_user, nodes))
end
visible
end
def current_user def current_user
context[:current_user] context[:current_user]
end end
......
module Banzai
module NoteRenderer
# Renders a collection of Note instances.
#
# notes - The notes to render.
# project - The project to use for rendering/redacting.
# user - The user viewing the notes.
# path - The request path.
# wiki - The project's wiki.
# git_ref - The current Git reference.
def self.render(notes, project, user = nil, path = nil, wiki = nil, git_ref = nil)
renderer = ObjectRenderer.new(project,
user,
requested_path: path,
project_wiki: wiki,
ref: git_ref,
pipeline: :note)
renderer.render(notes, :note)
end
end
end
module Banzai
# Class for rendering multiple objects (e.g. Note instances) in a single pass.
#
# Rendered Markdown is stored in an attribute in every object based on the
# name of the attribute containing the Markdown. For example, when the
# attribute `note` is rendered the HTML is stored in `note_html`.
class ObjectRenderer
attr_reader :project, :user
# Make sure to set the appropriate pipeline in the `raw_context` attribute
# (e.g. `:note` for Note instances).
#
# project - A Project to use for rendering and redacting Markdown.
# user - The user viewing the Markdown/HTML documents, if any.
# context - A Hash containing extra attributes to use in the rendering
# pipeline.
def initialize(project, user = nil, raw_context = {})
@project = project
@user = user
@raw_context = raw_context
end
# Renders and redacts an Array of objects.
#
# objects - The objects to render
# attribute - The attribute containing the raw Markdown to render.
#
# Returns the same input objects.
def render(objects, attribute)
documents = render_objects(objects, attribute)
redacted = redact_documents(documents)
objects.each_with_index do |object, index|
object.__send__("#{attribute}_html=", redacted.fetch(index))
end
objects
end
# Renders the attribute of every given object.
def render_objects(objects, attribute)
objects.map do |object|
render_attribute(object, attribute)
end
end
# Redacts the list of documents.
#
# Returns an Array containing the redacted documents.
def redact_documents(documents)
redactor = Redactor.new(project, user)
redactor.redact(documents).map do |document|
document.to_html.html_safe
end
end
# Returns a Banzai context for the given object and attribute.
def context_for(object, attribute)
context = base_context.merge(cache_key: [object, attribute])
if object.respond_to?(:author)
context[:author] = object.author
end
context
end
# Renders the attribute of an object.
#
# Returns a `Nokogiri::HTML::Document`.
def render_attribute(object, attribute)
context = context_for(object, attribute)
string = object.__send__(attribute)
html = Banzai.render(string, context)
Banzai::Pipeline[:relative_link].to_document(html, context)
end
def base_context
@base_context ||= @raw_context.merge(current_user: user, project: project)
end
end
end
module Banzai
module Pipeline
class RelativeLinkPipeline < BasePipeline
def self.filters
FilterArray[
Filter::RelativeLinkFilter
]
end
end
end
end
module Banzai
# Class for removing Markdown references a certain user is not allowed to
# view.
class Redactor
attr_reader :user, :project
# project - A Project to use for redacting links.
# user - The currently logged in user (if any).
def initialize(project, user = nil)
@project = project
@user = user
end
# Redacts the references in the given Array of documents.
#
# This method modifies the given documents in-place.
#
# documents - A list of HTML documents containing references to redact.
#
# Returns the documents passed as the first argument.
def redact(documents)
nodes = documents.flat_map do |document|
Querying.css(document, 'a.gfm[data-reference-type]')
end
redact_nodes(nodes)
documents
end
# Redacts the given nodes
#
# nodes - An Array of HTML nodes to redact.
def redact_nodes(nodes)
visible = nodes_visible_to_user(nodes)
nodes.each do |node|
unless visible.include?(node)
# The reference should be replaced by the original text,
# which is not always the same as the rendered text.
text = node.attr('data-original') || node.text
node.replace(text)
end
end
end
# Returns the nodes visible to the current user.
#
# nodes - The input nodes to check.
#
# Returns a new Array containing the visible nodes.
def nodes_visible_to_user(nodes)
per_type = Hash.new { |h, k| h[k] = [] }
visible = Set.new
nodes.each do |node|
per_type[node.attr('data-reference-type')] << node
end
per_type.each do |type, nodes|
parser = Banzai::ReferenceParser[type].new(project, user)
visible.merge(parser.nodes_visible_to_user(user, nodes))
end
visible
end
end
end
require 'spec_helper'
describe Banzai::NoteRenderer do
describe '.render' do
it 'renders a Note' do
note = double(:note)
project = double(:project)
wiki = double(:wiki)
user = double(:user)
expect(Banzai::ObjectRenderer).to receive(:new).
with(project, user,
requested_path: 'foo',
project_wiki: wiki,
ref: 'bar',
pipeline: :note).
and_call_original
expect_any_instance_of(Banzai::ObjectRenderer).
to receive(:render).with([note], :note)
described_class.render([note], project, user, 'foo', wiki, 'bar')
end
end
end
require 'spec_helper'
describe Banzai::ObjectRenderer do
let(:project) { create(:empty_project) }
let(:user) { project.owner }
describe '#render' do
it 'renders and redacts an Array of objects' do
renderer = described_class.new(project, user)
object = double(:object, note: 'hello', note_html: nil)
expect(renderer).to receive(:render_objects).with([object], :note).
and_call_original
expect(renderer).to receive(:redact_documents).
with(an_instance_of(Array)).
and_call_original
expect(object).to receive(:note_html=).with('<p>hello</p>')
renderer.render([object], :note)
end
end
describe '#render_objects' do
it 'renders an Array of objects' do
object = double(:object, note: 'hello')
renderer = described_class.new(project, user)
expect(renderer).to receive(:render_attribute).with(object, :note).
and_call_original
rendered = renderer.render_objects([object], :note)
expect(rendered).to be_an_instance_of(Array)
expect(rendered[0]).to be_an_instance_of(Nokogiri::HTML::DocumentFragment)
end
end
describe '#redact_documents' do
it 'redacts a set of documents and returns them as an Array of Strings' do
doc = Nokogiri::HTML.fragment('<p>hello</p>')
renderer = described_class.new(project, user)
expect_any_instance_of(Banzai::Redactor).to receive(:redact).
with([doc]).
and_call_original
redacted = renderer.redact_documents([doc])
expect(redacted).to eq(['<p>hello</p>'])
end
end
describe '#context_for' do
let(:object) { double(:object, note: 'hello') }
let(:renderer) { described_class.new(project, user) }
it 'returns a Hash' do
expect(renderer.context_for(object, :note)).to be_an_instance_of(Hash)
end
it 'includes the cache key' do
context = renderer.context_for(object, :note)
expect(context[:cache_key]).to eq([object, :note])
end
context 'when the object responds to "author"' do
it 'includes the author in the context' do
expect(object).to receive(:author).and_return('Alice')
context = renderer.context_for(object, :note)
expect(context[:author]).to eq('Alice')
end
end
context 'when the object does not respond to "author"' do
it 'does not include the author in the context' do
context = renderer.context_for(object, :note)
expect(context.key?(:author)).to eq(false)
end
end
end
describe '#render_attribute' do
it 'renders the attribute of an object' do
object = double(:doc, note: 'hello')
renderer = described_class.new(project, user, pipeline: :note)
doc = renderer.render_attribute(object, :note)
expect(doc).to be_an_instance_of(Nokogiri::HTML::DocumentFragment)
expect(doc.to_html).to eq('<p>hello</p>')
end
end
describe '#base_context' do
let(:context) do
described_class.new(project, user, pipeline: :note).base_context
end
it 'returns a Hash' do
expect(context).to be_an_instance_of(Hash)
end
it 'includes the custom attributes' do
expect(context[:pipeline]).to eq(:note)
end
it 'includes the current user' do
expect(context[:current_user]).to eq(user)
end
it 'includes the current project' do
expect(context[:project]).to eq(project)
end
end
end
require 'spec_helper'
describe Banzai::Redactor do
let(:user) { build(:user) }
let(:project) { build(:empty_project) }
let(:redactor) { described_class.new(project, user) }
describe '#redact' do
it 'redacts an Array of documents' do
doc1 = Nokogiri::HTML.
fragment('<a class="gfm" data-reference-type="issue">foo</a>')
doc2 = Nokogiri::HTML.
fragment('<a class="gfm" data-reference-type="issue">bar</a>')
expect(redactor).to receive(:nodes_visible_to_user).and_return([])
expect(redactor.redact([doc1, doc2])).to eq([doc1, doc2])
expect(doc1.to_html).to eq('foo')
expect(doc2.to_html).to eq('bar')
end
end
describe '#redact_nodes' do
it 'redacts an Array of nodes' do
doc = Nokogiri::HTML.fragment('<a href="foo">foo</a>')
node = doc.children[0]
expect(redactor).to receive(:nodes_visible_to_user).
with([node]).
and_return(Set.new)
redactor.redact_nodes([node])
expect(doc.to_html).to eq('foo')
end
end
describe '#nodes_visible_to_user' do
it 'returns a Set containing the visible nodes' do
doc = Nokogiri::HTML.fragment('<a data-reference-type="issue"></a>')
node = doc.children[0]
expect_any_instance_of(Banzai::ReferenceParser::IssueParser).
to receive(:nodes_visible_to_user).
with(user, [node]).
and_return([node])
expect(redactor.nodes_visible_to_user([node])).to eq(Set.new([node]))
end
end
end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment