From 9d6c00f994bb3b5c9bee2bb0fcb29a93d841316b Mon Sep 17 00:00:00 2001 From: Gary Paige Date: Mon, 15 Sep 2014 14:04:59 -0500 Subject: [PATCH] multiple delete of images --- CHANGELOG.md | 6 +- app/assets/javascripts/init.js | 2 + .../javascripts/jquery.image_actions.js | 49 ++++++++++ app/assets/stylesheets/panamax.css.scss | 50 ++++++++++ .../stylesheets/panamax/images.css.scss | 96 +++++++++++++++---- .../stylesheets/panamax/service_details.scss | 6 ++ app/controllers/images_controller.rb | 23 +++++ app/models/local_image.rb | 26 +++++ app/views/images/index.html.haml | 47 +++++---- config/routes.rb | 6 +- spec/controllers/images_controller_spec.rb | 52 ++++++++++ spec/javascripts/fixtures/images.html.haml | 15 +++ spec/javascripts/jquery.image_actions_spec.js | 76 +++++++++++++++ spec/models/local_image_spec.rb | 76 +++++++++++++++ .../fixtures/images_representation.json | 6 +- 15 files changed, 497 insertions(+), 39 deletions(-) create mode 100644 app/assets/javascripts/jquery.image_actions.js create mode 100644 spec/javascripts/fixtures/images.html.haml create mode 100644 spec/javascripts/jquery.image_actions_spec.js diff --git a/CHANGELOG.md b/CHANGELOG.md index 51bb674a..2fc5a031 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,11 @@ All notable changes to this project will be documented in this file. Latest ------------------ ### Fixed -- don't show scrollbar on envirnment variable field (#347) +- Don't show scrollbar on environment variable field (#347) + +### Added +- Edit Service Name +- Remove Multiple Images 0.2.0 - 2014-09-09 ------------------ diff --git a/app/assets/javascripts/init.js b/app/assets/javascripts/init.js index 23422cee..6996b0e0 100644 --- a/app/assets/javascripts/init.js +++ b/app/assets/javascripts/init.js @@ -45,6 +45,8 @@ $('#template_flow button.preview').previewTemplate(); + $('#images_flow section.image').imageActions(); + $('header.application h1 li:last-of-type').editApplicationName(); var enableNewItem = function(addedItem) { diff --git a/app/assets/javascripts/jquery.image_actions.js b/app/assets/javascripts/jquery.image_actions.js new file mode 100644 index 00000000..f6cb49ff --- /dev/null +++ b/app/assets/javascripts/jquery.image_actions.js @@ -0,0 +1,49 @@ +(function($) { + $.PMX.WatchImageSelections = function($el, options) { + var base = this; + + base.$el = $el; + + base.defaultOptions = { + allSelector: 'input#all', + observerSelector: 'input[type=checkbox]', + submitSelector: 'button[type=submit]', + deleteCheckbox: '.images input[type=checkbox]' + }; + + base.init = function () { + base.options = $.extend({}, base.defaultOptions, options); + base.bindEvents(); + }; + + base.bindEvents = function() { + base.$el.on('change', base.options.allSelector, base.checkboxStateHandler); + base.$el.on('change', base.options.observerSelector, base.submitStateHandler); + }; + + base.submitStateHandler = function(e) { + var checked = base.$el.find(base.options.deleteCheckbox).filter(':checked'), + submitButton = base.$el.find(base.options.submitSelector); + + if (checked.length > 0) { + submitButton.prop('disabled', false); + } else { + submitButton.prop('disabled', true); + } + }; + + base.checkboxStateHandler = function(e) { + var $target = $(e.currentTarget), + checked = $target.is(':checked'); + + base.$el.find(base.options.deleteCheckbox).prop('checked', checked); + }; + }; + + $.fn.imageActions = function(options) { + return this.each(function() { + (new $.PMX.WatchImageSelections($(this), options)).init(); + }); + }; + +})(jQuery); diff --git a/app/assets/stylesheets/panamax.css.scss b/app/assets/stylesheets/panamax.css.scss index 6f4aed36..c3e51fc4 100644 --- a/app/assets/stylesheets/panamax.css.scss +++ b/app/assets/stylesheets/panamax.css.scss @@ -2,6 +2,7 @@ @import 'ctl_base_ui/mixins'; @import 'ctl_base_ui/icons'; + body { min-width: 960px; } @@ -717,3 +718,52 @@ pre.prettyprint { } } + +.styled-check { + width: 20px; + margin: 0 auto; + position: relative; + + input[type=checkbox] { + visibility: hidden; + + &:checked + label:after { + opacity: 1.0; + } + } + + label { + cursor: pointer; + position: absolute; + width: 20px; + height: 20px; + top: 0; + left: 0; + border-radius: 4px; + border: 2px solid $grey; + + &:hover::after { + opacity: 0.33; + } + + &:after { + opacity: 0; + content: ''; + position: absolute; + width: 9px; + height: 5px; + background: transparent; + top: 4px; + left: 4px; + border: 3px solid $medium_grey; + border-top: none; + border-right: none; + + -webkit-transform: rotate(-45deg); + -moz-transform: rotate(-45deg); + -o-transform: rotate(-45deg); + -ms-transform: rotate(-45deg); + transform: rotate(-45deg); + } + } +} diff --git a/app/assets/stylesheets/panamax/images.css.scss b/app/assets/stylesheets/panamax/images.css.scss index f320873b..9de50bdd 100644 --- a/app/assets/stylesheets/panamax/images.css.scss +++ b/app/assets/stylesheets/panamax/images.css.scss @@ -7,13 +7,71 @@ } } +#images_flow { + .select-all { + margin-top: 30px; + float: right; + height: 36px; + width: 110px; + + .label { + float: left; + color: $grey; + padding: 5px 5px; + } + + .styled-check { + float: left; + } + } + + .submit-button { + margin-top: 20px; + width: 165px; + position: relative; + padding-left: 25px; + text-align: left; + text-decoration: none; + float: right; + + &::after { + @include icon-white; + @extend .icon-x; + content: ''; + position: absolute; + left: 9px; + top: 14px; + display: block; + height: 10px; + width: 10px; + } + &:disabled { + background-color: $grey; + + &:hover { + @include box-shadow(0, 0, 0, 0); + border: none; + cursor: default; + } + + &:after { + @include icon-light-grey; + } + } + } +} + +ul.images { + clear: both; +} + ul.images li { border-bottom: $light_grey 1px solid; overflow: auto; padding-bottom: 22px; h3 { - margin: 00; + margin: 0; padding: 22px 0 0 0; } @@ -41,27 +99,29 @@ ul.images li { dd { width: 90%; float: right; - } - } - .actions { - text-align: right; - float: right; - .delete-action { - margin-top: -5px; - float: right; - height: 20px; + .delete-action { + margin-top: -5px; + float: right; + height: 20px; - &:after { - top: 0; - left: 0; - @extend .icon-x-large; - @include icon-light-grey; - } + &:after { + top: 0; + left: 0; + @extend .icon-x-large; + @include icon-light-grey; + } - &:hover:after { - @include icon-red; + &:hover:after { + @include icon-red; + } } } } + .actions { + text-align: right; + float: right; + box-sizing: border-box; + width: 40px; + } } diff --git a/app/assets/stylesheets/panamax/service_details.scss b/app/assets/stylesheets/panamax/service_details.scss index dfd661b1..5fbcc2f2 100644 --- a/app/assets/stylesheets/panamax/service_details.scss +++ b/app/assets/stylesheets/panamax/service_details.scss @@ -5,6 +5,12 @@ .service-details { padding-bottom: 20px; + + a.button-add { + margin: 10px 0; + padding-left: 40px; + padding-right: 20px; + } } .service-help-icon { diff --git a/app/controllers/images_controller.rb b/app/controllers/images_controller.rb index d005968a..6e8dba8d 100644 --- a/app/controllers/images_controller.rb +++ b/app/controllers/images_controller.rb @@ -14,4 +14,27 @@ def destroy rescue => ex handle_exception(ex, redirect: images_url) end + + def destroy_multiple + if params[:delete] + 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 << "

