Commit 7e199a51 authored by Steve Mokris's avatar Steve Mokris Committed by Markus Koller

Allow diffing changes in wiki history

Introduces a new `diff` action on the wiki controllers which shows
the diff for the selected file and commit.

- Add `Gitlab::Diff::FileCollection::WikiPage`.
- Override diff helpers in wiki controller.
- Unify wiki views:
  - Always use limited container width.
  - Always add breadcrumbs for wiki page path.
  - Show sidebar in diff view.
  - Align title in diff view with history view.
  - Add helpers for wiki page headers.
parent dc718492
# frozen_string_literal: true
module WikiActions
include DiffHelper
include SendsBlob
include Gitlab::Utils::StrongMemoize
extend ActiveSupport::Concern
......@@ -11,8 +12,9 @@ module WikiActions
before_action :authorize_admin_wiki!, only: :destroy
before_action :wiki
before_action :page, only: [:show, :edit, :update, :history, :destroy]
before_action :page, only: [:show, :edit, :update, :history, :destroy, :diff]
before_action :load_sidebar, except: [:pages]
before_action :set_content_class
before_action only: [:show, :edit, :update] do
@valid_encoding = valid_encoding?
......@@ -25,6 +27,8 @@ module WikiActions
redirect_to wiki_path(wiki)
end
end
helper_method :view_file_button, :diff_file_html_data
end
def new
......@@ -136,6 +140,19 @@ module WikiActions
end
# rubocop:enable Gitlab/ModuleWithInstanceVariables
# rubocop:disable Gitlab/ModuleWithInstanceVariables
def diff
return render_404 unless page
apply_diff_view_cookie!
@diffs = page.diffs(diff_options)
@diff_notes_disabled = true
render 'shared/wikis/diff'
end
# rubocop:disable Gitlab/ModuleWithInstanceVariables
# rubocop:disable Gitlab/ModuleWithInstanceVariables
def destroy
WikiPages::DestroyService.new(container: container, current_user: current_user).execute(page)
......@@ -207,7 +224,7 @@ module WikiActions
def page_params
keys = [:id]
keys << :version_id if params[:action] == 'show'
keys << :version_id if %w[show diff].include?(params[:action])
params.values_at(*keys)
end
......@@ -233,4 +250,25 @@ module WikiActions
wiki.repository.blob_at(commit.id, params[:id])
end
end
def set_content_class
@content_class = 'limit-container-width' unless fluid_layout # rubocop:disable Gitlab/ModuleWithInstanceVariables
end
# Override CommitsHelper#view_file_button
def view_file_button(commit_sha, *args)
path = wiki_page_path(wiki, page, version_id: page.version.id)
helpers.link_to(path, class: 'btn') do
helpers.raw(_('View page @ ')) + helpers.content_tag(:span, Commit.truncate_sha(commit_sha), class: 'commit-sha')
end
end
# Override DiffHelper#diff_file_html_data
def diff_file_html_data(_project, _diff_file_path, diff_commit_id)
{
blob_diff_path: wiki_page_path(wiki, page, action: :diff, version_id: diff_commit_id),
view: diff_view
}
end
end
......@@ -181,15 +181,11 @@ module CommitsHelper
end
def view_file_button(commit_sha, diff_new_path, project, replaced: false)
path = project_blob_path(project, tree_join(commit_sha, diff_new_path))
title = replaced ? _('View replaced file @ ') : _('View file @ ')
link_to(
project_blob_path(project,
tree_join(commit_sha, diff_new_path)),
class: 'btn view-file js-view-file'
) do
raw(title) + content_tag(:span, Commit.truncate_sha(commit_sha),
class: 'commit-sha')
link_to(path, class: 'btn') do
raw(title) + content_tag(:span, truncate_sha(commit_sha), class: 'commit-sha')
end
end
......
......@@ -135,8 +135,7 @@ module DiffHelper
def diff_file_html_data(project, diff_file_path, diff_commit_id)
{
blob_diff_path: project_blob_diff_path(project,
tree_join(diff_commit_id, diff_file_path)),
blob_diff_path: project_blob_diff_path(project, tree_join(diff_commit_id, diff_file_path)),
view: diff_view
}
end
......
......@@ -27,7 +27,7 @@ module NavHelper
end
elsif current_path?('jobs#show')
%w[page-gutter build-sidebar right-sidebar-expanded]
elsif current_controller?('wikis') && current_action?('show', 'create', 'edit', 'update', 'history', 'git_access', 'destroy')
elsif current_controller?('wikis') && current_action?('show', 'create', 'edit', 'update', 'history', 'git_access', 'destroy', 'diff')
%w[page-gutter wiki-sidebar right-sidebar-expanded]
else
[]
......
......@@ -3,6 +3,30 @@
module WikiHelper
include API::Helpers::RelatedResourcesHelpers
def wiki_page_title(page, action = nil)
titles = [_('Wiki')]
if page.persisted?
titles << page.human_title
breadcrumb_title(page.human_title)
wiki_breadcrumb_dropdown_links(page.slug)
end
titles << action if action
page_title(*titles.reverse)
add_to_breadcrumbs(_('Wiki'), wiki_path(page.wiki))
end
def link_to_wiki_page(page, **options)
link_to page.human_title, wiki_page_path(page.wiki, page), **options
end
def wiki_sidebar_toggle_button
content_tag :button, class: 'btn btn-default sidebar-toggle js-sidebar-wiki-toggle', role: 'button', type: 'button' do
sprite_icon('chevron-double-lg-left')
end
end
# Produces a pure text breadcrumb for a given page.
#
# page_slug - The slug of a WikiPage object.
......
......@@ -301,6 +301,10 @@ class WikiPage
version&.commit&.committed_date
end
def diffs(diff_options = {})
Gitlab::Diff::FileCollection::WikiPage.new(self, diff_options: diff_options)
end
private
def serialize_front_matter(hash)
......
......@@ -37,7 +37,7 @@
an outdated change in
commit
%span.commit-sha= Commit.truncate_sha(discussion.commit_id)
%span.commit-sha= truncate_sha(discussion.commit_id)
- else
- unless discussion.active?
an old version of
......
......@@ -16,6 +16,8 @@
= diff_merge_request_whitespace_link(diffs.project, @merge_request, class: 'd-none d-sm-inline-block')
- elsif current_controller?(:compare)
= diff_compare_whitespace_link(diffs.project, params[:from], params[:to], class: 'd-none d-sm-inline-block')
- elsif current_controller?(:wikis)
= toggle_whitespace_link(url_for(params_with_whitespace), class: 'd-none d-sm-inline-block')
.btn-group
= inline_diff_btn
= parallel_diff_btn
......
......@@ -2,8 +2,7 @@
- page_title s_("WikiClone|Git Access"), _("Wiki")
.wiki-page-header.top-area.has-sidebar-toggle.py-3.flex-column.flex-lg-row
%button.btn.btn-default.d-block.d-sm-block.d-md-none.float-right.sidebar-toggle.js-sidebar-wiki-toggle{ role: "button", type: "button" }
= sprite_icon('chevron-double-lg-left')
= wiki_sidebar_toggle_button
.git-access-header.w-100.d-flex.flex-column.justify-content-center
%span
......
- wiki_page_title @page, _('Changes')
- commit = @diffs.diffable
.wiki-page-header.top-area.has-sidebar-toggle.flex-column.flex-lg-row
= wiki_sidebar_toggle_button
.nav-text
%h2.wiki-page-title
= link_to_wiki_page @page
%span.light
&middot;
= _('Changes')
.nav-controls.pb-md-3.pb-lg-0
= link_to wiki_page_path(@wiki, @page, action: :history), class: 'btn', role: 'button', data: { qa_selector: 'page_history_button' } do
= s_('Wiki|Page history')
.page-content-header
.header-main-content
%strong= markdown_field(commit, :title)
%span.d-none.d-sm-inline= _('authored')
#{time_ago_with_tooltip(commit.authored_date)}
%span= s_('ByAuthor|by')
= author_avatar(commit, size: 24, has_tooltip: false)
%strong
= commit_author_link(commit, avatar: true, size: 24)
- if commit.description.present?
%pre.commit-description<
= preserve(markdown_field(commit, :description))
= render 'projects/diffs/diffs', diffs: @diffs
= render 'shared/wikis/sidebar'
- @content_class = "limit-container-width" unless fluid_layout
- add_to_breadcrumbs _("Wiki"), wiki_page_path(@wiki, @page)
- breadcrumb_title @page.persisted? ? _("Edit") : _("New")
- page_title @page.persisted? ? _("Edit") : _("New"), @page.human_title, _("Wiki")
- wiki_page_title @page, @page.persisted? ? _('Edit') : _('New')
= wiki_page_errors(@error)
.wiki-page-header.top-area.has-sidebar-toggle.flex-column.flex-lg-row
%button.btn.btn-default.sidebar-toggle.js-sidebar-wiki-toggle{ role: "button", type: "button" }
= sprite_icon('chevron-double-lg-left')
= wiki_sidebar_toggle_button
.nav-text
%h2.wiki-page-title
- if @page.persisted?
= link_to @page.human_title, wiki_page_path(@wiki, @page)
= link_to_wiki_page @page
%span.light
&middot;
= s_("Wiki|Edit Page")
......
- page_title _("History"), @page.human_title, _("Wiki")
- wiki_page_title @page, _('History')
.wiki-page-header.top-area.has-sidebar-toggle.flex-column.flex-lg-row
%button.btn.btn-default.sidebar-toggle.js-sidebar-wiki-toggle{ role: "button", type: "button" }
= sprite_icon('chevron-double-lg-left')
= wiki_sidebar_toggle_button
.nav-text
%h2.wiki-page-title
= link_to @page.human_title, wiki_page_path(@wiki, @page)
= link_to_wiki_page @page
%span.light
&middot;
= _("History")
= _('History')
.table-holder
%table.table
%thead
%tr
%th= s_("Wiki|Page version")
%th= _("Author")
%th= _("Commit Message")
%th= _("Last updated")
%th= _("Format")
%tbody
- @page_versions.each_with_index do |version, index|
- commit = version
.prepend-top-default.gl-mb-3
.table-holder
%table.table.wiki-history
%thead
%tr
%td
= link_to wiki_page_path(@wiki, @page, version_id: index == 0 ? nil : commit.id) do
= truncate_sha(commit.id)
%td
= commit.author_name
%td
= commit.message
%td
#{time_ago_with_tooltip(version.authored_date)}
%td
%strong
= version.format
= paginate @page_versions, theme: 'gitlab'
%th= s_('Wiki|Page version')
%th= _('Author')
%th= _('Changes')
%th= _('Last updated')
%tbody
- @page_versions.each do |commit|
%tr
%td
= link_to wiki_page_path(@wiki, @page, version_id: commit.id) do
= truncate_sha(commit.id)
%td
= commit.author_name
%td
%span.str-truncated-60
= link_to wiki_page_path(@wiki, @page, action: :diff, version_id: commit.id), { title: commit.message } do
= commit.message
%td
= time_ago_with_tooltip(commit.authored_date)
= paginate @page_versions, theme: 'gitlab'
= render 'shared/wikis/sidebar'
- @content_class = "limit-container-width" unless fluid_layout
- breadcrumb_title @page.human_title
- wiki_breadcrumb_dropdown_links(@page.slug)
- page_title @page.human_title, _("Wiki")
- add_to_breadcrumbs _("Wiki"), wiki_path(@wiki)
- wiki_page_title @page
.wiki-page-header.top-area.has-sidebar-toggle.flex-column.flex-lg-row
%button.btn.btn-default.sidebar-toggle.js-sidebar-wiki-toggle{ role: "button", type: "button" }
= sprite_icon('chevron-double-lg-left')
= wiki_sidebar_toggle_button
.nav-text.flex-fill
%h2.wiki-page-title{ data: { qa_selector: 'wiki_page_title' } }= @page.human_title
%span.wiki-last-edit-by
- if @page.last_version
= (_("Last edited by %{name}") % { name: "<strong>#{@page.last_version.author_name}</strong>" }).html_safe
#{time_ago_with_tooltip(@page.last_version.authored_date)}
= time_ago_with_tooltip(@page.last_version.authored_date)
.nav-controls.pb-md-3.pb-lg-0
= render 'shared/wikis/main_links'
......
---
title: Allow diffing changes in wiki history
merge_request: 35330
author: gwhyte, Steve Mokris
type: added
......@@ -10,6 +10,7 @@ scope(controller: :wikis) do
scope(path: 'wikis/*id', as: :wiki, format: false) do
get :edit
get :history
get :diff
post :preview_markdown
get '/', action: :show
put '/', action: :update
......
# frozen_string_literal: true
module Gitlab
module Diff
module FileCollection
class WikiPage < Base
def initialize(page, diff_options:)
commit = page.wiki.commit(page.version.commit)
diff_options = diff_options.merge(
expanded: true,
paths: [page.path]
)
super(commit,
# TODO: Uncouple diffing from projects
# https://gitlab.com/gitlab-org/gitlab/-/issues/217752
project: page.wiki,
diff_options: diff_options,
diff_refs: commit.diff_refs)
end
end
end
end
end
......@@ -101,6 +101,10 @@ module Gitlab
wrapped_gitaly_errors do
gitaly_find_page(title: title, version: version, dir: dir)
end
rescue Gitlab::Git::CommandError
# Return nil for invalid versions.
# This can be removed with https://gitlab.com/gitlab-org/gitaly/-/merge_requests/2323 in place.
nil
end
def file(name, version)
......
......@@ -10178,9 +10178,6 @@ msgstr ""
msgid "Forks"
msgstr ""
msgid "Format"
msgstr ""
msgid "Format: %{dateFormat}"
msgstr ""
......@@ -25387,6 +25384,9 @@ msgstr ""
msgid "View open merge request"
msgstr ""
msgid "View page @ "
msgstr ""
msgid "View performance dashboard."
msgstr ""
......
......@@ -8,9 +8,10 @@ RSpec.describe 'User views a wiki page' do
let(:user) { create(:user) }
let(:project) { create(:project, :wiki_repo, namespace: user.namespace) }
let(:path) { 'image.png' }
let(:wiki) { project.wiki }
let(:wiki_page) do
create(:wiki_page,
wiki: project.wiki,
wiki: wiki,
title: 'home', content: "Look at this [image](#{path})\n\n ![alt text](#{path})")
end
......@@ -70,11 +71,13 @@ RSpec.describe 'User views a wiki page' do
click_on('Page history')
page.within(:css, '.nav-text') do
within('.nav-text') do
expect(page).to have_content('History')
end
find('a[href*="?version_id"]')
within('.wiki-history') do
expect(page).to have_css('a[href*="?version_id"]', count: 4)
end
end
end
......@@ -92,8 +95,8 @@ RSpec.describe 'User views a wiki page' do
let(:path) { upload_file_to_wiki(project, user, 'dk.png') }
it do
expect(page).to have_xpath("//img[@data-src='#{project.wiki.wiki_base_path}/#{path}']")
expect(page).to have_link('image', href: "#{project.wiki.wiki_base_path}/#{path}")
expect(page).to have_xpath("//img[@data-src='#{wiki.wiki_base_path}/#{path}']")
expect(page).to have_link('image', href: "#{wiki.wiki_base_path}/#{path}")
click_on('image')
......@@ -103,7 +106,7 @@ RSpec.describe 'User views a wiki page' do
end
it 'shows the creation page if file does not exist' do
expect(page).to have_link('image', href: "#{project.wiki.wiki_base_path}/#{path}")
expect(page).to have_link('image', href: "#{wiki.wiki_base_path}/#{path}")
click_on('image')
......@@ -114,7 +117,7 @@ RSpec.describe 'User views a wiki page' do
context 'when a page has history' do
before do
wiki_page.update(message: 'updated home', content: 'updated [some link](other-page)')
wiki_page.update(message: 'updated home', content: 'updated [some link](other-page)') # rubocop:disable Rails/SaveBang
end
it 'shows the page history' do
......@@ -134,13 +137,74 @@ RSpec.describe 'User views a wiki page' do
expect(page).not_to have_selector('a.btn', text: 'Edit')
end
context 'show the diff' do
def expect_diff_links(commit)
diff_path = wiki_page_path(wiki, wiki_page, version_id: commit, action: :diff)
expect(page).to have_link('Hide whitespace changes', href: "#{diff_path}&w=1")
expect(page).to have_link('Inline', href: "#{diff_path}&view=inline")
expect(page).to have_link('Side-by-side', href: "#{diff_path}&view=parallel")
expect(page).to have_link("View page @ #{commit.short_id}", href: wiki_page_path(wiki, wiki_page, version_id: commit))
expect(page).to have_css('.diff-file[data-blob-diff-path="%s"]' % diff_path)
end
it 'links to the correct diffs' do
visit project_wiki_history_path(project, wiki_page)
commit1 = wiki.commit('HEAD^')
commit2 = wiki.commit
expect(page).to have_link('created page: home', href: wiki_page_path(wiki, wiki_page, version_id: commit1, action: :diff))
expect(page).to have_link('updated home', href: wiki_page_path(wiki, wiki_page, version_id: commit2, action: :diff))
end
it 'between the current and the previous version of a page' do
commit = wiki.commit
visit wiki_page_path(wiki, wiki_page, version_id: commit, action: :diff)
expect(page).to have_content('by John Doe')
expect(page).to have_content('updated home')
expect(page).to have_content('Showing 1 changed file with 1 addition and 3 deletions')
expect(page).to have_content('some link')
expect_diff_links(commit)
end
it 'between two old versions of a page' do
wiki_page.update(message: 'latest home change', content: 'updated [another link](other-page)') # rubocop:disable Rails/SaveBang:
commit = wiki.commit('HEAD^')
visit wiki_page_path(wiki, wiki_page, version_id: commit, action: :diff)
expect(page).to have_content('by John Doe')
expect(page).to have_content('updated home')
expect(page).to have_content('Showing 1 changed file with 1 addition and 3 deletions')
expect(page).to have_content('some link')
expect(page).not_to have_content('latest home change')
expect(page).not_to have_content('another link')
expect_diff_links(commit)
end
it 'for the oldest version of a page' do
commit = wiki.commit('HEAD^')
visit wiki_page_path(wiki, wiki_page, version_id: commit, action: :diff)
expect(page).to have_content('by John Doe')
expect(page).to have_content('created page: home')
expect(page).to have_content('Showing 1 changed file with 4 additions and 0 deletions')
expect(page).to have_content('Look at this')
expect_diff_links(commit)
end
end
end
context 'when a page has special characters in its title' do
let(:title) { '<foo> !@#$%^&*()[]{}=_+\'"\\|<>? <bar>' }
before do
wiki_page.update(title: title )
wiki_page.update(title: title ) # rubocop:disable Rails/SaveBang
end
it 'preserves the special characters' do
......@@ -155,7 +219,7 @@ RSpec.describe 'User views a wiki page' do
let(:title) { '<script>alert("title")<script>' }
before do
wiki_page.update(title: title, content: 'foo <script>alert("content")</script> bar')
wiki_page.update(title: title, content: 'foo <script>alert("content")</script> bar') # rubocop:disable Rails/SaveBang
end
it 'safely displays the page' do
......@@ -168,7 +232,7 @@ RSpec.describe 'User views a wiki page' do
context 'when a page has XSS in its message' do
before do
wiki_page.update(message: '<script>alert(true)<script>', content: 'XSS update')
wiki_page.update(message: '<script>alert(true)<script>', content: 'XSS update') # rubocop:disable Rails/SaveBang
end
it 'safely displays the message' do
......
......@@ -51,6 +51,20 @@ RSpec.describe CommitsHelper do
end
end
describe '#view_file_button' do
let(:project) { build(:project) }
let(:path) { 'path/to/file' }
let(:sha) { '1234567890' }
subject do
helper.view_file_button(sha, path, project)
end
it 'links to project files' do
expect(subject).to have_link('1234567', href: helper.project_blob_path(project, "#{sha}/#{path}"))
end
end
describe '#view_on_environment_button' do
let(:project) { create(:project) }
let(:environment) { create(:environment, external_url: 'http://example.com') }
......
......@@ -303,6 +303,20 @@ RSpec.describe DiffHelper do
end
end
describe '#diff_file_html_data' do
let(:project) { build(:project) }
let(:path) { 'path/to/file' }
let(:sha) { '1234567890' }
subject do
helper.diff_file_html_data(project, path, sha)
end
it 'returns data for project files' do
expect(subject).to include(blob_diff_path: helper.project_blob_diff_path(project, "#{sha}/#{path}"))
end
end
describe '#diff_file_path_text' do
it 'returns full path by default' do
expect(diff_file_path_text(diff_file)).to eq(diff_file.new_path)
......
......@@ -3,6 +3,38 @@
require 'spec_helper'
RSpec.describe WikiHelper do
describe '#wiki_page_title' do
let_it_be(:page) { create(:wiki_page) }
it 'sets the title for the show action' do
expect(helper).to receive(:breadcrumb_title).with(page.human_title)
expect(helper).to receive(:wiki_breadcrumb_dropdown_links).with(page.slug)
expect(helper).to receive(:page_title).with(page.human_title, 'Wiki')
expect(helper).to receive(:add_to_breadcrumbs).with('Wiki', helper.wiki_path(page.wiki))
helper.wiki_page_title(page)
end
it 'sets the title for a custom action' do
expect(helper).to receive(:breadcrumb_title).with(page.human_title)
expect(helper).to receive(:wiki_breadcrumb_dropdown_links).with(page.slug)
expect(helper).to receive(:page_title).with('Edit', page.human_title, 'Wiki')
expect(helper).to receive(:add_to_breadcrumbs).with('Wiki', helper.wiki_path(page.wiki))
helper.wiki_page_title(page, 'Edit')
end
it 'sets the title for an unsaved page' do
expect(page).to receive(:persisted?).and_return(false)
expect(helper).not_to receive(:breadcrumb_title)
expect(helper).not_to receive(:wiki_breadcrumb_dropdown_links)
expect(helper).to receive(:page_title).with('Wiki')
expect(helper).to receive(:add_to_breadcrumbs).with('Wiki', helper.wiki_path(page.wiki))
helper.wiki_page_title(page)
end
end
describe '#breadcrumb' do
context 'when the page is at the root level' do
it 'returns the capitalized page name' do
......
......@@ -864,6 +864,24 @@ RSpec.describe WikiPage do
end
end
describe '#diffs' do
subject { existing_page }
it 'returns a diff instance' do
diffs = subject.diffs(foo: 'bar')
expect(diffs).to be_a(Gitlab::Diff::FileCollection::WikiPage)
expect(diffs.diffable).to be_a(Commit)
expect(diffs.diffable.id).to eq(subject.version.id)
expect(diffs.project).to be(subject.wiki)
expect(diffs.diff_options).to include(
expanded: true,
paths: [subject.path],
foo: 'bar'
)
end
end
private
def get_slugs(page_or_dir)
......
......@@ -104,6 +104,35 @@ RSpec.shared_examples 'wiki controller actions' do
end
end
describe 'GET #diff' do
context 'when commit exists' do
it 'renders the diff' do
get :diff, params: routing_params.merge(id: wiki_title, version_id: wiki.repository.commit.id)
expect(response).to have_gitlab_http_status(:ok)
expect(response).to render_template('shared/wikis/diff')
expect(assigns(:diffs)).to be_a(Gitlab::Diff::FileCollection::Base)
expect(assigns(:diff_notes_disabled)).to be(true)
end
end
context 'when commit does not exist' do
it 'returns a 404 error' do
get :diff, params: routing_params.merge(id: wiki_title, version_id: 'invalid')
expect(response).to have_gitlab_http_status(:not_found)
end
end
context 'when page does not exist' do
it 'returns a 404 error' do
get :diff, params: routing_params.merge(id: 'invalid')
expect(response).to have_gitlab_http_status(:not_found)
end
end
end
describe 'GET #show' do
render_views
......
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