Edit attachments after upload (#1326).

git-svn-id: http://svn.redmine.org/redmine/trunk@13665 e93f8b46-1217-0410-a6f0-8f06a7374b81

8 files changed, 128 insertions, 5 deletions

diff --git a/app/controllers/attachments_controller.rb b/app/controllers/attachments_controller.rb
index b7f856a1a..924e9a186 100644
--- a/app/controllers/attachments_controller.rb
+++ b/app/controllers/attachments_controller.rb
@@ -16,7 +16,8 @@
 # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
 
 class AttachmentsController < ApplicationController
-  before_filter :find_project, :except => :upload
+  before_filter :find_attachment, :only => [:show, :download, :thumbnail, :destroy]
+  before_filter :find_editable_attachments, :only => [:edit, :update]
   before_filter :file_readable, :read_authorize, :only => [:show, :download, :thumbnail]
   before_filter :delete_authorize, :only => :destroy
   before_filter :authorize_global, :only => :upload
@@ -99,6 +100,19 @@ class AttachmentsController < ApplicationController
     end
   end
 
+  def edit
+  end
+
+  def update
+    if params[:attachments].is_a?(Hash)
+      if Attachment.update_attachments(@attachments, params[:attachments])
+        redirect_back_or_default home_path
+        return
+      end
+    end
+    render :action => 'edit'
+  end
+
   def destroy
     if @attachment.container.respond_to?(:init_journal)
       @attachment.container.init_journal(User.current)
@@ -116,8 +130,9 @@ class AttachmentsController < ApplicationController
     end
   end
 
-private
-  def find_project
+  private
+
+  def find_attachment
     @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
@@ -126,6 +141,27 @@ private
     render_404
   end
 
+  def find_editable_attachments
+    klass = params[:object_type].to_s.singularize.classify.constantize rescue nil
+    unless klass && klass.reflect_on_association(:attachments)
+      render_404
+      return
+    end
+
+    @container = klass.find(params[:object_id])
+    if @container.respond_to?(:visible?) && !@container.visible?
+      render_403
+      return
+    end
+    @attachments = @container.attachments.select(&:editable?)
+    if @container.respond_to?(:project)
+      @project = @container.project
+    end
+    render_404 if @attachments.empty?
+  rescue ActiveRecord::RecordNotFound
+    render_404
+  end
+
   # Checks that the file exists and is readable
   def file_readable
     if @attachment.readable?
diff --git a/app/helpers/attachments_helper.rb b/app/helpers/attachments_helper.rb
index d86fe0dbe..387ed99d7 100644
--- a/app/helpers/attachments_helper.rb
+++ b/app/helpers/attachments_helper.rb
@@ -18,6 +18,15 @@
 # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
 
 module AttachmentsHelper
+
+  def container_attachments_edit_path(container)
+    object_attachments_edit_path container.class.name.underscore.pluralize, container.id
+  end
+
+  def container_attachments_path(container)
+    object_attachments_path container.class.name.underscore.pluralize, container.id
+  end
+
   # Displays view/delete links to the attachments of the given object
   # Options:
   #   :author -- author names are not displayed if set to false
@@ -28,7 +37,12 @@ module AttachmentsHelper
     if container.attachments.any?
       options = {:deletable => container.attachments_deletable?, :author => true}.merge(options)
       render :partial => 'attachments/links',
-        :locals => {:attachments => container.attachments, :options => options, :thumbnails => (options[:thumbnails] && Setting.thumbnails_enabled?)}
+        :locals => {
+          :container => container,
+          :attachments => container.attachments,
+          :options => options,
+          :thumbnails => (options[:thumbnails] && Setting.thumbnails_enabled?)
+        }
     end
   end
 
diff --git a/app/models/attachment.rb b/app/models/attachment.rb
index da82088f2..42690912a 100644
--- a/app/models/attachment.rb
+++ b/app/models/attachment.rb
@@ -166,6 +166,14 @@ class Attachment < ActiveRecord::Base
     end
   end
 
+  def editable?(user=User.current)
+    if container_id
+      container && container.attachments_editable?(user)
+    else
+      author == user
+    end
+  end
+
   def deletable?(user=User.current)
     if container_id
       container && container.attachments_deletable?(user)
@@ -254,6 +262,35 @@ class Attachment < ActiveRecord::Base
     result
   end
 
+  # Updates the filename and description of a set of attachments
+  # with the given hash of attributes. Returns true if all
+  # attachments were updated.
+  #
+  # Example:
+  #   Attachment.update_attachments(attachments, {
+  #     4 => {:filename => 'foo'},
+  #     7 => {:filename => 'bar', :description => 'file description'}
+  #   })
+  #
+  def self.update_attachments(attachments, params)
+    params = params.transform_keys {|key| key.to_i}
+
+    saved = true
+    transaction do
+      attachments.each do |attachment|
+        if p = params[attachment.id]
+          attachment.filename = p[:filename] if p.key?(:filename)
+          attachment.description = p[:description] if p.key?(:description)
+          saved &&= attachment.save
+        end
+      end
+      unless saved
+        raise ActiveRecord::Rollback
+      end
+    end
+    saved
+  end
+
   def self.latest_attach(attachments, filename)
     attachments.sort_by(&:created_on).reverse.detect do |att|
       att.filename.downcase == filename.downcase
diff --git a/app/models/news.rb b/app/models/news.rb
index 19a55bda4..6ade4f4f1 100644
--- a/app/models/news.rb
+++ b/app/models/news.rb
@@ -26,7 +26,8 @@ class News < ActiveRecord::Base
   validates_length_of :summary, :maximum => 255
   attr_protected :id
 
-  acts_as_attachable :delete_permission => :manage_news
+  acts_as_attachable :edit_permission => :manage_news,
+                     :delete_permission => :manage_news
   acts_as_searchable :columns => ['title', 'summary', "#{table_name}.description"],
                      :scope => preload(:project)
   acts_as_event :url => Proc.new {|o| {:controller => 'news', :action => 'show', :id => o.id}}
diff --git a/app/models/project.rb b/app/models/project.rb
index 1a3534217..c21f6ef7e 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -60,6 +60,7 @@ class Project < ActiveRecord::Base
 
   acts_as_nested_set :dependent => :destroy
   acts_as_attachable :view_permission => :view_files,
+                     :edit_permission => :manage_files,
                      :delete_permission => :manage_files
 
   acts_as_customizable
diff --git a/app/models/version.rb b/app/models/version.rb
index c8ac6ca0e..e97512c0c 100644
--- a/app/models/version.rb
+++ b/app/models/version.rb
@@ -22,6 +22,7 @@ class Version < ActiveRecord::Base
   has_many :fixed_issues, :class_name => 'Issue', :foreign_key => 'fixed_version_id', :dependent => :nullify
   acts_as_customizable
   acts_as_attachable :view_permission => :view_files,
+                     :edit_permission => :manage_files,
                      :delete_permission => :manage_files
 
   VERSION_STATUSES = %w(open locked closed)
diff --git a/app/views/attachments/_links.html.erb b/app/views/attachments/_links.html.erb
index d2ab22ced..a8dbd3584 100644
--- a/app/views/attachments/_links.html.erb
+++ b/app/views/attachments/_links.html.erb
@@ -1,4 +1,7 @@
 <div class="attachments">
+<div class="contextual">
+  <%= link_to image_tag('edit.png'), container_attachments_edit_path(container) if attachments.any?(&:editable?) %>
+</div>
 <% for attachment in attachments %>
 <p><%= link_to_attachment attachment, :class => 'icon icon-attachment', :download => true -%>
   <% if attachment.is_text? %>
diff --git a/app/views/attachments/edit.html.erb b/app/views/attachments/edit.html.erb
new file mode 100644
index 000000000..065488c67
--- /dev/null
+++ b/app/views/attachments/edit.html.erb
@@ -0,0 +1,30 @@
+<h2><%= l(:label_attachment_plural) %></h2> 
+
+<%= error_messages_for *@attachments %>
+
+<%= form_tag(container_attachments_path(@container), :method => 'patch') do %>
+  <%= back_url_hidden_field_tag %>
+  <div class="box attachments">
+  <table>
+  <% @attachments.each do |attachment| %>
+    <tr>
+      <td colspan="2">
+        <span class="icon icon-attachment"><%= attachment.filename_was %></span>
+        <span class="size">(<%= number_to_human_size attachment.filesize %>)</span>
+        <span class="author"><%= h(attachment.author) %>, <%= format_time(attachment.created_on) %></span>
+      </td>
+    </tr>
+    <tr id="attachment-<%= attachment.id %>">
+      <td><%= text_field_tag "attachments[#{attachment.id}][filename]", attachment.filename, :size => 40 %></td>
+      <td>
+        <%= text_field_tag "attachments[#{attachment.id}][description]", attachment.description, :size => 80, :placeholder => l(:label_optional_description) %>
+      </td>
+    </tr>
+  <% end %>
+  </table>
+  </div>
+  <p>
+    <%= submit_tag l(:button_save) %>
+    <%= link_to l(:button_cancel), back_url if back_url.present? %>
+  </p>
+<% end %>