#{message}

" } + end + end end diff --git a/app/models/local_image.rb b/app/models/local_image.rb index 530f0f40..0f1779ed 100644 --- a/app/models/local_image.rb +++ b/app/models/local_image.rb @@ -1,4 +1,10 @@ class LocalImage < BaseImage + PANAMAX_IMAGE_NAMES = ['centurylink/panamax-ui', 'centurylink/panamax-api'] + + def panamax_image? + PANAMAX_IMAGE_NAMES.include?(name) + end + def local? true end @@ -6,4 +12,24 @@ def local? def name tags.first end + + def self.batch_destroy(image_ids) + count = 0 + failed = 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, + failed: failed + } + end end diff --git a/app/views/images/index.html.haml b/app/views/images/index.html.haml index a2e0b723..fd78c3f3 100644 --- a/app/views/images/index.html.haml +++ b/app/views/images/index.html.haml @@ -5,20 +5,33 @@ "Images" ] - unless @images.blank? - %ul.images - - @images.each do |image| - %li - %h3= image.tags.first - %h6= image.tags[1..-1].try(:join, ', ') - %dl - %dt - Image ID: - %dd - =image.id - %dt - Image Size: - %dd - =number_to_human_size(image.virtual_size) - .actions - = link_to 'Delete', image_path(image.id), method: :delete, class: 'delete-action' - + = form_tag destroy_multiple_images_path, method: :delete do + %section.image + = button_tag 'Remove Selected', type: 'submit', class: 'button-negative submit-button', disabled: true + .select-all + .label + Select All + .styled-check + = check_box_tag 'all', 1 + %label{ :for => 'all' } + %ul.images + - @images.each do |image| + %li + %h3= image.tags.first + %h6= image.tags[1..-1].try(:join, ', ') + %dl + %dt + Image ID: + %dd + =image.id + %dt + Image Size: + %dd + =number_to_human_size(image.virtual_size) + .actions + - unless image.panamax_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 diff --git a/config/routes.rb b/config/routes.rb index a9e40732..ec3a6896 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -13,7 +13,11 @@ end end - resources :images, only: [:index, :destroy] + resources :images, only: [:index, :destroy] do + collection do + delete 'destroy_multiple' + end + end resources :template_repos, only: [:index, :create, :destroy] do member do diff --git a/spec/controllers/images_controller_spec.rb b/spec/controllers/images_controller_spec.rb index 4df69c9f..6a65f1e4 100644 --- a/spec/controllers/images_controller_spec.rb +++ b/spec/controllers/images_controller_spec.rb @@ -60,4 +60,56 @@ end end end + + describe 'DELETE #destroy_multiple' do + let(:fake_image) { double(:fake_image, id: 1, destroy: true) } + + before do + 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 'removing single image' do + delete :destroy_multiple, delete: { 'key_1' => 1 } + expect(flash[:notice]).to eq '1 image successfully removed' + end + + it 'removing multiple images' do + delete :destroy_multiple, delete: { 'key_1' => 1, 'key_2' => 2 } + expect(flash[:notice]).to eq '2 images successfully removed' + end + end + + 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 '

