From 647b7f0430d68be2f544a500cdb23b0b665f2801 Mon Sep 17 00:00:00 2001 From: wvengen Date: Wed, 21 May 2014 21:24:03 +0200 Subject: [PATCH] allow to synchronize all articles of a shared supplier --- app/controllers/articles_controller.rb | 21 +++- app/helpers/articles_helper.rb | 1 + app/helpers/suppliers_helper.rb | 8 +- app/models/article.rb | 18 +++- app/models/shared_supplier.rb | 8 ++ app/models/supplier.rb | 28 +++++- app/views/articles/_sync_table.html.haml | 53 ++++++++++ app/views/articles/index.html.haml | 3 +- app/views/articles/sync.html.haml | 97 ++++++------------- app/views/suppliers/_form.html.haml | 2 + app/views/suppliers/show.html.haml | 1 + config/locales/en.yml | 19 ++++ ...40521142651_add_sync_method_to_supplier.rb | 5 + db/schema.rb | 3 +- spec/factories/article.rb | 2 +- spec/models/article_spec.rb | 2 +- 16 files changed, 194 insertions(+), 77 deletions(-) create mode 100644 app/views/articles/_sync_table.html.haml create mode 100644 db/migrate/20140521142651_add_sync_method_to_supplier.rb diff --git a/app/controllers/articles_controller.rb b/app/controllers/articles_controller.rb index 9b26dd8a..e0c32ded 100644 --- a/app/controllers/articles_controller.rb +++ b/app/controllers/articles_controller.rb @@ -227,10 +227,10 @@ class ArticlesController < ApplicationController redirect_to supplier_articles_url(@supplier), :alert => I18n.t('articles.controller.sync.shared_alert', :supplier => @supplier.name) end # sync articles against external database - @updated_articles, @outlisted_articles = @supplier.sync_all + @updated_articles, @outlisted_articles, @new_articles = @supplier.sync_all # convert to db-compatible-string @updated_articles.each {|a, b| a.shared_updated_on = a.shared_updated_on.to_formatted_s(:db)} - if @updated_articles.empty? && @outlisted_articles.empty? + if @updated_articles.empty? && @outlisted_articles.empty? && @new_articles.empty? redirect_to supplier_articles_path(@supplier), :notice => I18n.t('articles.controller.sync.notice') end @ignored_article_count = @supplier.articles.where(order_number: [nil, '']).count @@ -246,8 +246,21 @@ class ArticlesController < ApplicationController end # Update articles - params[:articles].each do |id, attrs| - Article.find(id).update_attributes! attrs + if params[:articles] + params[:articles].each do |id, attrs| + Article.find(id).update_attributes! attrs + end + end + + # Add new articles + if params[:new_articles] + params[:new_articles].each do |attrs| + article = Article.new attrs + article.supplier = @supplier + article.availability = true if @supplier.shared_sync_method == 'all_available' + article.availability = false if @supplier.shared_sync_method == 'all_unavailable' + article.save! + end end end diff --git a/app/helpers/articles_helper.rb b/app/helpers/articles_helper.rb index b299fe9b..ecd45ec3 100644 --- a/app/helpers/articles_helper.rb +++ b/app/helpers/articles_helper.rb @@ -2,6 +2,7 @@ module ArticlesHelper # useful for highlighting attributes, when synchronizing articles def highlight_new(unequal_attributes, attribute) + return unless unequal_attributes unequal_attributes.detect {|a| a == attribute} ? "background-color: yellow" : "" end diff --git a/app/helpers/suppliers_helper.rb b/app/helpers/suppliers_helper.rb index 9876f11d..ffd54216 100644 --- a/app/helpers/suppliers_helper.rb +++ b/app/helpers/suppliers_helper.rb @@ -3,4 +3,10 @@ module SuppliersHelper def associated_supplier_names(shared_supplier) "(#{shared_supplier.suppliers.map(&:name).join(', ')})" end -end \ No newline at end of file + + def shared_sync_method_collection(shared_supplier) + shared_supplier.shared_sync_methods.map do |m| + [t("suppliers.shared_supplier_methods.#{m}"), m] + end + end +end diff --git a/app/models/article.rb b/app/models/article.rb index c3f7e969..5cb26ec8 100644 --- a/app/models/article.rb +++ b/app/models/article.rb @@ -59,7 +59,9 @@ class Article < ActiveRecord::Base validates_numericality_of :price, :greater_than_or_equal_to => 0 validates_numericality_of :unit_quantity, :greater_than => 0 validates_numericality_of :deposit, :tax - validates_uniqueness_of :name, :scope => [:supplier_id, :deleted_at, :type] + #validates_uniqueness_of :name, :scope => [:supplier_id, :deleted_at, :type], if: Proc.new {|a| a.supplier.shared_sync_method.blank? or a.supplier.shared_sync_method == 'import' } + #validates_uniqueness_of :name, :scope => [:supplier_id, :deleted_at, :type, :unit, :unit_quantity] + validate :uniqueness_of_name # Callbacks before_save :update_price_history @@ -214,4 +216,18 @@ class Article < ActiveRecord::Base def price_changed? changed.detect { |attr| attr == 'price' || 'tax' || 'deposit' || 'unit_quantity' } ? true : false end + + # We used have the name unique per supplier+deleted_at+type. With the addition of shared_sync_method all, + # this came in the way, and we now allow duplicate names for the 'all' methods - expecting foodcoops to + # make their own choice among products with different units by making articles available/unavailable. + def uniqueness_of_name + matches = Article.where(name: name, supplier_id: supplier_id, deleted_at: deleted_at, type: type) + matches = matches.where.not(id: id) unless new_record? + if supplier.shared_sync_method.blank? or supplier.shared_sync_method == 'import' + errors.add :name, :taken if matches.any? + else + errors.add :name, :taken_with_unit if matches.where(unit: unit, unit_quantity: unit_quantity).any? + end + end + end diff --git a/app/models/shared_supplier.rb b/app/models/shared_supplier.rb index 86eebda3..dc6ef201 100644 --- a/app/models/shared_supplier.rb +++ b/app/models/shared_supplier.rb @@ -14,5 +14,13 @@ class SharedSupplier < ActiveRecord::Base whitelist = %w(name address phone fax email url delivery_days note) attributes.select { |k,_v| whitelist.include?(k) } end + + # return list of synchronisation methods available for this supplier + def shared_sync_methods + methods = [] + methods += %w(all_available all_unavailable) if shared_articles.count < 200 + methods += %w(import) # perhaps, in the future: if shared_articles.count > 20 + methods + end end diff --git a/app/models/supplier.rb b/app/models/supplier.rb index 4ea5c93a..b4719765 100644 --- a/app/models/supplier.rb +++ b/app/models/supplier.rb @@ -9,12 +9,13 @@ class Supplier < ActiveRecord::Base include ActiveModel::MassAssignmentSecurity attr_accessible :name, :address, :phone, :phone2, :fax, :email, :url, :contact_person, :customer_number, - :delivery_days, :order_howto, :note, :shared_supplier_id, :min_order_quantity + :delivery_days, :order_howto, :note, :shared_supplier_id, :min_order_quantity, :shared_sync_method validates :name, :presence => true, :length => { :in => 4..30 } validates :phone, :presence => true, :length => { :in => 8..25 } validates :address, :presence => true, :length => { :in => 8..50 } validates_length_of :order_howto, :note, maximum: 250 + validate :valid_shared_sync_method validate :uniqueness_of_name scope :undeleted, -> { where(deleted_at: nil) } @@ -22,9 +23,11 @@ class Supplier < ActiveRecord::Base # sync all articles with the external database # returns an array with articles(and prices), which should be updated (to use in a form) # also returns an array with outlisted_articles, which should be deleted + # also returns an array with new articles, which should be added (depending on shared_sync_method) def sync_all updated_articles = Array.new outlisted_articles = Array.new + new_articles = Array.new for article in articles.undeleted # try to find the associated shared_article shared_article = article.shared_article @@ -63,7 +66,21 @@ class Supplier < ActiveRecord::Base outlisted_articles << article end end - return [updated_articles, outlisted_articles] + # Find any new articles, unless the import is manual + unless shared_sync_method == 'import' + for shared_article in shared_supplier.shared_articles + unless articles.undeleted.find_by_order_number(shared_article.number) + new_articles << shared_article.build_new_article(self) + end + end + end + return [updated_articles, outlisted_articles, new_articles] + end + + # default value + def shared_sync_method + return unless shared_supplier + self[:shared_sync_method] || 'import' end def deleted? @@ -79,6 +96,13 @@ class Supplier < ActiveRecord::Base protected + # make sure the shared_sync_method is allowed for the shared supplier + def valid_shared_sync_method + if shared_supplier and !shared_supplier.shared_sync_methods.include?(shared_sync_method) + errors.add :name, :included + end + end + # Make sure, the name is uniq, add usefull message if uniq group is already deleted def uniqueness_of_name supplier = Supplier.where(name: name) diff --git a/app/views/articles/_sync_table.html.haml b/app/views/articles/_sync_table.html.haml new file mode 100644 index 00000000..55abbdf7 --- /dev/null +++ b/app/views/articles/_sync_table.html.haml @@ -0,0 +1,53 @@ +%table.table + %thead + %tr + %th= heading_helper Article, :name + %th= heading_helper Article, :note + %th= heading_helper Article, :manufacturer + %th= heading_helper Article, :origin + %th= heading_helper Article, :unit + %th= heading_helper Article, :unit_quantity, short: true + %th= heading_helper Article, :price + %th= heading_helper Article, :tax + %th= heading_helper Article, :deposit + %th= heading_helper Article, :article_category + %tbody + - articles.each do |changed_article, attrs| + - unless changed_article.new_record? + - article = Article.find(changed_article.id) + %tr{:style => 'color:grey'} + %td= article.name + %td= article.note + %td= article.manufacturer + %td= article.origin + %td= article.unit + %td= article.unit_quantity + %td= number_to_currency article.price + %td= number_to_percentage article.tax + %td= number_to_currency article.deposit + %td= article.article_category.name if article.article_category + %tr + = fields_for "#{field}[]", changed_article do |form| + %td{:style => highlight_new(attrs, :name)} + = form.text_field 'name', :size => 0 + - hidden_fields.each do |field| + = form.hidden_field field + %td{:style => highlight_new(attrs, :note)}= form.text_field 'note', class: 'input-small' + %td{:style => highlight_new(attrs, :manufacturer)}= form.text_field 'manufacturer', class: 'input-small' + %td{:style => highlight_new(attrs, :origin)}= form.text_field 'origin', class: 'input-mini' + %td{:style => highlight_new(attrs, :unit)}= form.text_field 'unit', class: 'input-mini' + %td{:style => highlight_new(attrs, :unit_quantity)}= form.text_field 'unit_quantity', class: 'input-mini' + %td{:style => highlight_new(attrs, :price)} + .input-prepend + %span.add-on= t 'number.currency.format.unit' + = form.text_field 'price', class: 'input-mini', style: 'width: 45px' + %td{:style => highlight_new(attrs, :tax)} + .input-append + = form.text_field 'tax', class: 'input-mini', style: 'width: 45px' + %span.add-on % + %td{:style => highlight_new(attrs, :deposit)} + .input-prepend + %span.add-on= t 'number.currency.format.unit' + = form.text_field 'deposit', class: 'input-mini', style: 'width: 45px' + %td= form.select :article_category_id, ArticleCategory.all.map {|a| [ a.name, a.id ] }, + {include_blank: true}, class: 'input-small' diff --git a/app/views/articles/index.html.haml b/app/views/articles/index.html.haml index c3a00f1d..8e9bef3c 100644 --- a/app/views/articles/index.html.haml +++ b/app/views/articles/index.html.haml @@ -18,7 +18,8 @@ - if @supplier.shared_supplier .btn-group - = link_to t('.ext_db.import'), "#import", 'data-toggle-this' => '#import', class: 'btn btn-primary' + - if @supplier.shared_sync_method == 'import' + = link_to t('.ext_db.import'), "#import", 'data-toggle-this' => '#import', class: 'btn btn-primary' = link_to t('.ext_db.sync'), sync_supplier_articles_path(@supplier), method: :post, class: 'btn btn-success' .btn-group diff --git a/app/views/articles/sync.html.haml b/app/views/articles/sync.html.haml index a65878f1..65b4d97c 100644 --- a/app/views/articles/sync.html.haml +++ b/app/views/articles/sync.html.haml @@ -1,72 +1,39 @@ - title t('.title') = form_tag update_synchronized_supplier_articles_path(@supplier) do - %h2= t '.outlist.title' - %p - - unless @outlisted_articles.empty? - = t('.outlist.body').html_safe - %ul - - for article in @outlisted_articles - %li - = hidden_field_tag "outlisted_articles[#{article.id}]", '1' - = article.name - - if article.in_open_order - .alert= t '.outlist.alert_used', article: article.name - - else - %i= t '.outlist.body_skip' + - if @outlisted_articles.any? + %h2= t '.outlist.title' + %p + = t('.outlist.body').html_safe + %ul + - for article in @outlisted_articles + %li + = hidden_field_tag "outlisted_articles[#{article.id}]", '1' + = article.name + - if article.in_open_order + .alert= t '.outlist.alert_used', article: article.name + %hr/ + + - if @updated_articles.any? + %h2= t '.update.title' + %p + %i + = t '.update.update_msg', count: @updated_articles.size + = t '.update.body' + = render 'sync_table', articles: @updated_articles, field: 'articles', hidden_fields: %w(shared_updated_on) + %hr/ + + - if @new_articles.any? + %h2= t '.upnew.title' + %p + %i= t '.upnew.body_count', count: @new_articles.length + = render 'sync_table', articles: @new_articles, field: 'new_articles', hidden_fields: %w(shared_updated_on order_number) + %hr/ + - if @ignored_article_count > 0 - %i= t '.outlist.body_ignored', count: @ignored_article_count - %hr/ - %h2= t '.update.title' - %p - %i - = t '.update.update_msg', count: @updated_articles.size - = t '.update.body' - %table.table - %thead - %tr - %th= heading_helper Article, :name - %th= heading_helper Article, :note - %th= heading_helper Article, :manufacturer - %th= heading_helper Article, :origin - %th= heading_helper Article, :unit - %th= heading_helper Article, :unit_quantity, short: true - %th= heading_helper Article, :price - %th= heading_helper Article, :tax - %th= heading_helper Article, :deposit - %th= heading_helper Article, :article_category - %tbody - - @updated_articles.each do |updated_article, attrs| - - article = Article.find(updated_article.id) - %tr{:style => 'color:grey'} - %td= article.name - %td= article.note - %td= article.manufacturer - %td= article.origin - %td= article.unit - %td= article.unit_quantity - %td= article.price - %td= article.tax - %td= article.deposit - %td= article.article_category.name if article.article_category - %tr - = fields_for 'articles[]', updated_article do |form| - %td{:style => highlight_new(attrs, :name)} - = form.text_field 'name', :size => 0 - = form.hidden_field 'shared_updated_on' - %td{:style => highlight_new(attrs, :note)}= form.text_field 'note', class: 'input-small' - %td{:style => highlight_new(attrs, :manufacturer)}= form.text_field 'manufacturer', class: 'input-small' - %td{:style => highlight_new(attrs, :origin)}= form.text_field 'origin', class: 'input-mini' - %td{:style => highlight_new(attrs, :unit)}= form.text_field 'unit', class: 'input-mini' - %td{:style => highlight_new(attrs, :unit_quantity)}= form.text_field 'unit_quantity', class: 'input-mini' - %td{:style => highlight_new(attrs, :price)}= form.text_field 'price', class: 'input-mini' - %td{:style => highlight_new(attrs, :tax), class: 'input-append'} - = form.text_field 'tax', class: 'input-mini' - %span.add-on % - %td{:style => highlight_new(attrs, :deposit)}= form.text_field 'deposit', class: 'input-mini' - %td= form.select :article_category_id, ArticleCategory.all.map {|a| [ a.name, a.id ] }, - {include_blank: true}, class: 'input-small' - %hr/ + %p + %i= t '.outlist.body_ignored', count: @ignored_article_count + = hidden_field 'supplier', 'id' = submit_tag t('.submit'), class: 'btn btn-primary' = link_to t('ui.or_cancel'), supplier_articles_path(@supplier) diff --git a/app/views/suppliers/_form.html.haml b/app/views/suppliers/_form.html.haml index 93de2a97..0d612c43 100644 --- a/app/views/suppliers/_form.html.haml +++ b/app/views/suppliers/_form.html.haml @@ -16,6 +16,8 @@ = f.input :order_howto, as: :text, input_html: {rows: 5} = f.input :note, as: :text, input_html: {rows: 5} = f.input :min_order_quantity + - if @supplier.shared_supplier + = f.input :shared_sync_method, collection: shared_sync_method_collection(@supplier.shared_supplier), input_html: {class: 'input-xlarge'}, include_blank: false, disabled: @supplier.shared_supplier.shared_sync_methods.count < 2 .form-actions = f.submit class: 'btn' = link_to t('ui.or_cancel'), suppliers_path diff --git a/app/views/suppliers/show.html.haml b/app/views/suppliers/show.html.haml index 829509e9..ce7608d9 100644 --- a/app/views/suppliers/show.html.haml +++ b/app/views/suppliers/show.html.haml @@ -10,6 +10,7 @@ - if shared_supplier = @supplier.shared_supplier .alert.alert-info = t 'suppliers.shared_supplier_note' + = t "suppliers.shared_supplier_methods.#{@supplier.shared_sync_method}" %dl.dl-horizontal %dt= heading_helper(Supplier, :address) + ':' diff --git a/config/locales/en.yml b/config/locales/en.yml index 7a73cbeb..b4059672 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -135,6 +135,7 @@ en: order_howto: How to order phone: Phone phone2: Phone 2 + shared_sync_method: How to synchronize url: Homepage task: description: Description @@ -172,6 +173,15 @@ en: errors: has_many_left: is still associated with a %{collection}! models: + article: + attributes: + name: + taken: name is already taken + taken_with_unit: name and unit are already taken + supplier: + attributes: + shared_sync_method: + included: is not a valid option for this supplier task: attributes: done: @@ -396,6 +406,11 @@ en: update_msg: one: One article needs to be updated. other: '%{count} articles need to be updated.' + upnew: + body_count: + one: There is one new article to add. + other: 'There are %{count} articles to add.' + title: Add new ... upload: body:

