Merge pull request #165 from Oktavilla/replace-image-assets

Replace image assets

app/controllers/admin/assets_controller.rb

@@ -95,6 +95,7 @@ def update_multiple
   def update
     @asset = Asset.find(params[:id])
     if @asset.update_attributes(params[:asset])
+      @asset.touch_usages
       flash[:notice] = "#{@asset.full_name} #{t(:saved, :scope => [:app, :admin_general])}"
     end
     respond_with @asset, :location => (params[:return_to] || edit_admin_asset_url(@asset))

app/models/asset.rb

@@ -62,7 +62,7 @@ class Asset
   attr_accessor :file
   validates_presence_of :file, :if => :new_record?
 
-  before_validation :process, :if => :new_record?
+  before_validation :process
   after_destroy :cleanup
 
   after_save proc { |asset| asset.delay.update_tank_indexes }
@@ -88,25 +88,19 @@ def of_the_type(filetype)
   end
 
   def usages
-    @usages ||= _usages.map do |u|
-      u.except('container_id')
-    end.uniq.collect { |u| u['usage_type'].constantize.find(u['usage_id']) }
+    @usages ||= AssetUsages.new(self)
   end
 
-  def remove_usage(association)
-    usage = usage_from_association(association)
-    if _usages.include?(usage)
-      _usages.reject! { |u| u == usage }
-      save
-    end
+  def remove_usage object, context
+    usages.remove object, context
   end
 
-  def add_usage(association)
-    usage = usage_from_association(association)
-    unless _usages.include?(usage)
-      _usages << usage
-      save
-    end
+  def add_usage object, context
+    usages.add object, context
+  end
+
+  def touch_usages
+    usages.each &:touch
   end
 
   class << self
@@ -131,13 +125,17 @@ def filetype_for_extension(_extension)
     end
   end
 
-protected
+  protected
 
-  # before validation on create
   def process
-    extract_attributes_from_file
-    ensure_unique_name
-    store
+    if file.present?
+      extract_attributes_from_file
+      ensure_unique_name
+
+      cleanup unless new_record?
+
+      store
+    end
   end
 
   def extract_attributes_from_file
@@ -168,13 +166,4 @@ def store
   def cleanup
     Porthos.s3_storage.destroy(full_name)
   end
-
-  def usage_from_association(association)
-    {
-      'container_id' => association.id,
-      'usage_type' => association._root_document.class.model_name,
-      'usage_id' => association._root_document.id.to_s
-    }
-  end
-
 end

app/models/asset_usages.rb

@@ -0,0 +1,56 @@
+class AssetUsages
+  include Enumerable
+
+  attr_reader :asset
+
+  def initialize asset
+    @asset = asset
+  end
+
+  def each &block
+    using_objects.each &block
+  end
+
+  def add object, context = nil
+    usage = new_usage object, context
+
+    unless asset_usages.include?(usage)
+      asset_usages << usage
+      save_asset
+    end
+  end
+
+  def remove object, context = nil
+    usage = new_usage object, context
+    asset_usages.reject! { |u| u == usage }
+    save_asset
+  end
+
+  private
+
+  def asset_usages
+    asset._usages
+  end
+
+  def save_asset
+    asset.save
+  end
+
+  def using_objects
+    uniq_usages.collect do |usage|
+      usage["usage_type"].constantize.find usage["usage_id"]
+    end
+  end
+
+  def uniq_usages
+    asset_usages.uniq {|usage| [usage["usage_id"], usage["usage_type"]] }
+  end
+
+  def new_usage object, context
+    {
+      "container_id" => context.to_s,
+      "usage_type" => object.class.model_name,
+      "usage_id" => object.id.to_s
+    }
+  end
+end

app/models/data/asset_association.rb

@@ -87,14 +87,15 @@ def revert_to_asset_attributes
 
   def notify_asset
     if changes.include?(:asset_id)
