From 48fb02e3839c30667733e59415d98c373467876b Mon Sep 17 00:00:00 2001
From: Jean-Philippe Lang <jp_lang@yahoo.fr>
Date: Sun, 20 Jan 2013 11:38:20 +0000
Subject: [PATCH] Split "Manage documents" permission into create, edit and
 delete permissions (#12401).

git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@11206 e93f8b46-1217-0410-a6f0-8f06a7374b81
---
 app/models/document.rb                        |  2 +-
 app/views/documents/index.html.erb            |  2 +-
 app/views/documents/show.html.erb             |  4 +++-
 ...30110122628_split_documents_permissions.rb | 23 +++++++++++++++++++
 lib/redmine.rb                                |  4 +++-
 test/fixtures/roles.yml                       | 13 +++++++----
 test/unit/user_test.rb                        |  2 +-
 7 files changed, 41 insertions(+), 9 deletions(-)
 create mode 100644 db/migrate/20130110122628_split_documents_permissions.rb

diff --git a/app/models/document.rb b/app/models/document.rb
index 3fcdcee46..0c2ce1736 100644
--- a/app/models/document.rb
+++ b/app/models/document.rb
@@ -19,7 +19,7 @@ class Document < ActiveRecord::Base
   include Redmine::SafeAttributes
   belongs_to :project
   belongs_to :category, :class_name => "DocumentCategory", :foreign_key => "category_id"
-  acts_as_attachable :delete_permission => :manage_documents
+  acts_as_attachable :delete_permission => :delete_documents
 
   acts_as_searchable :columns => ['title', "#{table_name}.description"], :include => :project
   acts_as_event :title => Proc.new {|o| "#{l(:label_document)}: #{o.title}"},
diff --git a/app/views/documents/index.html.erb b/app/views/documents/index.html.erb
index 28fc74684..84567741e 100644
--- a/app/views/documents/index.html.erb
+++ b/app/views/documents/index.html.erb
@@ -1,6 +1,6 @@
 <div class="contextual">
 <%= link_to l(:label_document_new), new_project_document_path(@project), :class => 'icon icon-add',
-      :onclick => 'showAndScrollTo("add-document", "document_title"); return false;' if User.current.allowed_to?(:manage_documents, @project) %>
+      :onclick => 'showAndScrollTo("add-document", "document_title"); return false;' if User.current.allowed_to?(:add_documents, @project) %>
 </div>
 
 <div id="add-document" style="display:none;">
diff --git a/app/views/documents/show.html.erb b/app/views/documents/show.html.erb
index b1d58d522..ee5890936 100644
--- a/app/views/documents/show.html.erb
+++ b/app/views/documents/show.html.erb
@@ -1,6 +1,8 @@
 <div class="contextual">
-<% if User.current.allowed_to?(:manage_documents, @project) %>
+<% if User.current.allowed_to?(:edit_documents, @project) %>
 <%= link_to l(:button_edit), edit_document_path(@document), :class => 'icon icon-edit', :accesskey => accesskey(:edit) %>
+<% end %>
+<% if User.current.allowed_to?(:delete_documents, @project) %>
 <%= delete_link document_path(@document) %>
 <% end %>
 </div>
diff --git a/db/migrate/20130110122628_split_documents_permissions.rb b/db/migrate/20130110122628_split_documents_permissions.rb
new file mode 100644
index 000000000..0e010aa5c
--- /dev/null
+++ b/db/migrate/20130110122628_split_documents_permissions.rb
@@ -0,0 +1,23 @@
+class SplitDocumentsPermissions < ActiveRecord::Migration
+  def up
+    # :manage_documents permission split into 3 permissions:
+    # :add_documents, :edit_documents and :delete_documents
+    Role.all.each do |role|
+      if role.has_permission?(:manage_documents)
+        role.add_permission! :add_documents, :edit_documents, :delete_documents
+        role.remove_permission! :manage_documents
+      end
+    end
+  end
+
+  def down
+    Role.all.each do |role|
+      if role.has_permission?(:add_documents) ||
+          role.has_permission?(:edit_documents) ||
+          role.has_permission?(:delete_documents)
+        role.remove_permission! :add_documents, :edit_documents, :delete_documents
+        role.add_permission! :manage_documents
+      end
+    end
+  end
+end
diff --git a/lib/redmine.rb b/lib/redmine.rb
index 12cfe64bf..5b09b77b8 100644
--- a/lib/redmine.rb
+++ b/lib/redmine.rb
@@ -146,7 +146,9 @@ Redmine::AccessControl.map do |map|
   end
 
   map.project_module :documents do |map|
-    map.permission :manage_documents, {:documents => [:new, :create, :edit, :update, :destroy, :add_attachment]}, :require => :loggedin
+    map.permission :add_documents, {:documents => [:new, :create, :add_attachment]}, :require => :loggedin
+    map.permission :edit_documents, {:documents => [:edit, :update, :add_attachment]}, :require => :loggedin
+    map.permission :delete_documents, {:documents => [:destroy]}, :require => :loggedin
     map.permission :view_documents, {:documents => [:index, :show, :download]}, :read => true
   end
 
diff --git a/test/fixtures/roles.yml b/test/fixtures/roles.yml
index 1cacf0100..2f1e57804 100644
--- a/test/fixtures/roles.yml
+++ b/test/fixtures/roles.yml
@@ -38,7 +38,9 @@ roles_001:
     - :manage_news
     - :comment_news
     - :view_documents
-    - :manage_documents
+    - :add_documents
+    - :edit_documents
+    - :delete_documents
     - :view_wiki_pages
     - :export_wiki_pages
     - :view_wiki_edits
@@ -89,7 +91,9 @@ roles_002:
     - :manage_news
     - :comment_news
     - :view_documents
-    - :manage_documents
+    - :add_documents
+    - :edit_documents
+    - :delete_documents
     - :view_wiki_pages
     - :view_wiki_edits
     - :edit_wiki_pages
@@ -131,7 +135,9 @@ roles_003:
     - :manage_news
     - :comment_news
     - :view_documents
-    - :manage_documents
+    - :add_documents
+    - :edit_documents
+    - :delete_documents
     - :view_wiki_pages
     - :view_wiki_edits
     - :edit_wiki_pages
@@ -163,7 +169,6 @@ roles_004:
     - :view_time_entries
     - :comment_news
     - :view_documents
-    - :manage_documents
     - :view_wiki_pages
     - :view_wiki_edits
     - :edit_wiki_pages
diff --git a/test/unit/user_test.rb b/test/unit/user_test.rb
index 7f43cbc82..bf74dd6da 100644
--- a/test/unit/user_test.rb
+++ b/test/unit/user_test.rb
@@ -901,7 +901,7 @@ class UserTest < ActiveSupport::TestCase
       should "authorize nearly everything for admin users" do
         project = Project.find(1)
         assert ! @admin.member_of?(project)
-        %w(edit_issues delete_issues manage_news manage_documents manage_wiki).each do |p|
+        %w(edit_issues delete_issues manage_news add_documents manage_wiki).each do |p|
           assert_equal true, @admin.allowed_to?(p.to_sym, project)
         end
       end