AttachmentsController now handles attachments deletion.

git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@2116 e93f8b46-1217-0410-a6f0-8f06a7374b81
This commit is contained in:
Jean-Philippe Lang 2008-12-09 16:54:46 +00:00
parent 2b6e332318
commit 5d2899ee1b
24 changed files with 168 additions and 84 deletions

View File

@ -1,5 +1,5 @@
# redMine - project management software
# Copyright (C) 2006-2007 Jean-Philippe Lang
# Redmine - project management software
# Copyright (C) 2006-2008 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
@ -17,7 +17,11 @@
class AttachmentsController < ApplicationController
before_filter :find_project
before_filter :read_authorize, :except => :destroy
before_filter :delete_authorize, :only => :destroy
verify :method => :post, :only => :destroy
def show
if @attachment.is_diff?
@diff = File.new(@attachment.diskfile, "rb").read
@ -37,19 +41,32 @@ class AttachmentsController < ApplicationController
send_file @attachment.diskfile, :filename => filename_for_content_disposition(@attachment.filename),
:type => @attachment.content_type,
:disposition => (@attachment.image? ? 'inline' : 'attachment')
end
def destroy
# Make sure association callbacks are called
@attachment.container.attachments.delete(@attachment)
redirect_to :back
rescue ::ActionController::RedirectBackError
redirect_to :controller => 'projects', :action => 'show', :id => @project
end
private
def find_project
@attachment = Attachment.find(params[:id])
# Show 404 if the filename in the url is wrong
raise ActiveRecord::RecordNotFound if params[:filename] && params[:filename] != @attachment.filename
@project = @attachment.project
permission = @attachment.container.is_a?(Version) ? :view_files : "view_#{@attachment.container.class.name.underscore.pluralize}".to_sym
allowed = User.current.allowed_to?(permission, @project)
allowed ? true : (User.current.logged? ? render_403 : require_login)
rescue ActiveRecord::RecordNotFound
render_404
end
def read_authorize
@attachment.visible? ? true : deny_access
end
def delete_authorize
@attachment.deletable? ? true : deny_access
end
end

View File

@ -70,11 +70,6 @@ class DocumentsController < ApplicationController
Mailer.deliver_attachments_added(attachments) if !attachments.empty? && Setting.notified_events.include?('document_added')
redirect_to :action => 'show', :id => @document
end
def destroy_attachment
@document.attachments.find(params[:attachment_id]).destroy
redirect_to :action => 'show', :id => @document
end
private
def find_project

View File

@ -18,7 +18,7 @@
class IssuesController < ApplicationController
menu_item :new_issue, :only => :new
before_filter :find_issue, :only => [:show, :edit, :reply, :destroy_attachment]
before_filter :find_issue, :only => [:show, :edit, :reply]
before_filter :find_issues, :only => [:bulk_edit, :move, :destroy]
before_filter :find_project, :only => [:new, :update_form, :preview]
before_filter :authorize, :except => [:index, :changes, :gantt, :calendar, :preview, :update_form, :context_menu]
@ -313,17 +313,6 @@ class IssuesController < ApplicationController
@issues.each(&:destroy)
redirect_to :action => 'index', :project_id => @project
end
def destroy_attachment
a = @issue.attachments.find(params[:attachment_id])
a.destroy
journal = @issue.init_journal(User.current)
journal.details << JournalDetail.new(:property => 'attachment',
:prop_key => a.id,
:old_value => a.filename)
journal.save
redirect_to :action => 'show', :id => @issue
end
def gantt
@gantt = Redmine::Helpers::Gantt.new(params)

View File

@ -37,12 +37,6 @@ class VersionsController < ApplicationController
redirect_to :controller => 'projects', :action => 'settings', :tab => 'versions', :id => @project
end
def destroy_file
@version.attachments.find(params[:attachment_id]).destroy
flash[:notice] = l(:notice_successful_delete)
redirect_to :controller => 'projects', :action => 'list_files', :id => @project
end
def status_by
respond_to do |format|
format.html { render :action => 'show' }

View File

@ -20,7 +20,7 @@ require 'diff'
class WikiController < ApplicationController
before_filter :find_wiki, :authorize
verify :method => :post, :only => [:destroy, :destroy_attachment, :protect], :redirect_to => { :action => :index }
verify :method => :post, :only => [:destroy, :protect], :redirect_to => { :action => :index }
helper :attachments
include AttachmentsHelper
@ -187,13 +187,6 @@ class WikiController < ApplicationController
redirect_to :action => 'index', :page => @page.title
end
def destroy_attachment
@page = @wiki.find_page(params[:page])
return render_403 unless editable?
@page.attachments.find(params[:attachment_id]).destroy
redirect_to :action => 'index', :page => @page.title
end
private
def find_wiki

