On #42:

- Adapted order_txt to generalized creating the text table and added
  spec
- Code style fixes for order_fax

app/documents/order_fax.rb

@@ -75,28 +75,26 @@ def recipient_paragraph
 
   def articles_paragraph
     total = 0
-    data = [I18n.t('documents.order_fax.rows').clone]
-    any_order_number_present = false
+    any_order_number_present = order_articles.where.not(article_version: { order_number: nil }).any?
+    data = [get_header_labels(!any_order_number_present)]
     each_order_article do |oa|
       price = oa.article_version.price
       subtotal = oa.units_to_order * price
       total += subtotal
-      order_number = oa.article_version.order_number
-      any_order_number_present = true if order_number
-      data << [order_number,
-               format_units_to_order(oa),
-               format_supplier_order_unit_with_ratios(oa.price),
-               oa.article_version.name,
-               number_to_currency(price),
-               number_to_currency(subtotal)]
+      oa_data = []
+      oa_data += [oa.article_version.order_number] if any_order_number_present
+      oa_data += [
+        format_units_to_order(oa),
+        format_supplier_order_unit_with_ratios(oa.price),
+        oa.article_version.name,
+        number_to_currency(price),
+        number_to_currency(subtotal)
+      ]
+      data << oa_data
     end
 
-    total_row = [I18n.t('documents.order_fax.total'), nil, nil, nil, nil, number_to_currency(total)]
-
-    unless any_order_number_present
-      data.map { |row| row.delete_at(0) }
-      total_row.delete_at(1)
-    end
+    total_row_spacing_columns = [nil] * (any_order_number_present ? 4 : 3)
+    total_row = [I18n.t('documents.order_fax.total')] + total_row_spacing_columns + [number_to_currency(total)]
 
     data << total_row
 
@@ -130,4 +128,10 @@ def order_articles
   def each_order_article(&block)
     order_articles.find_each_with_order(batch_size: BATCH_SIZE, &block)
   end
+
+  def get_header_labels(exclude_order_number)
+    labels = I18n.t('documents.order_fax.rows').clone
+    labels.delete(0) if exclude_order_number
+    labels
+  end
 end

app/helpers/orders_helper.rb

@@ -23,11 +23,6 @@ def options_for_suppliers_to_select
     options_for_select(options)
   end
 
-  def format_amount(amount, order_article, strip_insignificant_zeros: false)
-    strip_insignificant_zeros = true unless order_article.article_version.supplier_order_unit_is_si_convertible
-    number_with_precision(amount, precision: 3, strip_insignificant_zeros: strip_insignificant_zeros)
-  end
-
   def format_units_to_order(order_article, strip_insignificant_zeros: false)
     format_amount(order_article.units_to_order, order_article, strip_insignificant_zeros: strip_insignificant_zeros)
   end
@@ -214,4 +209,11 @@ def receive_button(order, options = {})
               class: "btn#{' btn-success' unless order.received?} #{options[:class]}"
     end
   end
+
+  private
+
+  def format_amount(amount, order_article, strip_insignificant_zeros: false)
+    strip_insignificant_zeros = true unless order_article.article_version.supplier_order_unit_is_si_convertible
+    number_with_precision(amount, precision: 3, strip_insignificant_zeros: strip_insignificant_zeros)
+  end
 end

app/lib/order_txt.rb

@@ -21,16 +21,20 @@ def to_txt
     text += '****** ' + I18n.t('orders.fax.articles') + "\n\n"
 
     # prepare order_articles data
-    longest_number_string_length = 0
-    longest_amount_string_length = I18n.t('orders.fax.amount').length
-    longest_unit_string_length = I18n.t('orders.fax.unit').length
-    @order_positions = @order.order_articles.ordered.includes(:article_version).map do |oa|
+    order_articles = @order.order_articles.ordered.includes(:article_version).order('article_versions.order_number ASC, article_versions.name ASC')
+    any_number_present = order_articles.where.not(article_version: { order_number: nil }).any?
+
+    order_headers = {
+      number: any_number_present ? { label: I18n.t('orders.fax.number') } : nil,
+      amount: { label: I18n.t('orders.fax.amount'), align: :right },
+      unit: { label: I18n.t('orders.fax.unit') },
+      name: { label: I18n.t('orders.fax.name') }
+    }.compact
+
+    order_positions = order_articles.map do |oa|
       number = oa.article_version.order_number || ''
       amount = format_units_to_order(oa).to_s
       unit = format_supplier_order_unit_with_ratios(oa.price)
-      longest_number_string_length = number.length if number.length > longest_number_string_length
-      longest_amount_string_length = amount.length if amount.length > longest_amount_string_length
-      longest_unit_string_length = unit.length if unit.length > longest_unit_string_length
       {
         number: number,
         amount: amount,
@@ -39,20 +43,37 @@ def to_txt
       }
     end
 
