diff --git a/app/controllers/attachments_controller.rb b/app/controllers/attachments_controller.rb index 788bab94d..2851f91a6 100644 --- a/app/controllers/attachments_controller.rb +++ b/app/controllers/attachments_controller.rb @@ -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 diff --git a/app/controllers/documents_controller.rb b/app/controllers/documents_controller.rb index dbf9cd8e5..c6c93f9df 100644 --- a/app/controllers/documents_controller.rb +++ b/app/controllers/documents_controller.rb @@ -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 diff --git a/app/controllers/issues_controller.rb b/app/controllers/issues_controller.rb index 916229cbc..b5009ce3f 100644 --- a/app/controllers/issues_controller.rb +++ b/app/controllers/issues_controller.rb @@ -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) diff --git a/app/controllers/versions_controller.rb b/app/controllers/versions_controller.rb index 3a2221761..c269432f3 100644 --- a/app/controllers/versions_controller.rb +++ b/app/controllers/versions_controller.rb @@ -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' } diff --git a/app/controllers/wiki_controller.rb b/app/controllers/wiki_controller.rb index 04bc33a82..221f4aa81 100644 --- a/app/controllers/wiki_controller.rb +++ b/app/controllers/wiki_controller.rb @@ -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 diff --git a/app/helpers/attachments_helper.rb b/app/helpers/attachments_helper.rb index ebf417bab..29cdb9790 100644 --- a/app/helpers/attachments_helper.rb +++ b/app/helpers/attachments_helper.rb @@ -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 diff --git a/app/models/attachment.rb b/app/models/attachment.rb index 3bcc266bc..2ba75a3fd 100644 --- a/app/models/attachment.rb +++ b/app/models/attachment.rb @@ -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 diff --git a/app/models/document.rb b/app/models/document.rb index 627a2418f..95c3a52c8 100644 --- a/app/models/document.rb +++ b/app/models/document.rb @@ -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}"}, diff --git a/app/models/issue.rb b/app/models/issue.rb index 7488850af..c8befa727 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -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 diff --git a/app/models/message.rb b/app/models/message.rb index acb300f46..080e757b3 100644 --- a/app/models/message.rb +++ b/app/models/message.rb @@ -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'], diff --git a/app/models/version.rb b/app/models/version.rb index e379f4b05..1fd0d1710 100644 --- a/app/models/version.rb +++ b/app/models/version.rb @@ -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] diff --git a/app/models/wiki_page.rb b/app/models/wiki_page.rb index 2416fab74..0d96cc047 100644 --- a/app/models/wiki_page.rb +++ b/app/models/wiki_page.rb @@ -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) diff --git a/app/views/attachments/_links.rhtml b/app/views/attachments/_links.rhtml index 9aae909fe..19ab6734a 100644 --- a/app/views/attachments/_links.rhtml +++ b/app/views/attachments/_links.rhtml @@ -3,14 +3,14 @@
<%= link_to_attachment attachment, :class => 'icon icon-attachment' -%> <%= h(" - #{attachment.description}") unless attachment.description.blank? %> (<%= number_to_human_size attachment.filesize %>) - <% 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] %> <% end %>
diff --git a/app/views/documents/show.rhtml b/app/views/documents/show.rhtml index aa90c5518..4d18a7791 100644 --- a/app/views/documents/show.rhtml +++ b/app/views/documents/show.rhtml @@ -12,7 +12,7 @@<%= link_to l(:label_attachment_new), {}, :onclick => "Element.show('add_attachment_form'); Element.hide(this); Element.scrollTo('add_attachment_form'); return false;", diff --git a/app/views/issues/show.rhtml b/app/views/issues/show.rhtml index 6d6c41a33..5c174e05b 100644 --- a/app/views/issues/show.rhtml +++ b/app/views/issues/show.rhtml @@ -67,9 +67,7 @@ end %> <%= textilizable @issue, :description, :attachments => @issue.attachments %> -<% 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? %>
<%= file.digest %> | <% if delete_allowed %>- <%= 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 %> | <% end %>