View File

@ -16,10 +16,15 @@
# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
module AttachmentsHelper
# displays the links to a collection of attachments
def link_to_attachments(attachments, options = {})
if attachments.any?
render :partial => 'attachments/links', :locals => {:attachments => attachments, :options => options}
# Displays view/delete links to the attachments of the given object
# Options:
# :author -- author names are not displayed if set to false
def link_to_attachments(container, options = {})
options.assert_valid_keys(:author)
if container.attachments.any?
options = {:deletable => container.attachments_deletable?, :author => true}.merge(options)
render :partial => 'attachments/links', :locals => {:attachments => container.attachments, :options => options}
end
end

View File

@ -98,6 +98,14 @@ class Attachment < ActiveRecord::Base
container.project
end
def visible?(user=User.current)
container.attachments_visible?(user)
end
def deletable?(user=User.current)
container.attachments_deletable?(user)
end
def image?
self.filename =~ /\.(jpe?g|gif|png)$/i
end

View File

@ -18,7 +18,7 @@
class Document < ActiveRecord::Base
belongs_to :project
belongs_to :category, :class_name => "Enumeration", :foreign_key => "category_id"
has_many :attachments, :as => :container, :dependent => :destroy
acts_as_attachable :delete_permission => :manage_documents
acts_as_searchable :columns => ['title', "#{table_name}.description"], :include => :project
acts_as_event :title => Proc.new {|o| "#{l(:label_document)}: #{o.title}"},

View File

@ -26,13 +26,13 @@ class Issue < ActiveRecord::Base
belongs_to :category, :class_name => 'IssueCategory', :foreign_key => 'category_id'
has_many :journals, :as => :journalized, :dependent => :destroy
has_many :attachments, :as => :container, :dependent => :destroy
has_many :time_entries, :dependent => :delete_all
has_and_belongs_to_many :changesets, :order => "#{Changeset.table_name}.committed_on ASC, #{Changeset.table_name}.id ASC"
has_many :relations_from, :class_name => 'IssueRelation', :foreign_key => 'issue_from_id', :dependent => :delete_all
has_many :relations_to, :class_name => 'IssueRelation', :foreign_key => 'issue_to_id', :dependent => :delete_all
acts_as_attachable :after_remove => :attachment_removed
acts_as_customizable
acts_as_watchable
acts_as_searchable :columns => ['subject', "#{table_name}.description", "#{Journal.table_name}.notes"],
@ -261,4 +261,15 @@ class Issue < ActiveRecord::Base
def to_s
"#{tracker} ##{id}: #{subject}"
end
private
# Callback on attachment deletion
def attachment_removed(obj)
journal = init_journal(User.current)
journal.details << JournalDetail.new(:property => 'attachment',
:prop_key => obj.id,
:old_value => obj.filename)
journal.save
end
end

View File

@ -19,7 +19,7 @@ class Message < ActiveRecord::Base
belongs_to :board
belongs_to :author, :class_name => 'User', :foreign_key => 'author_id'
acts_as_tree :counter_cache => :replies_count, :order => "#{Message.table_name}.created_on ASC"
has_many :attachments, :as => :container, :dependent => :destroy
acts_as_attachable
belongs_to :last_reply, :class_name => 'Message', :foreign_key => 'last_reply_id'
acts_as_searchable :columns => ['subject', 'content'],

View File

@ -19,7 +19,8 @@ class Version < ActiveRecord::Base
before_destroy :check_integrity
belongs_to :project
has_many :fixed_issues, :class_name => 'Issue', :foreign_key => 'fixed_version_id'
has_many :attachments, :as => :container, :dependent => :destroy
acts_as_attachable :view_permission => :view_files,
:delete_permission => :manage_files
validates_presence_of :name
validates_uniqueness_of :name, :scope => [:project_id]

View File

@ -21,7 +21,7 @@ require 'enumerator'
class WikiPage < ActiveRecord::Base
belongs_to :wiki
has_one :content, :class_name => 'WikiContent', :foreign_key => 'page_id', :dependent => :destroy
has_many :attachments, :as => :container, :dependent => :destroy
acts_as_attachable :delete_permission => :delete_wiki_pages_attachments
acts_as_tree :order => 'title'
acts_as_event :title => Proc.new {|o| "#{l(:label_wiki)}: #{o.title}"},
@ -111,6 +111,10 @@ class WikiPage < ActiveRecord::Base
def editable_by?(usr)
!protected? || usr.allowed_to?(:protect_wiki_pages, wiki.project)
end
def attachments_deletable?(usr=User.current)
editable_by?(usr) && super(usr)
end
def parent_title
@parent_title || (self.parent && self.parent.pretty_title)

View File

