Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

wikiリダイレクトのリセット機能 #11 #22

Open
wants to merge 6 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions app/controllers/wiki_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ def show
@content.current_version? &&
Redmine::WikiFormatting.supports_section_edit?

@redirects_to_self = WikiRedirect.where(:redirects_to => @page.title, :redirects_to_wiki_id => @page.wiki_id)
respond_to do |format|
format.html
format.api
Expand Down
40 changes: 40 additions & 0 deletions app/controllers/wiki_redirects_controller.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
# frozen_string_literal: true

# Redmine - project management software
# Copyright (C) 2006-2020 Jean-Philippe Lang
#
# This program is free software; you can redistribute it and/or
# modify it under the terms of the GNU General Public License
# as published by the Free Software Foundation; either version 2
# of the License, or (at your option) any later version.
#
# This program is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
# GNU General Public License for more details.
#
# You should have received a copy of the GNU General Public License
# along with this program; if not, write to the Free Software
# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.

class WikiRedirectsController < ApplicationController
before_action :find_wiki_redirect, :authorize

def destroy
if @wiki_redirect.destroy
flash[:notice] = l(:notice_successful_delete)
redirect_to project_wiki_page_path(@page.project, @page.title)
end
end

private

def find_wiki_redirect
@project = Project.find(params[:project_id])
@page = Wiki.find_page(params[:wiki_page_id], project: @project)
@wiki_redirect=WikiRedirect.where(redirects_to: @page.title).find(params[:id])
render_404 unless @wiki_redirect
rescue ActiveRecord::RecordNotFound
render_404
end
end
5 changes: 5 additions & 0 deletions app/views/wiki/show.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,11 @@
<%= link_to_if_authorized(l(:button_lock), {:action => 'protect', :id => @page.title, :protected => 1}, :method => :post, :class => 'icon icon-lock') if [email protected]? %>
<%= link_to_if_authorized(l(:button_unlock), {:action => 'protect', :id => @page.title, :protected => 0}, :method => :post, :class => 'icon icon-unlock') if @page.protected? %>
<%= link_to_if_authorized(l(:button_rename), {:action => 'rename', :id => @page.title}, :class => 'icon icon-move') %>
<% if User.current.allowed_to?(:rename_wiki_pages, @project) %>
<% @redirects_to_self.map { |redirect| %>
<%= link_to(l(:button_delete_redirect, :page_title => WikiPage.pretty_title(redirect.title)), {:controller => 'wiki_redirects', :action => 'destroy', :project_id => @project.identifier, :wiki_page_id => @page.title, :id => redirect.id}, :method => :delete, :class => 'icon icon-link-break') %>
<% } %>
Comment on lines +19 to +21
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<% @redirects_to_self.map { |redirect| %>
<%= link_to(l(:button_delete_redirect, :page_title => WikiPage.pretty_title(redirect.title)), {:controller => 'wiki_redirects', :action => 'destroy', :project_id => @project.identifier, :wiki_page_id => @page.title, :id => redirect.id}, :method => :delete, :class => 'icon icon-link-break') %>
<% } %>
<% @redirects_to_self.map do |redirect| %>
<%= link_to(l(:button_delete_redirect, :page_title => WikiPage.pretty_title(redirect.title)), {:controller => 'wiki_redirects', :action => 'destroy', :project_id => @project.identifier, :wiki_page_id => @page.title, :id => redirect.id}, :method => :delete, :class => 'icon icon-link-break') %>
<% end %>

細かいのですが、ここのブロックはdo...endの方が他の書き方にあっていて良いと思います。

