From 2ee7f716ae6a677de022da3ef89302a692bb54af Mon Sep 17 00:00:00 2001 From: Julius Date: Wed, 26 Jun 2013 17:56:20 +0200 Subject: [PATCH] Improve delivery validation: StockArticles must not be associated more than once --- app/helpers/application_helper.rb | 8 +++++ app/models/delivery.rb | 9 ++++++ app/views/deliveries/_form.html.haml | 32 +++++++++++--------- app/views/deliveries/add_stock_change.js.erb | 22 +++++++------- app/views/shared/_base_errors.haml | 5 +++ config/locales/de.yml | 2 ++ config/locales/en.yml | 2 ++ 7 files changed, 55 insertions(+), 25 deletions(-) create mode 100644 app/views/shared/_base_errors.haml diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 78a2fc40..167a558d 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -158,5 +158,13 @@ module ApplicationHelper end flash_messages.join("\n").html_safe end + + # render base errors in a form after failed validation + # http://railsapps.github.io/twitter-bootstrap-rails.html + def base_errors resource + return '' if (resource.errors.empty?) or (resource.errors[:base].empty?) + messages = resource.errors[:base].map { |msg| content_tag(:li, msg) }.join + render :partial => 'shared/base_errors', :locals => {:error_messages => messages} + end end diff --git a/app/models/delivery.rb b/app/models/delivery.rb index b93c0567..ae0be007 100644 --- a/app/models/delivery.rb +++ b/app/models/delivery.rb @@ -10,6 +10,7 @@ class Delivery < ActiveRecord::Base scope :recent, :order => 'created_at DESC', :limit => 10 validates_presence_of :supplier_id, :delivered_on + validate :stock_articles_must_be_unique accepts_nested_attributes_for :stock_changes, :allow_destroy => :true @@ -23,6 +24,14 @@ class Delivery < ActiveRecord::Base self.stock_changes.map{|stock_change| stock_change.stock_article.id}.include? article.id end + protected + + def stock_articles_must_be_unique + unless stock_changes.reject{|sc| sc.marked_for_destruction?}.map {|sc| sc.stock_article.id}.uniq!.nil? + errors.add(:base, I18n.t('model.delivery.each_stock_article_must_be_unique')) + end + end + end diff --git a/app/views/deliveries/_form.html.haml b/app/views/deliveries/_form.html.haml index 9169a367..94fff06f 100644 --- a/app/views/deliveries/_form.html.haml +++ b/app/views/deliveries/_form.html.haml @@ -6,14 +6,15 @@ var stock_change = $(this).closest('tr'); stock_change.hide(); // do not remove (to ensure destruction) - unmark_article_unavailable_for_delivery( stock_change.data('id') ); + stock_change.removeAttr('id'); // remove id to allow re-adding + mark_article_for_delivery( stock_change.data('id') ); return false; }); $('.remove_new_stock_change').live('click', function() { var stock_change = $(this).closest('tr'); stock_change.remove(); - unmark_article_unavailable_for_delivery( stock_change.data('id') ); + mark_article_for_delivery( stock_change.data('id') ); return false; }) @@ -62,23 +63,26 @@ }); }); - function mark_article_unavailable_for_delivery(stock_article_id) { + function mark_article_for_delivery(stock_article_id) { var articleTr = $('#stock_article_' + stock_article_id); - articleTr.addClass('unavailable'); - $('.button-add-stock-change', articleTr).attr('disabled', 'disabled'); + if( is_article_available_for_delivery(stock_article_id) ) { + articleTr.removeClass('unavailable'); + $('.button-add-stock-change', articleTr).removeAttr('disabled'); + } + else { + articleTr.addClass('unavailable'); + $('.button-add-stock-change', articleTr).attr('disabled', 'disabled'); + } } - function unmark_article_unavailable_for_delivery(stock_article_id) { - var articleTr = $('#stock_article_' + stock_article_id); - articleTr.removeClass('unavailable'); - $('.button-add-stock-change', articleTr).removeAttr('disabled'); - } - function is_article_unavailable_for_delivery(stock_article_id) { - var articleTr = $('#stock_article_' + stock_article_id); - return articleTr.hasClass('unavailable'); + function is_article_available_for_delivery(stock_article_id) { + return ( 0 == $('#stock_change_stock_article_' + stock_article_id).length ); } = simple_form_for [@supplier, @delivery], validate: true do |f| - = f.hidden_field :supplier_id + = f.error_notification + = base_errors f.object + = f.association :supplier, :as => :hidden + %h2= t '.title_select_stock_articles' .well.well-small .btn-toolbar diff --git a/app/views/deliveries/add_stock_change.js.erb b/app/views/deliveries/add_stock_change.js.erb index 30d7daaf..d2c3a022 100644 --- a/app/views/deliveries/add_stock_change.js.erb +++ b/app/views/deliveries/add_stock_change.js.erb @@ -1,13 +1,5 @@ (function(w) { - if(is_article_unavailable_for_delivery(<%= @stock_change.stock_article.id %>)) { - return false; - } - - mark_article_unavailable_for_delivery(<%= @stock_change.stock_article.id %>); - - var quantity = w.prompt('<%= j(t('.how_many_units', :unit => @stock_change.stock_article.unit, :name => @stock_change.stock_article.name)) %>'); <%# how to properly escape here? %> - if(null === quantity) { - unmark_article_unavailable_for_delivery(<%= @stock_change.stock_article.id %>); // think wisely before changing this: What about double clicks on "deliver" button? + if(!is_article_available_for_delivery(<%= @stock_change.stock_article.id %>)) { return false; } @@ -17,8 +9,16 @@ '<%= j(render(:partial => 'stock_change', :locals => {:stock_change => @stock_change})) %>' ).addClass('success'); + $('#stock_changes').append(stock_change); + mark_article_for_delivery(<%= @stock_change.stock_article.id %>); + updateSort('#stock_changes'); + + var quantity = w.prompt('<%= j(t('.how_many_units', :unit => @stock_change.stock_article.unit, :name => @stock_change.stock_article.name)) %>'); <%# how to properly escape here? %> + if(null === quantity) { + stock_change.remove(); + mark_article_for_delivery(<%= @stock_change.stock_article.id %>); + return false; + } $('input.stock-change-quantity', stock_change).val(quantity); - $('#stock_changes').append(stock_change); - updateSort('#stock_changes'); })(window); diff --git a/app/views/shared/_base_errors.haml b/app/views/shared/_base_errors.haml new file mode 100644 index 00000000..9dd2dd82 --- /dev/null +++ b/app/views/shared/_base_errors.haml @@ -0,0 +1,5 @@ +.alert.alert-error.alert-block + %a.close{:href => '#', :data => {:dismiss => 'alert'}} + = t('ui.marks.close').html_safe + %ul + = error_messages.html_safe diff --git a/config/locales/de.yml b/config/locales/de.yml index 8e22a095..d77ad2b0 100644 --- a/config/locales/de.yml +++ b/config/locales/de.yml @@ -1163,6 +1163,8 @@ de: subject: ! 'Betreff:' title: Nachricht anzeigen model: + delivery: + each_stock_article_must_be_unique: Lieferung darf jeden Lagerartikel höchstens einmal auflisten. membership: no_admin_delete: Mitgliedschaft kann nicht beendet werden. Du bist die letzte Administratorin order_article: diff --git a/config/locales/en.yml b/config/locales/en.yml index f765a35e..26b3fbcc 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1166,6 +1166,8 @@ en: subject: ! 'Subject:' title: Show message model: + delivery: + each_stock_article_must_be_unique: Each stock article must not be listed more than once. membership: no_admin_delete: Membership can not be withdrawn as you are the last administrator. order_article: