[Spree Upgrade] Merging master into 2-0-stable (3rd run in Feb2019)

.github/ISSUE_TEMPLATE/tech-debt-template.md

@@ -0,0 +1,21 @@
+---
+name: Tech debt template
+about: File a refactoring task that aids future development
+title: ''
+labels: tech debt
+assignees: ''
+
+---
+
+### What we should change and why (this is tech debt)
+<!-- The use of X changed and part y is now redundant. We can remove it and... -->
+
+
+
+### Context
+<!-- This came up in PR #3451#issuecomment-461616636. -->
+
+
+
+### Impact and timeline
+<!-- Example: This simplifies the upgrade to Spree 3 and should be done after upgrading to Spree 2. -->

Gemfile

@@ -5,7 +5,7 @@ git_source(:github) { |repo_name| "https://github.com/#{repo_name}.git" }
 gem 'rails', '~> 3.2.22'
 gem 'rails-i18n', '~> 3.0.0'
 gem 'i18n', '~> 0.6.11'
-gem 'i18n-js', '~> 3.1.0'
+gem 'i18n-js', '~> 3.2.1'
 
 # Patched version. See http://rubysec.com/advisories/CVE-2015-5312/.
 gem 'nokogiri', '>= 1.6.7.1'

Gemfile.lock

@@ -195,7 +195,7 @@ GEM
     bcrypt-ruby (3.1.5)
       bcrypt (>= 3.1.3)
     blockenspiel (0.5.0)
-    bugsnag (4.1.0)
+    bugsnag (5.5.0)
     builder (3.0.4)
     byebug (9.0.6)
     cancan (1.6.10)
@@ -251,7 +251,7 @@ GEM
     css_parser (1.6.0)
       addressable
     daemons (1.3.1)
-    dalli (2.7.2)
+    dalli (2.7.9)
     database_cleaner (0.7.1)
     db2fog (0.9.0)
       activerecord (>= 3.2.0, < 5.0)
@@ -293,7 +293,7 @@ GEM
     faraday (0.15.4)
       multipart-post (>= 1.2, < 3)
     ffaker (1.22.1)
-    ffi (1.9.25)
+    ffi (1.10.0)
     figaro (1.1.1)
       thor (~> 0.14)
     fission (0.5.0)
@@ -489,8 +489,8 @@ GEM
     httparty (0.16.2)
       multi_xml (>= 0.5.2)
     i18n (0.6.11)
-    i18n-js (3.1.0)
-      i18n (>= 0.6.6, < 2)
+    i18n-js (3.2.1)
+      i18n (>= 0.6.6)
     immigrant (0.3.6)
       activerecord (>= 3.0)
     io-like (0.3.0)
@@ -660,7 +660,7 @@ GEM
     rspec-mocks (3.8.0)
       diff-lcs (>= 1.2.0, < 2.0)
       rspec-support (~> 3.8.0)
-    rspec-rails (3.8.1)
+    rspec-rails (3.8.2)
       actionpack (>= 3.0)
       activesupport (>= 3.0)
       railties (>= 3.0)
@@ -692,9 +692,9 @@ GEM
     select2-rails (3.4.9)
       sass-rails
       thor (~> 0.14)
-    selenium-webdriver (3.12.0)
+    selenium-webdriver (3.141.0)
       childprocess (~> 0.5)
-      rubyzip (~> 1.2)
+      rubyzip (~> 1.2, >= 1.2.2)
     shellany (0.0.1)
     shoulda-matchers (2.8.0)
       activesupport (>= 3.0.0)
@@ -705,7 +705,7 @@ GEM
     simplecov-html (0.10.2)
     skylight (1.7.2)
       activesupport (>= 3.0.0)
-    spinjs-rails (1.3)
+    spinjs-rails (1.4)
       rails (>= 3.1)
     spreadsheet (1.1.7)
       ruby-ole (>= 1.0)
@@ -812,7 +812,7 @@ DEPENDENCIES
   guard-rspec (~> 4.7.3)
   haml
   i18n (~> 0.6.11)
-  i18n-js (~> 3.1.0)
+  i18n-js (~> 3.2.1)
   immigrant
   jquery-migrate-rails
   jquery-rails (= 3.0.0)

app/assets/javascripts/admin/subscriptions/subscriptions.js.coffee

