diff --git a/app/controllers/bulky/updates_controller.rb b/app/controllers/bulky/updates_controller.rb index 9654c84..c48aaa1 100644 --- a/app/controllers/bulky/updates_controller.rb +++ b/app/controllers/bulky/updates_controller.rb @@ -20,8 +20,12 @@ def update private + helper_method def model_name + params.require(:model) + end + def model - @model ||= params[:model].classify.constantize + @model ||= model_name.classify.constantize end def ids diff --git a/app/helpers/bulky/form_helper.rb b/app/helpers/bulky/form_helper.rb index 83381d4..83d7703 100644 --- a/app/helpers/bulky/form_helper.rb +++ b/app/helpers/bulky/form_helper.rb @@ -2,7 +2,7 @@ module Bulky module FormHelper def whitelisted_attributes_collection(model) - model.accessible_attributes.select(&:present?).map {|a| [a.titleize, a]} + model.bulky_attributes.map(&:to_s).map {|a| [a.titleize, a]} end end diff --git a/app/views/bulky/updates/edit.html.haml b/app/views/bulky/updates/edit.html.haml index ddda0cc..e47acd8 100644 --- a/app/views/bulky/updates/edit.html.haml +++ b/app/views/bulky/updates/edit.html.haml @@ -1,6 +1,6 @@ %h1 Bulky -= form_for :bulk, url: bulky_update_path(model: params[:model]), html: {method: :put} do |f| += form_for :bulk, url: bulky_update_path(model: model_name), html: {method: :put} do |f| %p = f.select :attribute, whitelisted_attributes_collection(model), {include_blank: "Choose an Attribute"}, name: "attribute", onchange: "document.getElementById('bulk_value').name = 'bulk[' + this.value + ']'" diff --git a/bulky.gemspec b/bulky.gemspec index aba7901..76ac705 100644 --- a/bulky.gemspec +++ b/bulky.gemspec @@ -19,6 +19,7 @@ Gem::Specification.new do |s| s.add_dependency "resque", "~> 1.25" s.add_dependency "haml", ">= 4.0" + s.add_development_dependency "pry" s.add_development_dependency "sqlite3" s.add_development_dependency "mysql2", "~> 0.3.16" s.add_development_dependency "rspec-rails", "~> 2.14.2" diff --git a/lib/bulky.rb b/lib/bulky.rb index 2edf0b9..377d189 100644 --- a/lib/bulky.rb +++ b/lib/bulky.rb @@ -24,4 +24,5 @@ def log_bulk_update(ids, updates) end +require 'bulky/model' require 'bulky/updater' diff --git a/lib/bulky/model.rb b/lib/bulky/model.rb new file mode 100644 index 0000000..5b1d746 --- /dev/null +++ b/lib/bulky/model.rb @@ -0,0 +1,14 @@ +module Bulky + module Model + + def self.extended(base) + base.class_attribute :bulky_attributes + base.bulky_attributes = [] + end + + def bulky(*attributes) + self.bulky_attributes += attributes + end + + end +end diff --git a/lib/bulky/updater.rb b/lib/bulky/updater.rb index ff15967..57c4e33 100644 --- a/lib/bulky/updater.rb +++ b/lib/bulky/updater.rb @@ -1,6 +1,8 @@ module Bulky class Updater + attr_reader :model, :bulk_update + def self.perform(model_name, update_id, bulk_update_id) model = model_name.constantize.find(update_id) new(model, bulk_update_id).update! @@ -9,26 +11,30 @@ def self.perform(model_name, update_id, bulk_update_id) def initialize(model, bulk_update_id) @bulk_update = Bulky::BulkUpdate.find(bulk_update_id) @model = model - @updates = @bulk_update.updates end - def update! - @model.tap do - @log = @bulk_update.updated_records.build { |r| r.updatable = @model } + def log + @log ||= bulk_update.updated_records.build { |r| r.updatable = model } + end - begin - @model.attributes = @updates - @log.updatable_changes = @model.changes - @model.save! - rescue => e - @log.error_message = e.message - @log.error_backtrace = e.backtrace.join("\n") - raise e - ensure - @log.save! - end - end + def strong_updates + ActionController::Parameters.new(bulk_update.updates) end + def updates + @updates ||= strong_updates.permit(*model.bulky_attributes) + end + + def update! + model.attributes = updates + log.updatable_changes = model.changes + model.save! + rescue => e + log.error_message = e.message + log.error_backtrace = e.backtrace.join("\n") + raise e + ensure + log.save! + end end end diff --git a/spec/bulky/model_spec.rb b/spec/bulky/model_spec.rb new file mode 100644 index 0000000..7bcf74d --- /dev/null +++ b/spec/bulky/model_spec.rb @@ -0,0 +1,13 @@ +require 'spec_helper' + +describe Bulky::Model do + + let(:example) { Class.new { extend Bulky::Model } } + + it "allows defining attributes for bulk update" do + example.bulky :foo + expect(example.bulky_attributes).to include :foo + end + +end + diff --git a/spec/bulky/updater_spec.rb b/spec/bulky/updater_spec.rb index aae6dbc..502d6bf 100644 --- a/spec/bulky/updater_spec.rb +++ b/spec/bulky/updater_spec.rb @@ -15,7 +15,7 @@ Account.stub(:find).with(1).and_return(@account = Account.new) Bulky::BulkUpdate.stub(:find).with(5).and_return(@bulk = Bulky::BulkUpdate.new) Bulky::UpdatedRecord.any_instance.stub(:save!).and_return(true) - @account.stub!(:save!).and_return(true) + @account.stub(:save!).and_return(true) end it "will find the class it is supposed to update" do @@ -30,7 +30,7 @@ end it "will update the attributes on the instance with the given updates" do - @account.should_receive(:attributes=).with(updates = {"business" => "Adam Inc"}) + @account.should_receive(:attributes=).with("business" => "Adam Inc") Bulky::BulkUpdate.stub(:find).and_return(@bulk = Bulky::BulkUpdate.new { |b| b.updates = {"business" => "Adam Inc"} }) Bulky::Updater.perform('Account', 1, 5) end @@ -54,18 +54,27 @@ log.updatable_changes.should eq('business' => ['Higher Inc.', 'Fallen Corp']) end + describe "bulky attributes" do + let(:bulk_update) { Bulky::BulkUpdate.create! { |b| b.ids = [1]; b.updates = {"id" => 10} } } + let(:updater) { Bulky::Updater.new(account, bulk_update.id) } + + it "only passes through bulky_attributes as permitted" do + expect(updater.updates).not_to have_key('id') + end + end + describe "that has errors" do - let(:bulk_update) { Bulky::BulkUpdate.create! { |b| b.ids = [1,2]; b.updates = {"age" => 27} } } - let(:updater) { Bulky::Updater.new(account, bulk_update.id) } - let(:log) { Bulky::UpdatedRecord.last } + let(:bulk_update) { Bulky::BulkUpdate.create! { |b| b.ids = [1,2]; b.updates = {"business" => ''} } } + let(:updater) { Bulky::Updater.new(account, bulk_update.id) } + let(:log) { Bulky::UpdatedRecord.last } it "logs any errors that happen when saving the model" do updater.update! rescue nil - log.error_message.should eq("Can't mass-assign protected attributes: age") + log.error_message.should eq("Validation failed: Business can't be blank") end it "reraises any errors that happen when saving the model" do - expect { updater.update! }.to raise_error(ActiveModel::MassAssignmentSecurity::Error) + expect { updater.update! }.to raise_error(ActiveRecord::RecordInvalid) end end end diff --git a/spec/bulky_spec.rb b/spec/bulky_spec.rb index f636dca..aaccde0 100644 --- a/spec/bulky_spec.rb +++ b/spec/bulky_spec.rb @@ -7,7 +7,7 @@ describe ".enqueue_update" do before :each do - Bulky.stub(:log_bulk_update).and_return(mock('BulkUpdate', id: 5, updates: updates)) + Bulky.stub(:log_bulk_update).and_return(double('BulkUpdate', id: 5, updates: updates)) end it "will enqueue a Bulky::Update with the class and updates for each id provided" do diff --git a/spec/dummy/app/models/account.rb b/spec/dummy/app/models/account.rb index 963af56..b1db78c 100644 --- a/spec/dummy/app/models/account.rb +++ b/spec/dummy/app/models/account.rb @@ -1,4 +1,9 @@ class Account < ActiveRecord::Base + + extend Bulky::Model + bulky :business, :contact, :last_contracted_on + validates :business, presence: true - attr_accessible :business, :contact, :last_contacted_on + # attr_accessible :business, :contact, :last_contacted_on + end diff --git a/spec/dummy/config/application.rb b/spec/dummy/config/application.rb index 4bc5ac5..dfc26d0 100644 --- a/spec/dummy/config/application.rb +++ b/spec/dummy/config/application.rb @@ -4,7 +4,6 @@ require "active_record/railtie" require "action_controller/railtie" require "action_mailer/railtie" -require "active_resource/railtie" require "sprockets/railtie" # require "rails/test_unit/railtie" @@ -53,7 +52,7 @@ class Application < Rails::Application # This will create an empty whitelist of attributes available for mass-assignment for all models # in your app. As such, your models will need to explicitly whitelist or blacklist accessible # parameters by using an attr_accessible or attr_protected declaration. - config.active_record.whitelist_attributes = true + # config.active_record.whitelist_attributes = true # Enable the asset pipeline config.assets.enabled = true diff --git a/spec/dummy/config/environments/development.rb b/spec/dummy/config/environments/development.rb index 82c74d1..5085381 100644 --- a/spec/dummy/config/environments/development.rb +++ b/spec/dummy/config/environments/development.rb @@ -1,4 +1,5 @@ Dummy::Application.configure do + config.eager_load = false # Settings specified here will take precedence over those in config/application.rb # In the development environment your application's code is reloaded on @@ -23,11 +24,11 @@ config.action_dispatch.best_standards_support = :builtin # Raise exception on mass assignment protection for Active Record models - config.active_record.mass_assignment_sanitizer = :strict + # config.active_record.mass_assignment_sanitizer = :strict # Log the query plan for queries taking more than this (works # with SQLite, MySQL, and PostgreSQL) - config.active_record.auto_explain_threshold_in_seconds = 0.5 + # config.active_record.auto_explain_threshold_in_seconds = 0.5 # Do not compress assets config.assets.compress = false diff --git a/spec/dummy/config/environments/test.rb b/spec/dummy/config/environments/test.rb index f1a4814..c2155f7 100644 --- a/spec/dummy/config/environments/test.rb +++ b/spec/dummy/config/environments/test.rb @@ -1,4 +1,5 @@ Dummy::Application.configure do + config.eager_load = false # Settings specified here will take precedence over those in config/application.rb # The test environment is used exclusively to run your application's @@ -30,7 +31,7 @@ config.action_mailer.delivery_method = :test # Raise exception on mass assignment protection for Active Record models - config.active_record.mass_assignment_sanitizer = :strict + # config.active_record.mass_assignment_sanitizer = :strict # Print deprecation notices to the stderr config.active_support.deprecation = :stderr diff --git a/spec/helpers/bulky/form_helper_spec.rb b/spec/helpers/bulky/form_helper_spec.rb index 76f16b8..6bf2547 100644 --- a/spec/helpers/bulky/form_helper_spec.rb +++ b/spec/helpers/bulky/form_helper_spec.rb @@ -8,7 +8,7 @@ let(:html_value) { collection.first[0] } it "creates a list of attributes for each whitelisted attribute" do - collection.length.should eq(Account.accessible_attributes.count - 1) + collection.length.should eq(Account.bulky_attributes.count) end it "uses the column name the option" do diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 4504027..e503e24 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -7,6 +7,7 @@ require "capybara/rspec" require "capybara/rails" require "database_cleaner" +require "pry" Rails.backtrace_cleaner.remove_silencers!