@ -3,14 +3,14 @@
<p><%= link_to_attachment attachment, :class => 'icon icon-attachment' -%>
<%= h(" - #{attachment.description}") unless attachment.description.blank? %>
<span class="size">(<%= number_to_human_size attachment.filesize %>)</span>
<% if options[:delete_url] %>
<%= link_to image_tag('delete.png'), options[:delete_url].update({:attachment_id => attachment}),
<% if options[:deletable] %>
<%= link_to image_tag('delete.png'), {:controller => 'attachments', :action => 'destroy', :id => attachment},
:confirm => l(:text_are_you_sure),
:method => :post,
:class => 'delete',
:title => l(:button_delete) %>
<% end %>
<% unless options[:no_author] %>
<% if options[:author] %>
<span class="author"><%= attachment.author %>, <%= format_time(attachment.created_on) %></span>
<% end %>
</p>

View File

@ -12,7 +12,7 @@
</div>
<h3><%= l(:label_attachment_plural) %></h3>
<%= link_to_attachments @attachments, :delete_url => (authorize_for('documents', 'destroy_attachment') ? {:controller => 'documents', :action => 'destroy_attachment', :id => @document} : nil) %>
<%= link_to_attachments @document %>
<% if authorize_for('documents', 'add_attachment') %>
<p><%= link_to l(:label_attachment_new), {}, :onclick => "Element.show('add_attachment_form'); Element.hide(this); Element.scrollTo('add_attachment_form'); return false;",

View File

@ -67,9 +67,7 @@ end %>
<%= textilizable @issue, :description, :attachments => @issue.attachments %>
</div>
<% if @issue.attachments.any? %>
<%= link_to_attachments @issue.attachments, :delete_url => (authorize_for('issues', 'destroy_attachment') ? {:controller => 'issues', :action => 'destroy_attachment', :id => @issue} : nil) %>
<% end %>
<%= link_to_attachments @issue %>
<% if authorize_for('issue_relations', 'new') || @issue.relations.any? %>
<hr />

View File

@ -15,7 +15,7 @@
<div class="wiki">
<%= textilizable(@topic.content, :attachments => @topic.attachments) %>
</div>
<%= link_to_attachments @topic.attachments, :no_author => true %>
<%= link_to_attachments @topic, :author => false %>
</div>
<br />
@ -31,7 +31,7 @@
<div class="message reply">
<h4><%=h message.subject %> - <%= authoring message.created_on, message.author %></h4>
<div class="wiki"><%= textilizable message, :content, :attachments => message.attachments %></div>
<%= link_to_attachments message.attachments, :no_author => true %>
<%= link_to_attachments message, :author => false %>
</div>
<% end %>
<% end %>

View File

@ -4,7 +4,7 @@
<h2><%=l(:label_attachment_plural)%></h2>
<% delete_allowed = authorize_for('versions', 'destroy_file') %>
<% delete_allowed = User.current.allowed_to?(:manage_files, @project) %>
<table class="list">
<thead><tr>
@ -30,7 +30,8 @@
<td align="center"><small><%= file.digest %></small></td>
<% if delete_allowed %>
<td align="center">
<%= link_to_if_authorized image_tag('delete.png'), {:controller => 'versions', :action => 'destroy_file', :id => version, :attachment_id => file}, :confirm => l(:text_are_you_sure), :method => :post %>
<%= link_to image_tag('delete.png'), {:controller => 'attachments', :action => 'destroy', :id => file},
:confirm => l(:text_are_you_sure), :method => :post %>
</td>
<% end %>
</tr>

View File

@ -28,7 +28,7 @@
<%= render(:partial => "wiki/content", :locals => {:content => @content}) %>
<%= link_to_attachments @page.attachments, :delete_url => ((@editable && authorize_for('wiki', 'destroy_attachment')) ? {:controller => 'wiki', :action => 'destroy_attachment', :page => @page.title} : nil) %>
<%= link_to_attachments @page %>
<% if @editable && authorize_for('wiki', 'add_attachment') %>
<p><%= link_to l(:label_attachment_new), {}, :onclick => "Element.show('add_attachment_form'); Element.hide(this); Element.scrollTo('add_attachment_form'); return false;",

View File