oops

' + end + end + + context 'when an error occurs' do + before do + LocalImage.stub(:batch_destroy).and_raise(StandardError, 'oops') + end + + it 'redirects the user to the image page' do + delete :destroy_multiple, delete: { 'key_1' => 1, 'key_2' => 2 } + expect(response).to redirect_to images_url + end + + it 'flashes the error message' do + delete :destroy_multiple, delete: { 'key_1' => 1, 'key_2' => 2 } + expect(flash[:alert]).to eq 'oops' + end + end + end end diff --git a/spec/javascripts/fixtures/images.html.haml b/spec/javascripts/fixtures/images.html.haml new file mode 100644 index 00000000..e2247068 --- /dev/null +++ b/spec/javascripts/fixtures/images.html.haml @@ -0,0 +1,15 @@ +#images_flow + %section.image + = button_tag 'Remove Selected', type: 'submit', class: 'button-negative submit-button disabled' + .select-all + .label + Select All + .styled-check + = check_box_tag 'all', 1 + %label{ :for => 'all' } + %ul.images + %li + .actions + .styled-check + = check_box_tag "delete[1]", 1 + %label{ :for => "delete_1" } diff --git a/spec/javascripts/jquery.image_actions_spec.js b/spec/javascripts/jquery.image_actions_spec.js new file mode 100644 index 00000000..b8796f3c --- /dev/null +++ b/spec/javascripts/jquery.image_actions_spec.js @@ -0,0 +1,76 @@ +describe('$.fn.imageActions', function() { + var subject; + + beforeEach(function() { + fixture.load('images.html'); + subject = new $.PMX.WatchImageSelections($('#images_flow section.image')); + subject.init(); + }); + + describe('clicking "Select All" ', function() { + describe('when not checked', function() { + it('check all of the delete image checkboxes', function() { + var selectAll = $('input#all'); + selectAll.prop('checked', false); + + selectAll.click(); + expect($('.images input[type=checkbox]').filter(':checked').length).toEqual(1); + }); + + it('enables the submit button', function() { + var selectAll = $('input#all'); + selectAll.prop('checked', false); + + selectAll.click(); + expect($('button[type=submit]').prop('disabled')).toBeFalsy(); + }); + }); + + describe('when checked', function() { + it('uncheck all of the delete image checkboxes', function() { + var selectAll = $('input#all'), + allBoxes = $('.images input[type=checkbox]'); + + selectAll.prop('checked', true); + allBoxes.prop('checked', true); + + selectAll.click(); + expect($('.images input[type=checkbox]').filter(':checked').length).toEqual(0); + }); + + it('disables the submit button', function() { + var selectAll = $('input#all'); + selectAll.prop('checked', true); + + selectAll.click(); + expect($('button[type=submit]').first().prop('disabled')).toBeTruthy(); + }); + }); + }); + + describe('clicking an image checkbox', function() { + describe('when not checked', function() { + it('enables the submit button', function() { + var checkbox = $('.images input[type=checkbox]').first(), + submitButton = $('button[type=submit]'); + + checkbox.prop('checked', false); + + checkbox.click(); + expect(submitButton.prop('disabled')).toBeFalsy() + }); + }); + + describe('when checked', function() { + it('disables the submit button', function() { + var checkbox = $('.images input[type=checkbox]').first(), + submitButton = $('button[type=submit]'); + + checkbox.prop('checked', true); + + checkbox.click(); + expect(submitButton.prop('disabled')).toBeTruthy() + }); + }); + }); +}); diff --git a/spec/models/local_image_spec.rb b/spec/models/local_image_spec.rb index 2c7f9605..e0d33f5a 100644 --- a/spec/models/local_image_spec.rb +++ b/spec/models/local_image_spec.rb @@ -7,6 +7,7 @@ let(:attributes) do { 'id' => 77, + 'tags' => ['blah/not-panamax'], 'source' => 'boom/shaka', 'description' => 'this thing goes boom shaka laka', 'star_count' => 127, @@ -23,12 +24,87 @@ end end + describe '#panamax_image?' do + let(:panamax_ui) do + { + 'tags' => ['centurylink/panamax-ui'] + } + end + + let(:panamax_api) do + { + 'tags' => ['centurylink/panamax-api'] + } + end + it 'is true when name is centurylink/panamax-ui' do + pmx_ui = described_class.new(panamax_ui) + expect(pmx_ui.panamax_image?).to be_true + end + + it 'is true when name is centurylink/panamax-api' do + pmx_api = described_class.new(panamax_api) + expect(pmx_api.panamax_image?).to be_true + end + + it 'is false when name is not panamax image' do + expect(subject.panamax_image?).to be_false + end + end + describe '#docker_index_url' do it 'is nil' do expect(subject.docker_index_url).to be_nil end end + 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 it 'is local by default' do expect(subject.status_label).to eql 'Local' diff --git a/spec/support/fixtures/images_representation.json b/spec/support/fixtures/images_representation.json index f93322cf..916f2f18 100644 --- a/spec/support/fixtures/images_representation.json +++ b/spec/support/fixtures/images_representation.json @@ -4,14 +4,16 @@ "virtual_size": "123123", "tags": [ "socialize_api:latest" - ] + ], + "virtual_size": 100 }, { "id": "156edca13ead93fe108e3432294a10d49771822aa802baf8435661ca8f1c0c26", "virtual_size": "123123", "tags": [ "centurylinklabs/socialize-api:latest" - ] + ], + "virtual_size": 100 } ]