<% end %>
<%= link_to_if_authorized(l(:button_delete), {:action => 'destroy', :id => @page.title}, :method => :delete, :data => {:confirm => l(:text_are_you_sure)}, :class => 'icon icon-del') %>
<% else %>
<%= link_to_if_authorized(l(:button_rollback), {:action => 'edit', :id => @page.title, :version => @content.version }, :class => 'icon icon-cancel') %>
Expand Down
1 change: 1 addition & 0 deletions config/locales/de.yml
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,7 @@ de:
button_create_and_continue: Anlegen und weiter
button_delete: Löschen
button_delete_my_account: Mein Benutzerkonto löschen
button_delete_redirect: "Lösche Umleitung von %{page_title}"
button_download: Herunterladen
button_edit: Bearbeiten
button_edit_associated_wikipage: "Zugehörige Wikiseite bearbeiten: %{page_title}"
Expand Down
1 change: 1 addition & 0 deletions config/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -1176,6 +1176,7 @@ en:
button_unarchive: Unarchive
button_reset: Reset
button_rename: Rename
button_delete_redirect: "Delete redirect from %{page_title}"
button_change_password: Change password
button_copy: Copy
button_copy_and_follow: Copy and follow
Expand Down
1 change: 1 addition & 0 deletions config/locales/ja.yml
Original file line number Diff line number Diff line change
Expand Up @@ -852,6 +852,7 @@ ja:
button_unarchive: アーカイブ解除
button_reset: リセット
button_rename: 名前変更
button_delete_redirect: "%{page_title}からのリダイレクトを削除"
button_change_password: パスワード変更
button_copy: コピー
button_copy_and_follow: コピー後表示
Expand Down
1 change: 1 addition & 0 deletions config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,7 @@

match 'wiki/index', :controller => 'wiki', :action => 'index', :via => :get
resources :wiki, :except => [:index, :create], :as => 'wiki_page' do
resources :redirects, controller: 'wiki_redirects', only: :destroy
member do
get 'rename'
post 'rename'
Expand Down
2 changes: 1 addition & 1 deletion lib/redmine/preparation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ def self.prepare
map.permission :view_wiki_edits, {:wiki => [:history, :diff, :annotate]}, :read => true
map.permission :export_wiki_pages, {:wiki => [:export]}, :read => true
map.permission :edit_wiki_pages, :wiki => [:new, :edit, :update, :preview, :add_attachment], :attachments => :upload
map.permission :rename_wiki_pages, {:wiki => :rename}, :require => :member
map.permission :rename_wiki_pages, {:wiki => :rename, :wiki_redirects => :destroy}, :require => :member
map.permission :delete_wiki_pages, {:wiki => [:destroy, :destroy_version]}, :require => :member
map.permission :delete_wiki_pages_attachments, {}
map.permission :view_wiki_page_watchers, {}, :read => true
Expand Down
43 changes: 43 additions & 0 deletions test/functional/wiki_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,49 @@ def test_show_protected_page_shoud_show_locked_badge
end
end

def test_show_delete_redirect_links
@request.session[:user_id] = 2

wiki_page = WikiPage.find_by(title: 'CookBook_documentation')
wiki_page.title = 'Old_Cookbook'
wiki_page.save

wiki_page.title = 'New_Cookbook'
wiki_page.save

cookbook_doc_redirect = WikiRedirect.find_by(title: 'CookBook_documentation', redirects_to: 'New_Cookbook')
old_cookbook_redirect = WikiRedirect.find_by(title: 'Old_Cookbook', redirects_to: 'New_Cookbook')

get :show, :params => {:project_id => 'ecookbook', :id => 'New_Cookbook'}

assert_select '.drdn-items' do
assert_select 'a.icon-link-break[href=?]',
"/projects/ecookbook/wiki/New_Cookbook/redirects/#{cookbook_doc_redirect.id}",
text: 'Delete redirect from CookBook documentation'
assert_select 'a.icon-link-break[href=?]',
"/projects/ecookbook/wiki/New_Cookbook/redirects/#{old_cookbook_redirect.id}",
text: 'Delete redirect from Old Cookbook'
end
end

def test_hide_delete_redirect_links_without_permission
@request.session[:user_id] = 2

wiki_page = WikiPage.find_by(title: 'CookBook_documentation')
wiki_page.title = 'Old_Cookbook'
wiki_page.save

project = wiki_page.wiki.project
role = User.find(2).members.find_by(project: project).roles.first
role.remove_permission! :rename_wiki_pages

get :show, :params => {:project_id => 'ecookbook', :id => 'Old_Cookbook'}

assert_select '.drdn-items' do
assert_select 'a.icon-link-break', count: 0
end
end

def test_get_new
@request.session[:user_id] = 2

Expand Down
81 changes: 81 additions & 0 deletions test/functional/wiki_redirects_controller_test.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
# frozen_string_literal: true

