From d66066f7cb4a2981c74224ad2cb6ec3c00ca529e Mon Sep 17 00:00:00 2001 From: Nick Thomas Date: Mon, 15 Oct 2018 20:07:02 +0100 Subject: [PATCH] Harden the wiki against missing last_versions Currently, we assume a "last_version" always exists for a wiki page. In production, this is not always true. So, guard uses of it with a null check. --- app/models/wiki_page.rb | 4 ++- .../projects/wikis/_pages_wiki_page.html.haml | 3 +- app/views/projects/wikis/show.html.haml | 5 ++-- ...rden-wiki-against-missing-last-version.yml | 5 ++++ spec/models/wiki_page_spec.rb | 30 ++++++++++++------- 5 files changed, 33 insertions(+), 14 deletions(-) create mode 100644 changelogs/unreleased/52527-harden-wiki-against-missing-last-version.yml diff --git a/app/models/wiki_page.rb b/app/models/wiki_page.rb index 42fd213d03b..c5e349ae913 100644 --- a/app/models/wiki_page.rb +++ b/app/models/wiki_page.rb @@ -160,7 +160,9 @@ class WikiPage # Returns boolean True or False if this instance # is an old version of the page. def historical? - @page.historical? && last_version.sha != version.sha + return false unless last_commit_sha && version + + @page.historical? && last_commit_sha != version.sha end # Returns boolean True or False if this instance diff --git a/app/views/projects/wikis/_pages_wiki_page.html.haml b/app/views/projects/wikis/_pages_wiki_page.html.haml index cbb441d7509..c156f8cbf50 100644 --- a/app/views/projects/wikis/_pages_wiki_page.html.haml +++ b/app/views/projects/wikis/_pages_wiki_page.html.haml @@ -2,4 +2,5 @@ = link_to wiki_page.title, project_wiki_path(@project, wiki_page) %small (#{wiki_page.format}) .float-right - %small= (s_("Last edited %{date}") % { date: time_ago_with_tooltip(wiki_page.last_version.authored_date) }).html_safe + - if wiki_page.last_version + %small= (s_("Last edited %{date}") % { date: time_ago_with_tooltip(wiki_page.last_version.authored_date) }).html_safe diff --git a/app/views/projects/wikis/show.html.haml b/app/views/projects/wikis/show.html.haml index 19b9744b508..fbf248c2058 100644 --- a/app/views/projects/wikis/show.html.haml +++ b/app/views/projects/wikis/show.html.haml @@ -11,8 +11,9 @@ .nav-text %h2.wiki-page-title= @page.title.capitalize %span.wiki-last-edit-by - = (_("Last edited by %{name}") % { name: "#{@page.last_version.author_name}" }).html_safe - #{time_ago_with_tooltip(@page.last_version.authored_date)} + - if @page.last_version + = (_("Last edited by %{name}") % { name: "#{@page.last_version.author_name}" }).html_safe + #{time_ago_with_tooltip(@page.last_version.authored_date)} .nav-controls = render 'main_links' diff --git a/changelogs/unreleased/52527-harden-wiki-against-missing-last-version.yml b/changelogs/unreleased/52527-harden-wiki-against-missing-last-version.yml new file mode 100644 index 00000000000..c7c33e47d01 --- /dev/null +++ b/changelogs/unreleased/52527-harden-wiki-against-missing-last-version.yml @@ -0,0 +1,5 @@ +--- +title: Fix a bug displaying certain wiki pages +merge_request: 22377 +author: +type: fixed diff --git a/spec/models/wiki_page_spec.rb b/spec/models/wiki_page_spec.rb index b4732ec0cd5..821946a47e5 100644 --- a/spec/models/wiki_page_spec.rb +++ b/spec/models/wiki_page_spec.rb @@ -457,6 +457,12 @@ describe WikiPage do end describe '#historical?' do + let(:page) { wiki.find_page('Update') } + let(:old_version) { page.versions.last.id } + let(:old_page) { wiki.find_page('Update', old_version) } + let(:latest_version) { page.versions.first.id } + let(:latest_page) { wiki.find_page('Update', latest_version) } + before do create_page('Update', 'content') @page = wiki.find_page('Update') @@ -468,23 +474,27 @@ describe WikiPage do end it 'returns true when requesting an old version' do - old_version = @page.versions.last.id - old_page = wiki.find_page('Update', old_version) - - expect(old_page.historical?).to eq true + expect(old_page.historical?).to be_truthy end it 'returns false when requesting latest version' do - latest_version = @page.versions.first.id - latest_page = wiki.find_page('Update', latest_version) - - expect(latest_page.historical?).to eq false + expect(latest_page.historical?).to be_falsy end it 'returns false when version is nil' do - latest_page = wiki.find_page('Update', nil) + expect(latest_page.historical?).to be_falsy + end - expect(latest_page.historical?).to eq false + it 'returns false when the last version is nil' do + expect(old_page).to receive(:last_version) { nil } + + expect(old_page.historical?).to be_falsy + end + + it 'returns false when the version is nil' do + expect(old_page).to receive(:version) { nil } + + expect(old_page.historical?).to be_falsy end end