added panamax image filtering and more detailed exception reporting

app/assets/stylesheets/panamax/images.css.scss

@@ -53,6 +53,10 @@
         border: none;
         cursor: default;
       }
+
+      &:after {
+        @include icon-light-grey;
+      }
     }
   }
 }

app/controllers/images_controller.rb

@@ -17,11 +17,24 @@ def destroy
 
   def destroy_multiple
     if params[:delete]
-      count = LocalImage.batch_destroy(params[:delete].keys)
-      flash[:notice] = "#{count} #{'image'.pluralize(count)} successfully removed"
+      result = LocalImage.batch_destroy(params[:delete].keys)
+      build_message(result)
     end
     redirect_to images_url
   rescue => ex
     handle_exception(ex, redirect: images_url)
   end
+
+  private
+
+  def build_message(result)
+    count = result[:count]
+    failed = result[:failed]
+    unless count == 0
+      flash[:notice] = "#{count} #{'image'.pluralize(count)} successfully removed"
+    end
+    unless failed.empty?
+      flash[:alert] = failed.inject('') { | memo, message| memo << "<p>#{message}</p>" }
+    end
+  end
 end

app/helpers/images_helper.rb

@@ -0,0 +1,5 @@
+module ImagesHelper
+  def panamax_image?(image)
+    'centurylink/panamax-ui'.match(image.name) || 'centurylink/panamax-api'.match(image.name)
+  end
+end

app/models/local_image.rb

@@ -9,9 +9,21 @@ def name
 
   def self.batch_destroy(image_ids)
     count = 0
-    image_ids.each do |id|
-      count += 1 if LocalImage.find(id).destroy
+    fail_list = image_ids.each_with_object(Set.new) do |id, fail_list|
+      begin
+        image = LocalImage.find_by_id(id)
+        if image.destroy
+          count += 1
+        else
+          fail_list << "#{image.name} failed to be removed"
+        end
+      rescue => ex
+        fail_list << ex.message
+      end
     end
-    count
+    {
+      count: count,
+      failed: fail_list
+    }
   end
 end

app/views/images/index.html.haml

@@ -29,8 +29,9 @@
               %dd
                 =number_to_human_size(image.virtual_size)
             .actions
-              .styled-check
-                = check_box_tag "delete[#{image.id}]", 1
-                %label{ :for => "delete_#{image.id}" }
+              - unless panamax_image? image
+                .styled-check
+                  = check_box_tag "delete[#{image.id}]", 1
+                  %label{ :for => "delete_#{image.id}" }
       %section
         = button_tag 'Remove Selected', type: 'submit', class: 'button-negative submit-button', disabled: true

spec/controllers/images_controller_spec.rb

@@ -65,15 +65,17 @@
     let(:fake_image) { double(:fake_image, id: 1, destroy: true) }
 
     before do
-      LocalImage.stub(:find).and_return(fake_image)
+      LocalImage.stub(:find_by_id).and_return(fake_image)
     end
 
     it 'redirects to the listing page' do
       delete :destroy_multiple, delete: { 'key_1' => 1 }
       expect(response).to redirect_to images_url
     end
 
-    describe 'sets a success notice when ' do
+    it ''
+
+    describe 'sets a success notice when' do
       it 'removing single image' do
         delete :destroy_multiple, delete: { 'key_1' => 1 }
         expect(flash[:notice]).to eq '1 image successfully removed'
@@ -85,10 +87,20 @@
       end
     end
 
-    context 'when an error occurs' do
+    describe 'sets alert notice when' do
+      before do
+        LocalImage.stub(:find_by_id).and_raise(StandardError, 'oops')
+      end
+
+      it 'flashes the error message' do
+        delete :destroy_multiple, delete: { 'key_1' => 1, 'key_2' => 2 }
+        expect(flash[:alert]).to eq '<p>oops</p>'
+      end
+    end
 
+    context 'when an error occurs' do
       before do
-        LocalImage.stub(:find).and_raise(StandardError, 'oops')
+        LocalImage.stub(:batch_destroy).and_raise(StandardError, 'oops')
       end
 
       it 'redirects the user to the image page' do

spec/helpers/images_helper_spec.rb

@@ -0,0 +1,29 @@
+require 'spec_helper'
+
+describe ImagesHelper do
+  describe '#panamax_image?' do
+    let(:panamax_ui) do
+      double(:panamax_ui, id: 1, name: 'centurylink/panamax-ui')
+    end
+
+    let(:panamax_api) do
+      double(:panamax_api, id: 2, name: 'centurylink/panamax-api')
+    end
+
+    let(:not_panamax) do
+      double(:not_panamax, id: 3, name: 'blah/blah-api')
+    end
+
+    it 'returns true if image name is centurylink/panamax-ui' do
+      expect(panamax_image? panamax_ui).to be_true
+    end
+
+    it 'returns true if image name is centurylink/panamax-api' do
+      expect(panamax_image? panamax_api).to be_true
+    end
+
+    it 'returns false if image name is not a panamax image' do
+      expect(panamax_image? not_panamax).to be_false
+    end
+  end
+end

spec/models/local_image_spec.rb

@@ -29,8 +29,52 @@
     end
   end
 
-  describe '#batch_destroy' do
+  describe '.batch_destroy' do
+    describe 'when successful' do
+      let(:fake_image) { double(:fake_image, id: 1, destroy: true) }
 
+      before do
+        LocalImage.stub(:find_by_id).and_return(fake_image)
+      end
+
+      it 'calls #destroy on all the image provided' do
+        fake_image.should_receive(:destroy)
+        LocalImage.batch_destroy [1]
+      end
+
+      it 'returns the count of successfully removed images' do
+        result = LocalImage.batch_destroy [1, 2, 3, 4, 5]
+        expect(result[:count]).to eq 5
+      end
+    end
+
+    describe 'with failures' do
+      let(:fake_image) { double(:fake_image, name: 'bad_image', destroy: false) }
+
+      before do
+        LocalImage.stub(:find_by_id).and_return(fake_image)
+      end
+
+      it 'returns the set of failed images' do
+        result = LocalImage.batch_destroy [1]
+        expect(result[:count]). to eq 0
+        expect(result[:failed].to_a).to match_array ['bad_image failed to be removed']
+      end
+    end
+
+    describe 'with errors' do
+      let(:fake_image) { double(:fake_image, id: 1, destroy: true) }
+
+      before do
+        LocalImage.stub(:find_by_id).and_return(fake_image)
+        fake_image.stub(:destroy).and_raise(StandardError, 'oops')
+      end
+
+      it 'returns the set of failed error messages' do
+        result = LocalImage.batch_destroy [1]
+        expect(result[:failed].to_a).to match_array ['oops']
+      end
+    end
   end
 
   describe '#status_label' do