@@ -1 +1 @@
-angular.module("admin.subscriptions", ['ngResource','admin.indexUtils','admin.dropdown'])
+angular.module("admin.subscriptions", ['ngResource','admin.indexUtils','admin.dropdown', 'admin.utils'])

app/assets/javascripts/admin/utils/filters/localize_currency.js.coffee

@@ -0,0 +1,14 @@
+angular.module("admin.utils").filter "localizeCurrency", (currencyConfig)->
+  # Convert number to string currency using injected currency configuration.
+  (amount) ->
+    # Set country code (eg. "US").
+    currency_code = if currencyConfig.display_currency then " " + currencyConfig.currency else ""
+    # Set decimal points, 2 or 0 if hide_cents.
+    decimals = if currencyConfig.hide_cents == "true" then 0 else 2
+    # Set format if the currency symbol should come after the number, otherwise (default) use the locale setting.
+    format = if currencyConfig.symbol_position == "after" then "%n %u" else undefined
+    # We need to use parseFloat as the amount should come in as a string.
+    amount = parseFloat(amount)
+
+    # Build the final price string.
+    I18n.toCurrency(amount, {precision: decimals, unit: currencyConfig.symbol, format: format}) + currency_code

app/controllers/admin/product_import_controller.rb

@@ -84,7 +84,8 @@ def save_uploaded_file(upload)
       directory = 'tmp/product_import'
       Dir.mkdir(directory) unless File.exist?(directory)
       File.open(Rails.root.join(directory, filename + extension), 'wb') do |f|
-        f.write(upload.read)
+        data = UploadSanitizer.new(upload.read).call
+        f.write(data)
         f.path
       end
     end

app/controllers/spree/admin/orders_controller_decorator.rb

@@ -54,9 +54,7 @@ def resend
   end
 
   def invoice
-    pdf = render_to_string pdf: "invoice-#{@order.number}.pdf",
-                           template: invoice_template,
-                           formats: [:html], encoding: "UTF-8"
+    pdf = InvoiceRenderer.new.render_to_string(@order)
 
     Spree::OrderMailer.invoice_email(@order.id, pdf).deliver
     flash[:success] = t('admin.orders.invoice_email_sent')
@@ -65,7 +63,7 @@ def invoice
   end
 
   def print
-    render pdf: "invoice-#{@order.number}", template: invoice_template, encoding: "UTF-8"
+    render InvoiceRenderer.new.args(@order)
   end
 
   def print_ticket
@@ -78,10 +76,6 @@ def update_distribution_charge
 
   private
 
-  def invoice_template
-    Spree::Config.invoice_style2? ? "spree/admin/orders/invoice2" : "spree/admin/orders/invoice"
-  end
-
   def require_distributor_abn
     unless @order.distributor.abn.present?
       flash[:error] = t(:must_have_valid_business_number, enterprise_name: @order.distributor.name)

app/helpers/admin/injection_helper.rb

@@ -65,6 +65,10 @@ def admin_inject_column_preferences(opts={})
       admin_inject_json_ams_array opts[:module], "columns", column_preferences, Api::Admin::ColumnPreferenceSerializer
     end
 