# Redmine - project management software
# Copyright (C) 2006-2020 Jean-Philippe Lang
#
# This program is free software; you can redistribute it and/or
# modify it under the terms of the GNU General Public License
# as published by the Free Software Foundation; either version 2
# of the License, or (at your option) any later version.
#
# This program is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
# GNU General Public License for more details.
#
# You should have received a copy of the GNU General Public License
# along with this program; if not, write to the Free Software
# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.

require File.expand_path('../../test_helper', __FILE__)

class WikiRedirectsControllerTest < Redmine::ControllerTest
fixtures :projects, :users, :email_addresses, :roles, :members, :member_roles,
:enabled_modules, :wikis, :wiki_pages, :wiki_contents,
:wiki_content_versions, :attachments,
:issues, :issue_statuses, :trackers

def setup
User.current = nil
@request.session[:user_id] = 1
end

def test_destroy
wiki_page = WikiPage.find(2)
old_title = wiki_page.title
wiki_page.title = 'Test'
wiki_page.save

wiki_redirect = WikiRedirect.find_by(title: old_title, redirects_to: 'Test')

delete :destroy, params: {id: wiki_redirect.id, project_id: wiki_page.wiki.project_id, wiki_page_id: 'Test'}

assert_redirected_to '/projects/ecookbook/wiki/Test'
assert_equal 'Successful deletion.', flash[:notice]
assert_not WikiRedirect.exists?(id: wiki_redirect.id)
end

def test_destroy_without_permission
@request.session[:user_id] = User.generate!.id
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • test_destroyとtest_destroy_without_permissionの間で結局何の権限の違いによって結果が違うのかが分かりづらい(rename_wiki_pages権限の有無によって違うはず)
  • test_destroyとtest_destroy_without_permissionで別のユーザーを使っているため、rename_wiki_pages権限以外の前提にも違い出てしまっている

という2点が気になりました。
前提(@request.session[:user_id] = 2)は合わせた上で、権限無しのテストでだけ Role.find(1).remove_permission! :rename_wiki_pages という書き方のほうが個人的には好きです。

diff --git a/test/functional/wiki_redirects_controller_test.rb b/test/functional/wiki_redirects_controller_test.rb
index 7949ec3b5..e9ae9f2f7 100644
--- a/test/functional/wiki_redirects_controller_test.rb
+++ b/test/functional/wiki_redirects_controller_test.rb
@@ -31,6 +31,8 @@ class WikiRedirectsControllerTest < Redmine::ControllerTest
   end
 
   def test_destroy
+    @request.session[:user_id] = 2
+
     wiki_page = WikiPage.find(2)
     old_title = wiki_page.title
     wiki_page.title = 'Test'
@@ -45,8 +47,9 @@ class WikiRedirectsControllerTest < Redmine::ControllerTest
     assert_not WikiRedirect.exists?(id: wiki_redirect.id)
   end
 
-  def test_destroy_without_permission
-    @request.session[:user_id] = User.generate!.id
+  def test_destroy_without_rename_wiki_pages_permission
+    @request.session[:user_id] = 2
+    Role.find(1).remove_permission! :rename_wiki_pages # User 2 role
 
     wiki_page = WikiPage.find(2)
     old_title = wiki_page.title

(気になった点としてコメントしただけなので、この書き方でも問題なくコミットされると思います。)


wiki_page = WikiPage.find(2)
old_title = wiki_page.title
wiki_page.title = 'Test'
wiki_page.save

wiki_redirect = WikiRedirect.find_by(title: old_title, redirects_to: 'Test')

delete :destroy, params: {id: wiki_redirect.id, project_id: wiki_page.wiki.project_id, wiki_page_id: 'Test'}

assert_response :forbidden
assert WikiRedirect.exists?(id: wiki_redirect.id)
end

def test_invalid_redirect_should_respond_with_404
wiki_page = WikiPage.find(1)
old_title = wiki_page.title
wiki_page.title = 'New_Title'
wiki_page.save

other_wiki_page = WikiPage.find(2)
other_wiki_page.title = 'Other_New_Title'
other_wiki_page.save

wiki_redirect = WikiRedirect.find_by(title: old_title, redirects_to: 'New_Title')

delete :destroy, params: {id: wiki_redirect.id, project_id: other_wiki_page.wiki.project_id, wiki_page_id: 'Other_New_Title'}

assert_response :not_found
assert WikiRedirect.exists?(id: wiki_redirect.id)
end
end