-      unless asset_id_was.nil?
-        if old_asset = Asset.find(asset_id_was)
-          old_asset.remove_usage(self)
+      if old_asset = Asset.find(asset_id_was)
+        old_asset.remove_usage _root_document, self.id
+      end
+
+      if asset_id.present?
+        if new_asset = Asset.find(asset_id)
+          new_asset.add_usage _root_document, self.id
         end
       end
-      Asset.find(asset_id).tap do |new_asset|
-        new_asset.add_usage(self) if new_asset
-      end if asset_id.present?
     end
   end
 

app/views/admin/assets/edit.html.erb

@@ -25,6 +25,11 @@
           <div class="fullwidth"><%= f.text_field "reference_number", :size => 50 %></div>
           <label for="asset_tag_names">Nyckelord <span class="form_help">dela flera nyckelord med mellanslag</span></label>
           <div class="fullwidth"><%= f.text_field :tag_names, :class => 'asset_tags', :size => 50 %></div>
+
+          <% if @asset.is_a? ImageAsset %>
+            <label for="asset_file">Ändra bildfil</label>
+            <%= f.file_field :file, size: 15, accept: "image/*" %>
+          <% end %>
         </fieldset>
         <fieldset class="submit">
           <%= submit_tag I18n.t(:save) %> <%= I18n.t(:or) %> <%= link_to I18n.t(:cancel), previous_view_path(admin_assets_path) %>
@@ -85,7 +90,13 @@
         <h2>Används på:</h2>
         <ul>
         <% @asset.usages.each do |usage| %>
-          <li><%= link_to usage.title, admin_item_path(usage) %></li>
+          <li>
+            <% if usage.is_a?(Item) %>
+              <%= link_to usage.title, admin_item_path(usage) %>
+            <% else %>
+              <%= link_to usage.name, [:admin, usage] %>
+            <% end %>
+          </li>
         <% end %>
         </ul>
       </div>

lib/porthos/test_helpers/assets_test_helper.rb

@@ -1,13 +1,13 @@
 module PorthosAssetTestHelpers
-  def new_tempfile(type = 'image')
+  def new_tempfile(type = 'image', filename = nil)
     file = case type
     when 'image' then 'image.jpg'
     when 'text'  then 'page.txt'
     end
     tempfile = Tempfile.new(Time.now.to_s)
     tempfile.write IO.read(stub_file_path(file))
     tempfile.rewind
-    uploaded_file = ActionDispatch::Http::UploadedFile.new(:filename => file, :tempfile => tempfile)
+    uploaded_file = ActionDispatch::Http::UploadedFile.new(:filename => (filename || file), :tempfile => tempfile)
     uploaded_file
   end
 
@@ -16,14 +16,27 @@ def stub_file_path(filename = 'page.txt')
     File.join(path, filename)
   end
 
-  def stub_resizor_post(filename = 'image.jpg')
+  def stub_resizor_post(filename = 'image.jpg', returned_id = 1)
+    asset_response = {
+      asset: {
+        id: returned_id,
+        name: "image",
+        extension: "jpg",
+        mime_type: "image/jpeg",
+        height: 500,
+        width: 332,
+        file_size: 666,
+        created_at: "2010-10-23T13:07:25Z"
+      }
+    }
+
     stub_http_request(:post, "https://resizor.com:443/assets.json").with { |request|
       request.body.include?("Content-Disposition: form-data; name=\"file\"; filename=\"#{filename}")
-    }.to_return(:status => 201, :body => '{"asset": { "id":1, "name":"i", "extension":"jpg", "mime_type":"image/jpeg", "height":500, "width":332, "file_size":666, "created_at":"2010-10-23T13:07:25Z"}}')
+    }.to_return :status => 201, :body => asset_response.to_json
   end
 