@ -35,7 +35,7 @@ Redmine::AccessControl.map do |map|
:queries => :index,
:reports => :issue_report}, :public => true
map.permission :add_issues, {:issues => :new}
map.permission :edit_issues, {:issues => [:edit, :reply, :bulk_edit, :destroy_attachment]}
map.permission :edit_issues, {:issues => [:edit, :reply, :bulk_edit]}
map.permission :manage_issue_relations, {:issue_relations => [:new, :destroy]}
map.permission :add_issue_notes, {:issues => [:edit, :reply]}
map.permission :edit_issue_notes, {:journals => :edit}, :require => :loggedin
@ -67,12 +67,12 @@ Redmine::AccessControl.map do |map|
end
map.project_module :documents do |map|
map.permission :manage_documents, {:documents => [:new, :edit, :destroy, :add_attachment, :destroy_attachment]}, :require => :loggedin
map.permission :manage_documents, {:documents => [:new, :edit, :destroy, :add_attachment]}, :require => :loggedin
map.permission :view_documents, :documents => [:index, :show, :download]
end
map.project_module :files do |map|
map.permission :manage_files, {:projects => :add_file, :versions => :destroy_file}, :require => :loggedin
map.permission :manage_files, {:projects => :add_file}, :require => :loggedin
map.permission :view_files, :projects => :list_files, :versions => :download
end
@ -83,7 +83,7 @@ Redmine::AccessControl.map do |map|
map.permission :view_wiki_pages, :wiki => [:index, :special]
map.permission :view_wiki_edits, :wiki => [:history, :diff, :annotate]
map.permission :edit_wiki_pages, :wiki => [:edit, :preview, :add_attachment]
map.permission :delete_wiki_pages_attachments, :wiki => :destroy_attachment
map.permission :delete_wiki_pages_attachments, {}
map.permission :protect_wiki_pages, {:wiki => :protect}, :require => :member
end

View File

@ -76,4 +76,33 @@ class AttachmentsControllerTest < Test::Unit::TestCase
get :download, :id => 7
assert_redirected_to 'account/login'
end
def test_destroy_issue_attachment
issue = Issue.find(3)
@request.session[:user_id] = 2
assert_difference 'issue.attachments.count', -1 do
post :destroy, :id => 1
end
# no referrer
assert_redirected_to 'projects/show/ecookbook'
assert_nil Attachment.find_by_id(1)
j = issue.journals.find(:first, :order => 'created_on DESC')
assert_equal 'attachment', j.details.first.property
assert_equal '1', j.details.first.prop_key
assert_equal 'error281.txt', j.details.first.old_value
end
def test_destroy_wiki_page_attachment
@request.session[:user_id] = 2
assert_difference 'Attachment.count', -1 do
post :destroy, :id => 3
end
end
def test_destroy_without_permission
post :destroy, :id => 3
assert_redirected_to '/login'
assert Attachment.find_by_id(3)
end
end

View File

@ -713,17 +713,4 @@ class IssuesControllerTest < Test::Unit::TestCase
assert_equal 2, TimeEntry.find(1).issue_id
assert_equal 2, TimeEntry.find(2).issue_id
end
def test_destroy_attachment
issue = Issue.find(3)
a = issue.attachments.size
@request.session[:user_id] = 2
post :destroy_attachment, :id => 3, :attachment_id => 1
assert_redirected_to 'issues/show/3'
assert_nil Attachment.find_by_id(1)
issue.reload
assert_equal((a-1), issue.attachments.size)
j = issue.journals.find(:first, :order => 'created_on DESC')
assert_equal 'attachment', j.details.first.property
end
end

View File

@ -251,11 +251,4 @@ class WikiControllerTest < Test::Unit::TestCase
assert_response :success
assert_template 'edit'
end
def test_destroy_attachment
@request.session[:user_id] = 2
assert_difference 'Attachment.count', -1 do
post :destroy_attachment, :id => 1, :page => 'Page_with_an_inline_image', :attachment_id => 3
end
end
end

View File

@ -0,0 +1,2 @@
require File.dirname(__FILE__) + '/lib/acts_as_attachable'
ActiveRecord::Base.send(:include, Redmine::Acts::Attachable)

View File

@ -0,0 +1,57 @@
# Redmine - project management software
# Copyright (C) 2006-2008 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.
module Redmine
module Acts
module Attachable
def self.included(base)
base.extend ClassMethods
end
module ClassMethods
def acts_as_attachable(options = {})
cattr_accessor :attachable_options
self.attachable_options = {}
attachable_options[:view_permission] = options.delete(:view_permission) || "view_#{self.name.pluralize.underscore}".to_sym
attachable_options[:delete_permission] = options.delete(:delete_permission) || "edit_#{self.name.pluralize.underscore}".to_sym
has_many :attachments, options.merge(:as => :container,
:order => "#{Attachment.table_name}.created_on",
:dependent => :destroy)
send :include, Redmine::Acts::Attachable::InstanceMethods
end
end
module InstanceMethods
def self.included(base)
base.extend ClassMethods
end
def attachments_visible?(user=User.current)
user.allowed_to?(self.class.attachable_options[:view_permission], self.project)
end
def attachments_deletable?(user=User.current)
user.allowed_to?(self.class.attachable_options[:delete_permission], self.project)
end
module ClassMethods
end
end
end
end
end