The file has to be a text file with the ending '.csv' The first line will be ignored when imported

The fields have to be separated with semicolons (';') and the text enclosed by double quotation marks ("text...").

As character set UTF-8 is demanded. Correct order of the column:

fields: @@ -1437,6 +1452,10 @@ en: new: title: New supplier shared_supplier_note: Supplier is connected to the external database. + shared_supplier_methods: + import: Articles are imported manually. + all_available: New articles are available by default. + all_unavailable: New articles are not available by default. shared_suppliers: body:

Suppliers of the external database are displayed here.

You can import external suppliers by subscribing (see below).

A new supplier will be created and connected to the external database.

subscribe: Subscribe diff --git a/db/migrate/20140521142651_add_sync_method_to_supplier.rb b/db/migrate/20140521142651_add_sync_method_to_supplier.rb new file mode 100644 index 00000000..1c60dd96 --- /dev/null +++ b/db/migrate/20140521142651_add_sync_method_to_supplier.rb @@ -0,0 +1,5 @@ +class AddSyncMethodToSupplier < ActiveRecord::Migration + def change + add_column :suppliers, :shared_sync_method, :string + end +end diff --git a/db/schema.rb b/db/schema.rb index 94cc7f65..5b3cd143 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20140318173000) do +ActiveRecord::Schema.define(version: 20140521142651) do create_table "article_categories", force: true do |t| t.string "name", default: "", null: false @@ -317,6 +317,7 @@ ActiveRecord::Schema.define(version: 20140318173000) do t.integer "shared_supplier_id" t.string "min_order_quantity" t.datetime "deleted_at" + t.string "shared_sync_method" end add_index "suppliers", ["name"], name: "index_suppliers_on_name", unique: true, using: :btree diff --git a/spec/factories/article.rb b/spec/factories/article.rb index d79535fc..6955863a 100644 --- a/spec/factories/article.rb +++ b/spec/factories/article.rb @@ -9,7 +9,7 @@ FactoryGirl.define do tax { [6, 21].sample } deposit { rand(10) < 8 ? 0 : [0.0, 0.80, 1.20, 12.00].sample } unit_quantity { rand(5) < 3 ? 1 : rand(1..20) } - #supplier_id + supplier { create :supplier } article_category { create :article_category } end diff --git a/spec/models/article_spec.rb b/spec/models/article_spec.rb index 48d593bc..90be62a5 100644 --- a/spec/models/article_spec.rb +++ b/spec/models/article_spec.rb @@ -108,7 +108,7 @@ describe Article do it 'does not synchronise when it has no order number' do article.update_attributes :order_number => nil - expect(supplier.sync_all).to eq [[], []] + expect(supplier.sync_all).to eq [[], [], []] end end end