-  def stub_resizor_delete
-    stub_http_request(:delete, /https\:\/\/resizor.com:443\/assets\/1.json\?api_key=/).to_return(:status => 200)
+  def stub_resizor_delete resizor_id = 1
+    stub_http_request(:delete, /https\:\/\/resizor.com:443\/assets\/#{resizor_id}.json\?api_key=/).to_return(:status => 200)
   end
 
   def stub_s3_put

test/integration/admin/assets_test.rb

@@ -99,4 +99,33 @@ class AssetsTest < ActiveSupport::IntegrationCase
       assert assets_list.has_content?(asset3.title), 'Should display assets3 in the assets list'
     end
   end
+
+
+  test 'updating a image assets image file' do
+    original_resizor_id = 123
+    stub_resizor_post "original_image.jpg", original_resizor_id
+    original_image = new_tempfile "image", "original_image.jpg"
+    asset = FactoryGirl.create :image_asset, :file => original_image
+    stub_resizor_delete original_resizor_id
+
+    new_resizor_id = 543
+    new_image = new_tempfile "image"
+    stub_resizor_post "image.jpg", new_resizor_id
+
+    visit edit_admin_asset_path(asset)
+    assert page_has_image? "#{original_resizor_id}.jpg"
+
+    attach_file 'asset_file', stub_file_path("image.jpg")
+    click_button I18n.t(:save)
+
+    assert page_has_image? "#{new_resizor_id}.jpg"
+  end
+
+  private
+
+  def page_has_image? image_name
+    within ".ImageAsset" do
+      page.has_xpath?("//img[contains(@src, \"#{image_name}\")]")
+    end
+  end
 end

test/support/integration_case.rb

@@ -39,4 +39,4 @@ def logout!
     visit admin_logout_path
   end
 
-end
+end

test/unit/asset_test.rb

@@ -46,6 +46,23 @@ class AssetTest < ActiveSupport::TestCase
       @asset.destroy
       assert_requested :delete, "http://#{Porthos.s3_storage.bucket_name}.s3.amazonaws.com/#{@asset.full_name}?"
     end
+
+    context "updating" do
+      should "cleanup and save new file if file attribute is present" do
+        @asset.expects :store
+        @asset.expects :cleanup
+
+        @asset.update_attributes file: new_tempfile('text')
+      end
+
+      should "not touch stored file if file attribute is empty" do
+        @asset.reload
+        @asset.expects(:store).never
+        @asset.expects(:cleanup).never
+
+        @asset.update_attributes title: "A file"
+      end
+    end
   end
 
   context 'the Asset class' do

test/unit/asset_usages_test.rb

@@ -0,0 +1,51 @@
+require_relative "../test_helper"
+require "minitest/autorun"
+
+describe "AssetUsages" do
+  let(:item){ FactoryGirl.build :item, id: 123 }
+  let(:asset){ FactoryGirl.build :asset, created_by: FactoryGirl.build(:user) }
+  subject{ AssetUsages.new asset }
+
+  it "is initialized with a asset" do
+    subject.asset.must_equal asset
+  end
+
+  it "adds usages to asset" do
+    usage = {
+      "usage_type" => "Item",
+      "usage_id" => "123",
+      "container_id" => "a context",
+    }
+    subject.expects :save_asset
+
+    subject.add item, "a context"
+
+    asset._usages.must_equal [usage]
+  end
+
+  it "deletes usages from asset" do
+    asset.stubs _usages: [{
+      "usage_type" => "Item",
+      "usage_id" => "123",
+      "container_id" => "a context"
+    }]
+    subject.expects :save_asset
+
+    subject.remove item, "a context"
+
+    asset._usages.must_equal []
+  end
+
+  it "is enumerable" do
+    Item.stubs(:find).with("123").returns item
+    asset.stubs _usages: [{
+      "usage_type" => "Item",
+      "usage_id" => "123",
+      "container_id" => "a context"
+    }]
+    usages = []
+    subject.each {|using_object| usages << using_object }
+
+    usages.must_equal [item]
+  end
+end