On #8: Started with unit tests for supplier article sharing

.rubocop_todo.yml

@@ -413,6 +413,7 @@ RSpec/DescribedClass:
 RSpec/EmptyExampleGroup:
   Exclude:
     - 'spec/requests/api/v1/article_categories_controller_spec.rb'
+    - 'spec/requests/api/v1/shared_suppliers/articles_controller_spec.rb'
     - 'spec/requests/api/v1/configs_controller_spec.rb'
     - 'spec/requests/api/v1/financial_transaction_classes_controller_spec.rb'
     - 'spec/requests/api/v1/financial_transaction_types_controller_spec.rb'

app/assets/javascripts/article-form.js

@@ -243,11 +243,7 @@ class ArticleForm {
       .text(chosenOptionLabel !== undefined ? chosenOptionLabel : unitVal);
 
     const converter = this.getUnitsConverter();
-    if (converter.isUnitSiConversible(this.supplierUnitSelect$.val())) {
-      this.minimumOrderQuantity$.removeAttr('step');
-    } else {
-      this.minimumOrderQuantity$.attr('step', 1);
-    }
+    this.minimumOrderQuantity$.attr('step', converter.isUnitSiConversible(this.supplierUnitSelect$.val()) ? 'any' : 1);
   }
 
   bindAddRatioButton() {

app/controllers/api/v1/articles_controller.rb

@@ -2,62 +2,60 @@ class Api::V1::ArticlesController < Api::V1::BaseController
   skip_before_action :authenticate
 
   def index
-    supplier = Supplier.find_by_external_uuid(index_params.fetch(:shared_supplier_uuid))
-    raise ActionController::RoutingError, 'Not Found' if supplier.nil?
+    supplier = Supplier.find_by_external_uuid!(index_params.fetch(:shared_supplier_uuid))
 
     @articles = Article.with_latest_versions_and_categories.undeleted.where(supplier_id: supplier, type: nil)
-    if index_params.include?(:updated_after)
-      @articles = @articles.where('article_versions.updated_at > ?',
-                                  index_params[:updated_after].to_datetime)
-    end
-    if index_params.include?(:name)
-      @articles = @articles.where('article_versions.name LIKE ?',
-                                  "%#{index_params[:name]}%")
-    end
+    @articles = @articles.where('article_versions.updated_at > ?', index_params[:updated_after].to_datetime) if index_params.include?(:updated_after)
+    @articles = @articles.where('article_versions.name LIKE ?', "%#{index_params[:name]}%") if index_params.include?(:name)
     @articles = @articles.where(article_versions: { origin: index_params[:origin] }) if index_params.include?(:origin)
     @articles = @articles.page(index_params[:page]).per(index_params.fetch(:per_page)) if index_params.include?(:page)
 
-    data = @articles.map do |article|
-      version_attributes = article.latest_article_version.attributes
-      version_attributes.delete_if { |key| key == 'id' || key.end_with?('_id') }
+    data = @articles.map { |article| get_article_version_data(article) }
 
-      version_attributes['article_unit_ratios'] = article.latest_article_version.article_unit_ratios.map do |ratio|
-        ratio_attributes = ratio.attributes
-        ratio_attributes.delete_if { |key| key == 'id' || key.end_with?('_id') }
-      end
+    render json: { articles: data, pagination: pagination_response, latest_update: get_latest_article_update(supplier) }
+  end
+
+  protected
+
+  def index_params
+    params.permit(:shared_supplier_uuid, :updated_after, :name, :origin, :page, :per_page)
+  end
 
-      version_attributes
+  def get_article_version_data(article)
+    version_attributes = article.latest_article_version.attributes
+    version_attributes.delete_if { |key| key == 'id' || key.end_with?('_id') }
+
+    version_attributes['article_unit_ratios'] = article.latest_article_version.article_unit_ratios.map do |ratio|
+      ratio_attributes = ratio.attributes
+      ratio_attributes.delete_if { |key| key == 'id' || key.end_with?('_id') }
     end
 
+    version_attributes
+  end
+
+  def get_latest_article_update(supplier)
     latest_update = Article
                     .with_latest_versions
                     .undeleted
                     .where(supplier_id: supplier, type: nil)
                     .order('article_versions.updated_at DESC')
                     .limit(1)
                     .first&.updated_at
-    latest_update = latest_update.utc unless latest_update.nil?
-
-    pagination = nil
-    if index_params.include?(:page)
-      current = @articles.current_page
-      total = @articles.total_pages
-      per_page = @articles.limit_value
-      pagination = {
-        current_page: current,
-        previous_page: (current > 1 ? (current - 1) : nil),
-        next_page: (current == total ? nil : (current + 1)),
-        per_page: per_page,
-        total_pages: total,
-        number: @articles.total_count
-      }
-    end
-    render json: { articles: data, pagination: pagination, latest_update: latest_update }
+    latest_update&.utc
   end
 
-  protected
-
-  def index_params
-    params.permit(:shared_supplier_uuid, :updated_after, :name, :origin, :page, :per_page)
+  def pagination_response
+    return nil unless index_params.include?(:page)
+
+    current = @articles.current_page
+    total = @articles.total_pages
+    {
+      current_page: current,
+      previous_page: (current > 1 ? (current - 1) : nil),
+      next_page: (current == total ? nil : (current + 1)),
+      per_page: @articles.limit_value,
+      total_pages: total,
+      number: @articles.total_count
+    }
   end
 end

app/controllers/articles_controller.rb

@@ -2,7 +2,7 @@ class ArticlesController < ApplicationController
   before_action :authenticate_article_meta, :find_supplier
 
   before_action :load_article, only: %i[edit update]
-  before_action :load_article_units, only: %i[edit update new create sync update_synchronized]
+  before_action :load_article_units, only: %i[edit update new create]
   before_action :load_article_categories, only: %i[edit_all migrate_units update_all]
   before_action :new_empty_article_ratio,
                 only: %i[edit edit_all migrate_units update new create parse_upload sync update_synchronized]
@@ -270,11 +270,9 @@ def parse_upload
   # renders a form with articles, which should be updated
   def sync
     @updated_article_pairs, @outlisted_articles, @new_articles = @supplier.sync_from_remote
-    if @updated_article_pairs.empty? && @outlisted_articles.empty? && @new_articles.empty?
-      redirect_to supplier_articles_path(@supplier),
-                  notice: I18n.t('articles.controller.parse_upload.notice')
-    end
+    redirect_to(supplier_articles_path(@supplier), notice: I18n.t('articles.controller.parse_upload.notice')) if @updated_article_pairs.empty? && @outlisted_articles.empty? && @new_articles.empty?
     @ignored_article_count = 0
+    load_article_units((@new_articles + @updated_article_pairs.map(&:first)).map(&:current_article_units).flatten.uniq)
   rescue StandardError => e
     redirect_to upload_supplier_articles_path(@supplier), alert: I18n.t('errors.general_msg', msg: e.message)
   end
@@ -319,6 +317,7 @@ def update_synchronized
     end
 
     if has_error
+      load_article_units((@new_articles + @updated_articles).map(&:current_article_units).flatten.uniq)
       @updated_article_pairs = @updated_articles.map do |article|
         orig_article = Article.find(article.id)
         [article, orig_article.unequal_attributes(article)]

app/controllers/supplier_shares_controller.rb

@@ -1,4 +1,6 @@
 class SupplierSharesController < ApplicationController
+  before_action :authenticate_suppliers
+
   def create
     @supplier = Supplier.find(params[:supplier_id])
     @supplier.update_attribute(:external_uuid, SecureRandom.uuid)

app/models/article_version.rb

@@ -117,7 +117,12 @@ def minimum_order_quantity=(value)
     if value.blank?
       self[:minimum_order_quantity] = nil
     else
-      super
+      begin
+        value = value.to_i if Float(value) % 1 == 0
+      rescue ArgumentException
+        # not any number -> let validation handle this
+      end
+      super(value)
     end
   end
 

spec/controllers/supplier_shares_controller_spec.rb

@@ -0,0 +1,24 @@
+require_relative '../spec_helper'
+
+describe SupplierSharesController do
+  let(:user) { create(:user, groups: [create(:workgroup, role_suppliers: true)]) }
+  let(:supplier) { create(:supplier) }
+
+  before do
+    login user
+  end
+
+  it 'share supplier' do
+    post_with_defaults :create, params: { supplier_id: supplier.id }, xhr: true
+    expect(response).to have_http_status :ok
+    expect(supplier.reload.external_uuid).not_to be_nil
+  end
+
+  it 'unshare supplier' do
+    supplier.update_attribute(:external_uuid, 'something')
+
+    delete_with_defaults :destroy, params: { supplier_id: supplier.id }, xhr: true
+    expect(response).to have_http_status :ok
+    expect(supplier.reload.external_uuid).to be_nil
+  end
+end

spec/integration/articles_spec.rb

@@ -2,6 +2,7 @@
 
 feature ArticlesController do
   let(:user) { create(:user, groups: [create(:workgroup, role_article_meta: true)]) }
+  let(:remote_supplier) { create(:supplier, external_uuid: 'TestUUID', article_count: 10) }
   let(:supplier) { create(:supplier) }
   let!(:article_unit) { create(:article_unit, unit: 'XPK') }
   let!(:article_category) { create(:article_category) }
@@ -83,6 +84,34 @@
     end
   end
 
+  describe ':sync', :js do
+    before do
+      supplier.update(
+        supplier_remote_source: api_v1_shared_supplier_articles_url(remote_supplier.external_uuid,
+                                                                    foodcoop: FoodsoftConfig[:default_scope],
+                                                                    host: Capybara.current_session.server.host,
+                                                                    port: Capybara.current_session.server.port),
+        shared_sync_method: 'all_available'
+      )
+      remote_supplier.articles.each_with_index do |article, index|
+        article.latest_article_version.update(order_number: index.to_s)
+      end
+    end
+
+    it 'synchronises articles from external suppliers' do
+      visit supplier_articles_path(supplier_id: supplier.id)
+      click_on I18n.t('articles.index.ext_db.sync')
+      expect(page).to have_css('.sync-table tbody tr', count: 10)
+
+      10.times do |index|
+        select ArticleCategory.first.name, from: "new_articles_#{index}_article_category_id"
+      end
+
+      click_on I18n.t('articles.sync.submit')
+      expect(page).to have_css('.just-updated.article', count: 10)
+    end
+  end
+
   describe ':upload' do
     let(:filename) { 'foodsoft_file_02.csv' }
     let(:file)     { Rails.root.join("spec/fixtures/#{filename}") }

spec/requests/api/v1/shared_suppliers/articles_controller_spec.rb

@@ -0,0 +1,141 @@
+require 'swagger_helper'
+
+def success_schema
+  {
+    type: :object,
+    properties: {
+      articles: {
+        type: :array,
+        items: {
+          '$ref': '#/components/schemas/ExternalArticle'
+        }
+      },
+      latest_update: {
+        type: :string,
+        nullable: true,
+        format: 'date-time',
+        description: 'latest update to any of the supplier\'s articles'
+      },
+      pagination: {
+        '$ref': '#/components/schemas/Pagination'
+      }
+    }
+  }
+end
+
+describe 'shared supplier articles' do
+  include ApiHelper
+  let(:api_scopes) { ['shared_suppliers:articles'] }
+
+  path '/shared_suppliers/{uuid}/articles' do
+    get 'retrieve articles' do
+      tags 'Supplier'
+      produces 'application/json'
+      parameter name: :uuid, in: :path, type: :string, required: true, description: 'the external UUID of the supplier'
+      parameter name: :updated_after, in: :query, type: :string, format: 'date-time', required: false, description: 'only retrieve articles after this date time'
+      parameter name: :origin, in: :query, type: :string, required: false, description: 'filter by article origin'
+      parameter name: :name, in: :query, type: :string, required: false, description: 'filter by article name fragment'
+      parameter name: :page, in: :query, type: :number, required: false, description: 'pagination: number of the page to retrieve'
+      parameter name: :per_page, in: :query, type: :number, required: false, description: 'pagination: items per page'
+      let(:supplier) { create(:supplier, article_count: 10, external_uuid: 'test') }
+      let(:uuid) { supplier.external_uuid }
+
+      response '200', 'success' do
+        schema(success_schema)
+        run_test! do |response|
+          expect(JSON.parse(response.body)['articles'].length).to eq 10
+        end
+      end
+
+      context 'when filtered by updated_after' do
+        let(:supplier) { create(:supplier, article_count: 10, external_uuid: 'test') }
+        let(:uuid) { supplier.external_uuid }
+        let(:updated_after) { 1.day.ago }
+
+        before do
+          supplier.articles[0..4].each do |article|
+            article.update_attribute(:updated_at, 10.days.ago)
+          end
+          supplier.articles[5..9].each do |article|
+            article.update_attribute(:updated_at, Time.now)
+          end
+        end
+
+        response '200', 'success' do
+          schema(success_schema)
+          run_test! do |response|
+            expect(JSON.parse(response.body)['articles'].length).to eq 5
+          end
+        end
+      end
+
+      context 'when filtered by origin' do
+        let(:supplier) { create(:supplier, article_count: 10, external_uuid: 'test') }
+        let(:uuid) { supplier.external_uuid }
+        let(:origin) { 'TestOrigin' }
+
+        before do
+          supplier.articles[0..4].each do |article|
+            article.update_attribute(:origin, 'TestOrigin')
+          end
+        end
+
+        response '200', 'success' do
+          schema(success_schema)
+          run_test! do |response|
+            expect(JSON.parse(response.body)['articles'].length).to eq 5
+          end
+        end
+      end
+
+      context 'when filtered by name' do
+        let(:supplier) { create(:supplier, article_count: 10, external_uuid: 'test') }
+        let(:uuid) { supplier.external_uuid }
+        let(:name) { 'TestName' }
+
+        before do
+          supplier.articles[0..4].each_with_index do |article, index|
+            article.update_attribute(:name, "TestName#{index}")
+          end
+        end
+
+        response '200', 'success' do
+          schema(success_schema)
+          run_test! do |response|
+            expect(JSON.parse(response.body)['articles'].length).to eq 5
+          end
+        end
+      end
+
+      context 'when paginating' do
+        let(:supplier) { create(:supplier, article_count: 10, external_uuid: 'test') }
+        let(:uuid) { supplier.external_uuid }
+        let(:page) { 2 }
+        let(:per_page) { 5 }
+
+        response '200', 'success' do
+          schema(success_schema)
+          run_test! do |response|
+            parsed_response = JSON.parse(response.body)
+            expect(parsed_response['articles']&.length).to eq 5
+            pagination_response = parsed_response['pagination']
+            expect(pagination_response&.dig('current_page')).to eq 2
+            expect(pagination_response&.dig('total_pages')).to eq 2
+            expect(pagination_response&.dig('previous_page')).to eq 1
+            expect(pagination_response&.dig('next_page')).to be_nil
+          end
+        end
+      end
+
+      context 'with invalid supplier uuid' do
+        let(:uuid) { 'invalid' }
+
+        response '404', 'not found' do
+          schema '$ref' => '#/components/schemas/Error404'
+
+          run_test!
+        end
+      end
+    end
+  end
+end