diff --git a/app/controllers/batch_update_placements_controller.rb b/app/controllers/batch_update_placements_controller.rb index 741afb4..a4dccb2 100644 --- a/app/controllers/batch_update_placements_controller.rb +++ b/app/controllers/batch_update_placements_controller.rb @@ -1,8 +1,8 @@ class BatchUpdatePlacementsController < ApplicationController - + before_filter :find_plan_by_id before_filter :verify_params - + include PlanStaleDataChecking after_filter :create_plan_modification before_filter :check_plan_freshness @@ -11,8 +11,8 @@ class BatchUpdatePlacementsController < ApplicationController def create PlanRow.update_placements_by_ids(params[:placement_ids], params[:attributes]) render :nothing => true - rescue => e - render :status => 400, :json => e.message.to_json + rescue PlanRow::BatchUpdatePlacementsError => placement_errors + render :status => 400, :json => placement_errors.message.to_json end private diff --git a/app/models/plan_row.rb b/app/models/plan_row.rb index 94c3cb0..6d4d3ef 100644 --- a/app/models/plan_row.rb +++ b/app/models/plan_row.rb @@ -97,14 +97,16 @@ class PlanRow < ActiveRecord::Base def self.update_placements_by_ids(placement_ids, attributes) transaction do Placement.find(placement_ids).each do |placement| - placement.attributes = placement.attributes.merge(attributes) - placement.save! + placement.update_attributes!(attributes) end end - rescue ActiveRecord::RecordInvalid => e - placement = e.record - errors = { placement.id => placement.errors.full_messages } - raise BatchUpdatePlacementsError, errors + rescue ActiveRecord::RecordInvalid => placement_exception + placement_errors = placement_exception.record.errors + errors_by_attr = placement_errors.inject(Hash.new{|h,k| h[k]=[]}) do |hash, err| + hash[err.first] << err.last + hash + end + raise BatchUpdatePlacementsError, errors_by_attr end def assigned_contacts diff --git a/public/javascripts/app/plans/batch_editor.js b/public/javascripts/app/plans/batch_editor.js index 1635a80..d1868c2 100644 --- a/public/javascripts/app/plans/batch_editor.js +++ b/public/javascripts/app/plans/batch_editor.js @@ -62,7 +62,7 @@ var BatchEditor = Class.create({ _selector: function(colType) { return 'td.'+colType; }, - + _getCell: function(row, colType) { return row.down(this._selector(colType)); }, @@ -110,12 +110,9 @@ var BatchEditor = Class.create({ }, failureHandler: function(response) { - // FIXME: boo-hoo! refactor this!!! - var errors = $H(response.responseJSON).values().first(); - errors.each(function(error) { - var column = this.columns.first(); - if (error.startsWith('End date')) { column = this.columns.last(); } - column.setError(error); + var errors = $H(response.responseJSON); + errors.each(function(pair){ + this.columns.invoke('setError', pair.key, pair.value); }.bind(this)); }, @@ -123,7 +120,7 @@ var BatchEditor = Class.create({ this.columns.invoke('revert'); this._exitBatchEditMode(); }, - + _exitBatchEditMode: function() { this.plan.stateMachine.sendEvent('exitBatchEditMode'); this.submitButton.removeClassName('active'); @@ -133,7 +130,7 @@ var BatchEditor = Class.create({ _addObservers: function(){ this.clickHandler = this.onClickDispatcher.bindAsEventListener(this); this.planElem.observe('click', this.clickHandler); - + this.columnChangeHandler = function() { this.submitButton.addClassName('active'); }.bind(this); @@ -170,6 +167,7 @@ BatchEditor.Column = Class.create({ this._insertInputElem(attributeName, activate); this._highlightMatchingCells(); + this.attributeName = attributeName; }, _insertInputElem: function(attributeName, activate) { @@ -193,9 +191,11 @@ BatchEditor.Column = Class.create({ return this.inputElem.serialize(); }, - setError: function(message) { - this.inputElem.title = message; - this.inputElem.addClassName('error'); + setError: function(attr_name, messages) { + if (this.attributeName.match(attr_name)){ + this.inputElem.title = messages.join(', '); + this.inputElem.addClassName('error'); + } }, clearError: function() { diff --git a/test/functional/batch_update_placements_controller_test.rb b/test/functional/batch_update_placements_controller_test.rb index 3d085f1..a9347be 100644 --- a/test/functional/batch_update_placements_controller_test.rb +++ b/test/functional/batch_update_placements_controller_test.rb @@ -1,6 +1,5 @@ require File.dirname(__FILE__) + '/../test_helper' - class BatchUpdatePlacementsTest < MmsTestCase def setup super @@ -74,14 +73,14 @@ class BatchUpdatePlacementsTest < MmsTestCase context "when validations on placements fail" do setup do @payload[:attributes] = { :start_date => '10/22/2008', :end_date => '10/02/2008' } - post :create, @payload end should_respond_with 400 - should "return an error msg with the validation failure msg" do - assert_match /End date must not be earlier than start date/, @response.body + should "return JSON of the errors" do + error = {:end_date => ['must not be earlier than start date']} + assert_equal(error.to_json, @response.body) end end diff --git a/test/js_unit/plans/batch_editor_column_test.html b/test/js_unit/plans/batch_editor_column_test.html index 8ebcd2d..facf620 100644 --- a/test/js_unit/plans/batch_editor_column_test.html +++ b/test/js_unit/plans/batch_editor_column_test.html @@ -130,10 +130,17 @@ assertEqual(inputElem.serialize(), column.serialize()); }}, - testSetErrorShouldAddErrorClassAndSetTitle: function() { with(this) { - column.setError('oops!'); - assertEqual('oops!', inputElem.title); - assert(inputElem.hasClassName('error')); + testSetErrorShouldSetTitleAndClassWhenAttributeNameMatches: function () { with(this) { + this.column.setError('ad_type_name', ['is dumb', 'is wrong']); + + assert(this.inputElem.hasClassName('error')); + assertEqual('is dumb, is wrong', this.inputElem.title); + }}, + + testSetErrorShouldNotDoAnythingWhenAttributeNameDoesNotMatch: function () { with(this) { + this.column.setError('start_date', ['error', 'message']); + + assert(!this.inputElem.hasClassName('error')); }}, testClearErrorShouldRemoveErrorClassAndClearTitle: function() { with(this) { diff --git a/test/js_unit/plans/batch_editor_test.html b/test/js_unit/plans/batch_editor_test.html index 4e9183e..bea9901 100644 --- a/test/js_unit/plans/batch_editor_test.html +++ b/test/js_unit/plans/batch_editor_test.html @@ -290,24 +290,20 @@ }); }}, - testOnSubmitFailureAddHilightToInputElement: function() { with(this) { - fireMouseEvent($('ad_type_1'), 'click'); - - var adType1Input = $('ad_type_1').down('input'); + testOnSubmitFailureShouldCallSetErrorOnColumns: function() { with(this) { + startDateCol = new BatchEditor.Column($('placement_1'), [$('placement_3')], + '.start_date_col', 'start_date_string'); + endDateCol = new BatchEditor.Column($('placement_1'), [$('placement_3')], + '.end_date_col', 'end_date_string'); + this.batchEditor.columns = [ startDateCol, endDateCol ]; var rsp = { - responseJSON: { - 1: ['foo bar'], - 3: ['foo bar'] - } + responseJSON: {'start_date': ['is not a valid date']} }; - adType1Input.value = 'foo/bar'; - wait(300, function() { - submitButton.click(); - Ajax.mock_requests[0].options.onFailure(rsp); - - assert(adType1Input.hasClassName('error'), "doesn't have error class"); - assertEqual('foo bar', adType1Input.title) - }); + + mock(startDateCol).expects('setError').with('start_date', ['is not a valid date']); + mock(endDateCol).expects('setError').with('start_date', ['is not a valid date']); + + this.batchEditor.failureHandler(rsp); }} }); diff --git a/test/unit/plan_row_test.rb b/test/unit/plan_row_test.rb index b60171f..950dad3 100644 --- a/test/unit/plan_row_test.rb +++ b/test/unit/plan_row_test.rb @@ -764,13 +764,14 @@ class PlanRowTest < MmsTestCase end end - should "return a hash of errors keyed on the placement_id with a failed validation" do + should "return a hash of placement errors" do begin as_actor(virgil) do PlanRow.update_placements_by_ids(@ids, @attributes) end - rescue Exception => e - assert @ids.size, e.message.size + rescue Exception => placement_errors + assert_equal({'end_date' => ['must not be earlier than start date']}, + placement_errors.message) end end end diff --git a/vendor/plugins/ersatz b/vendor/plugins/ersatz index 5cbf426..be4891d 160000 --- a/vendor/plugins/ersatz +++ b/vendor/plugins/ersatz @@ -1 +1 @@ -Subproject commit 5cbf4265a2b1837f5abec2020410c426e8859b5c +Subproject commit be4891df7d5b652944d113205a8ac8db19dda04a