-    if (any_number_present = longest_number_string_length > 0) && longest_number_string_length < I18n.t('orders.fax.number').length
-      longest_number_string_length = I18n.t('orders.fax.number').length
+    text += text_table(order_headers, order_positions)
+    text
+  end
+
+  private
+
+  def text_table(headers, rows)
+    table_keys = headers.keys
+    columns = table_keys.each_with_index.map do |key, index|
+      header = headers[key]
+      label = header[:label]
+      {
+        key: key,
+        label: label,
+        align: header[:align],
+        characters: index + 1 < table_keys.length ? (rows.pluck(key) + [label]).map(&:length).max : nil
+      }
     end
 
-    # header for order articles table
-    text += format('%s  ', I18n.t('orders.fax.number').ljust(longest_number_string_length)) if any_number_present
-    text += format("%s %s  %s\n", I18n.t('orders.fax.amount').rjust(longest_amount_string_length),
-                   I18n.t('orders.fax.unit').ljust(longest_unit_string_length), I18n.t('orders.fax.name'))
+    header_txt = columns.map { |column| align_text_column(column[:label], column[:characters], column[:align]) }.join(' ')
 
-    # now display all ordered articles
-    @order_positions.each do |op|
-      text += format('%s  ', op[:number].ljust(longest_number_string_length)) if any_number_present
-      text += format("%s %s  %s\n", op[:amount].rjust(longest_amount_string_length), op[:unit].ljust(longest_unit_string_length), op[:name])
+    rows_texts = rows.map do |row|
+      columns.map { |column| align_text_column(row[column[:key]], column[:characters], column[:align]) }.join(' ')
     end
-    text
+
+    ([header_txt] + rows_texts).join("\n")
+  end
+
+  def align_text_column(text, characters, align)
+    return text if characters.nil?
+
+    align == :right ? text.rjust(characters) : text.ljust(characters)
   end
 end

spec/lib/order_txt_csv_spec.rb

@@ -0,0 +1,49 @@
+require_relative '../spec_helper'
+
+describe OrderTxt do
+  let(:user) { create(:user, groups: [create(:ordergroup)]) }
+  let(:order) { create(:order, created_by: user, starts: Date.yesterday, ends: 1.hour.ago, end_action: :auto_close, article_count: 3) }
+  let(:supplier) { order.supplier }
+  let(:go) { create(:group_order, order: order, ordergroup: user.ordergroup) }
+  let(:articles) { order.articles }
+  let(:order_articles) { order.order_articles }
+  let(:first_article_version) { articles.first.latest_article_version }
+  let(:second_article_version) { articles.second.latest_article_version }
+  let(:third_article_version) { articles.third.latest_article_version }
+
+  it 'creates a proper csv table sorted by order_number from an order' do
+    first_article_version.update(name: 'Short name', supplier_order_unit: 'XPK')
+    second_article_version.update(name: 'Much longer complicated name', supplier_order_unit: 'KGM')
+    third_article_version.update(name: 'Quite short name', supplier_order_unit: 'GRM')
+    order_articles.where(article_version: first_article_version).update(units_to_order: 1)
+    order_articles.where(article_version: second_article_version).update(units_to_order: 1.421)
+    order_articles.where(article_version: third_article_version).update(units_to_order: 4.432643311)
+
+    result = described_class.new(order).to_txt
+    expected_table = %(
+Number Amount Unit    Name
+0           1 Package Short name
+1       1.421 kg      Much longer complicated name
+2       4.433 g       Quite short name
+    )
+    expect(result.strip).to end_with(expected_table.strip)
+  end
+
+  it 'omits the order_number column and sort alphabetically if none of the ordered articles have an order_number' do
+    first_article_version.update(name: 'Short name', supplier_order_unit: 'XPK', order_number: nil)
+    second_article_version.update(name: 'Much longer complicated name', supplier_order_unit: 'KGM', order_number: nil)
+    third_article_version.update(name: 'Quite short name', supplier_order_unit: 'GRM', order_number: nil)
+    order_articles.where(article_version: first_article_version).update(units_to_order: 1)
+    order_articles.where(article_version: second_article_version).update(units_to_order: 1.421)
+    order_articles.where(article_version: third_article_version).update(units_to_order: 4.432643311)
+
+    result = described_class.new(order).to_txt
+    expected_table = %(
+Amount Unit    Name
+ 1.421 kg      Much longer complicated name
+ 4.433 g       Quite short name
+     1 Package Short name
+    )
+    expect(result.strip).to end_with(expected_table.strip)
+  end
+end