+    def admin_inject_currency_config
+      admin_inject_json_ams 'admin.utils', "currencyConfig", {}, Api::CurrencyConfigSerializer
+    end
+
     def admin_inject_enterprise_permissions
       permissions =
         {can_manage_shipping_methods: can?(:manage_shipping_methods, @enterprise),

app/models/product_import/entry_processor.rb

@@ -175,7 +175,7 @@ def save_new_product(entry)
         assign_errors product.errors.full_messages, entry.line_number
       end
 
-      @already_created[entry.enterprise_id] = { entry.name => product.id }
+      @already_created.deep_merge! entry.enterprise_id => { entry.name => product.id }
     end
 
     def save_variant(entry)
@@ -212,12 +212,20 @@ def assign_defaults(object, entry)
         case setting['mode']
         when 'overwrite_all'
           object.assign_attributes(attribute => setting['value'])
+          # In case of new products, some attributes are saved on the variant.
+          # We write them to the entry here to be copied to the variant later.
+          if entry.respond_to? "#{attribute}="
+            entry.public_send("#{attribute}=", setting['value'])
+          end
         when 'overwrite_empty'
           if object.public_send(attribute).blank? ||
              ((attribute == 'on_hand' || attribute == 'count_on_hand') &&
              entry.on_hand_nil)
 
             object.assign_attributes(attribute => setting['value'])
+            if entry.respond_to? "#{attribute}="
+              entry.public_send("#{attribute}=", setting['value'])
+            end
           end
         end
       end

app/overrides/spree/layouts/admin/add_currency_config.html.haml.deface

@@ -0,0 +1,3 @@
+/ insert_before "div#wrapper"
+
+= admin_inject_currency_config

app/services/bulk_invoice_service.rb

@@ -1,5 +1,4 @@
 class BulkInvoiceService
-  include WickedPdf::PdfHelper
   attr_reader :id
 
   def initialize
@@ -11,10 +10,7 @@ def start_pdf_job(order_ids)
     orders = Spree::Order.where(id: order_ids)
 
     orders.each do |order|
-      invoice = renderer.render_to_string pdf: "invoice-#{order.number}.pdf",
-                                          template: invoice_template,
-                                          formats: [:html], encoding: "UTF-8",
-                                          locals: { :@order => order }
+      invoice = renderer.render_to_string(order)
 
       pdf << CombinePDF.parse(invoice)
     end
@@ -42,11 +38,7 @@ def directory
   end
 
   def renderer
-    ApplicationController.new
-  end
-
-  def invoice_template
-    Spree::Config.invoice_style2? ? "spree/admin/orders/invoice2" : "spree/admin/orders/invoice"
+    @renderer ||= InvoiceRenderer.new
   end
 
   def file_directory

app/services/invoice_renderer.rb

@@ -0,0 +1,29 @@
+class InvoiceRenderer
+  def render_to_string(order)
+    renderer.render_to_string(args(order))
+  end
+
+  def args(order)
+    {
+      pdf: "invoice-#{order.number}.pdf",
+      template: invoice_template,
+      formats: [:html],
+      encoding: "UTF-8",
+      locals: { :@order => order }
+    }
+  end
+
+  private
+
+  def renderer
+    ApplicationController.new
+  end
+
+  def invoice_template
+    if Spree::Config.invoice_style2?
+      "spree/admin/orders/invoice2"
+    else
+      "spree/admin/orders/invoice"
+    end
+  end
+end

app/services/upload_sanitizer.rb

@@ -0,0 +1,18 @@
+# Formats uploaded files to UTF-8 encoding and strips unexpected BOM characters.
+# Takes an open File object as input
+class UploadSanitizer
+  def initialize(upload)
+    @data = upload
+  end
+
+  def call
+    @data.force_encoding('UTF-8')
+    strip_bom_character
+  end
+
+  private
+
+  def strip_bom_character
+    @data.gsub("\xEF\xBB\xBF".force_encoding("UTF-8"), '')
+  end
+end

app/views/admin/subscriptions/_orders_panel.html.haml

@@ -19,7 +19,7 @@
                 %div{ ng: { bind: "::orderCycleCloses(proxyOrder.order_cycle_id)" } }
               %td.text-center
                 %span.state{ ng: { class: "proxyOrder.state", bind: 'stateText(proxyOrder.state)' } }
-              %td.text-center{ ng: { bind: '(proxyOrder.total || subscription.estimatedTotal()) | currency' } }
+              %td.text-center{ ng: { bind: '(proxyOrder.total || subscription.estimatedTotal()) | localizeCurrency' } }
               %td.actions
                 %a.edit-order.icon-edit.no-text{ href: '{{::proxyOrder.edit_path}}', target: '_blank', 'ofn-with-tip' => t(:edit_order), confirm_order_edit: true }
                 %a.cancel-order.icon-remove.no-text{ href: 'javascript:void(0)', ng: { hide: "proxyOrder.state == 'canceled'", click: "cancelOrder(proxyOrder)" }, 'ofn-with-tip' => t(:cancel_order) }

app/views/admin/subscriptions/_review.html.haml

@@ -75,24 +75,24 @@
                   .description {{ item.description }}
                   .not-in-open-and-upcoming-order-cycles-warning{ ng: { if: '!item.in_open_and_upcoming_order_cycles' } }
                     = t(".no_open_or_upcoming_order_cycle")
-                %td.price.align-center {{ item.price_estimate | currency }}
+                %td.price.align-center {{ item.price_estimate | localizeCurrency }}
                 %td.quantity {{ item.quantity }}
-                %td.total.align-center {{ (item.price_estimate * item.quantity) | currency }}
+                %td.total.align-center {{ (item.price_estimate * item.quantity) | localizeCurrency }}
             %tbody#subtotal.no-border-top{"data-hook" => "admin_order_form_subtotal"}
               %tr#subtotal-row
                 %td{:colspan => "3"}
                   %b
                     = t(:subtotal)
                     \:
                 %td.total.align-center
-                  %span {{ subscription.estimatedSubtotal() | currency }}
+                  %span {{ subscription.estimatedSubtotal() | localizeCurrency }}
             %tbody#order-total.grand-total.no-border-top{"data-hook" => "admin_order_form_total"}
               %tr
                 %td{:colspan => "3"}
                   %b
                     = t(:order_total_price)
                     \:
                 %td.total.align-center
-                  %span#order_form_total {{ subscription.estimatedTotal() | currency }}
+                  %span#order_form_total {{ subscription.estimatedTotal() | localizeCurrency }}
           %p.notice
             = t "this_is_an_estimate", scope: 'admin.subscriptions.subscription_line_items'

app/views/admin/subscriptions/_subscription_line_items.html.haml

@@ -19,10 +19,10 @@
         .description {{ item.description }}
         .not-in-open-and-upcoming-order-cycles-warning{ ng: { if: '!item.in_open_and_upcoming_order_cycles' } }
           = t(".not_in_open_and_upcoming_order_cycles_warning")
-      %td.price.align-center {{ item.price_estimate | currency }}
+      %td.price.align-center {{ item.price_estimate | localizeCurrency }}
       %td.quantity
         %input{ name: 'quantity', type: 'number', min: 0, ng: { model: 'item.quantity' } }
-      %td.total.align-center {{ (item.price_estimate * item.quantity) | currency }}
+      %td.total.align-center {{ (item.price_estimate * item.quantity) | localizeCurrency }}
       %td.actions
         %a.delete-item.icon-trash.no-text{ ng: { click: 'removeSubscriptionLineItem(item)'}, :href => "javascript:void(0)" }
   %tbody#subtotal.no-border-top{"data-hook" => "admin_order_form_subtotal"}
@@ -32,7 +32,7 @@
           = t(:subtotal)
           \:
       %td.total.align-center
-        %span#order_subtotal {{ subscription.estimatedSubtotal() | currency }}
+        %span#order_subtotal {{ subscription.estimatedSubtotal() | localizeCurrency }}
       %td.actions
   %tbody#fees.no-border-top{ ng: { show: "subscription.estimatedFees() > 0" } }
     %tr#fees-row
@@ -41,7 +41,7 @@
           = t(:fees)
           \:
       %td.total.align-center
-        %span#order_fees {{ subscription.estimatedFees() | currency }}
+        %span#order_fees {{ subscription.estimatedFees() | localizeCurrency }}
       %td.actions
   %tbody#order-total.grand-total.no-border-top{"data-hook" => "admin_order_form_total"}
     %tr
@@ -50,7 +50,7 @@
           = t(:order_total_price)
           \:
       %td.total.align-center
-        %span#order_form_total {{ subscription.estimatedTotal() | currency }}
+        %span#order_form_total {{ subscription.estimatedTotal() | localizeCurrency }}
       %td.actions
 %p.notice
   = t ".this_is_an_estimate"

app/views/spree/admin/orders/_invoice_table.html.haml

@@ -14,6 +14,9 @@
       %tr
         %td
           = render 'spree/shared/line_item_name', line_item: item
+          %br
+          %small
+            %em= raw(item.variant.product.supplier.name)
         %td{:align => "right"}
           = item.quantity
         %td{:align => "right"}

app/views/spree/admin/orders/_invoice_table2.html.haml

@@ -17,6 +17,9 @@
       %tr
         %td
           = render 'spree/shared/line_item_name', line_item: item
+          %br
+          %small
+            %em= raw(item.variant.product.supplier.name)
         %td{:align => "right"}
           = item.quantity
         %td{:align => "right"}