From 722b04a211bd63158f548b27b371c2c05cca1575 Mon Sep 17 00:00:00 2001 From: Gaetan Craig-Riou Date: Wed, 8 Feb 2023 11:26:48 +1100 Subject: [PATCH 1/4] Add missing collumns on new_products page It also includes specs for ProductComponent --- app/components/product_component.rb | 43 +++++++++-- app/components/products_table_component.rb | 71 ++++++++++++------ config/locales/en.yml | 14 ++++ spec/components/product_component_spec.rb | 85 ++++++++++++++++++++++ 4 files changed, 186 insertions(+), 27 deletions(-) create mode 100644 spec/components/product_component_spec.rb diff --git a/app/components/product_component.rb b/app/components/product_component.rb index e90eaa33be..62b1019b15 100644 --- a/app/components/product_component.rb +++ b/app/components/product_component.rb @@ -1,30 +1,61 @@ # frozen_string_literal: true class ProductComponent < ViewComponentReflex::Component + DATETIME_FORMAT = '%F %T' + def initialize(product:, columns:) super @product = product @image = @product.images[0] if product.images.any? - @columns = columns.map { |c| + @columns = columns.map do |c| { id: c[:value], value: column_value(c[:value]) } - } + end + end + + # This must be define when using ProductComponent.with_collection() + def collection_key + @product.id end def column_value(column) + # unless it is a column that needs specific formatting, we just use the product method + return @product.public_send(column.to_sym) if use_product_method?(column) + case column - when 'name' - @product.name - when 'price' - @product.price when 'unit' "#{@product.unit_value} #{@product.variant_unit}" when 'producer' @product.supplier.name when 'category' @product.taxons.map(&:name).join(', ') + when 'on_hand' + return 0 if @product.on_hand.nil? + + @product.on_hand + when 'tax_category' + @product.tax_category.name + when 'available_on' + format_date(@product.available_on) + when 'import_date' + format_date(@product.import_date) end end + + private + + def use_product_method?(column) + # columns that need some specific formatting + exceptions = %w[on_hand tax_category available_on import_date] + + @product.respond_to?(column.to_sym) && !exceptions.include?(column) + end + + def format_date(date) + return '' if date.nil? + + date.strftime(DATETIME_FORMAT) + end end diff --git a/app/components/products_table_component.rb b/app/components/products_table_component.rb index 6d9dc7de65..4db7749adb 100644 --- a/app/components/products_table_component.rb +++ b/app/components/products_table_component.rb @@ -3,27 +3,37 @@ class ProductsTableComponent < ViewComponentReflex::Component include Pagy::Backend - SORTABLE_COLUMNS = ["name"].freeze - SELECTABLE_COMUMNS = [{ label: I18n.t("admin.products_page.columns_selector.price"), - value: "price" }, - { label: I18n.t("admin.products_page.columns_selector.unit"), - value: "unit" }, - { label: I18n.t("admin.products_page.columns_selector.producer"), - value: "producer" }, - { label: I18n.t("admin.products_page.columns_selector.category"), - value: "category" }].sort { |a, b| + SORTABLE_COLUMNS = ['name', 'import_date'].freeze + SELECTABLE_COMUMNS = [ + { label: I18n.t("admin.products_page.columns_selector.price"), value: "price" }, + { label: I18n.t("admin.products_page.columns_selector.unit"), value: "unit" }, + { label: I18n.t("admin.products_page.columns_selector.producer"), value: "producer" }, + { label: I18n.t("admin.products_page.columns_selector.category"), value: "category" }, + { label: I18n.t("admin.products_page.columns_selector.sku"), value: "sku" }, + { label: I18n.t("admin.products_page.columns_selector.on_hand"), value: "on_hand" }, + { label: I18n.t("admin.products_page.columns_selector.on_demand"), value: "on_demand" }, + { label: I18n.t("admin.products_page.columns_selector.tax_category"), value: "tax_category" }, + { + label: I18n.t("admin.products_page.columns_selector.inherits_properties"), + value: "inherits_properties" + }, + { label: I18n.t("admin.products_page.columns_selector.available_on"), value: "available_on" }, + { label: I18n.t("admin.products_page.columns_selector.import_date"), value: "import_date" } + ].sort do |a, b| a[:label] <=> b[:label] - }.freeze + end.freeze + PER_PAGE_VALUE = [10, 25, 50, 100].freeze PER_PAGE = PER_PAGE_VALUE.map { |value| { label: value, value: value } } - NAME_COLUMN = { label: I18n.t("admin.products_page.columns.name"), value: "name", - sortable: true }.freeze + NAME_COLUMN = { + label: I18n.t("admin.products_page.columns.name"), value: "name", sortable: true + }.freeze def initialize(user:) super @user = user @selectable_columns = SELECTABLE_COMUMNS - @columns_selected = ["price", "unit"] + @columns_selected = ['unit', 'price', 'on_hand', 'category', 'import_date'] @per_page = PER_PAGE @per_page_selected = [10] @categories = [{ label: "All", value: "all" }] + @@ -40,16 +50,20 @@ class ProductsTableComponent < ViewComponentReflex::Component @search_term = "" end + # any change on a "reflex_data_attributes" (defined in the template) will trigger a re render def before_render fetch_products refresh_columns end + # Element refers to the component the data is set on def search_term + # Element is SearchInputComponent @search_term = element.dataset['value'] end def toggle_column + # Element is SelectorComponent column = element.dataset['value'] @columns_selected = if @columns_selected.include?(column) @columns_selected - [column] @@ -59,26 +73,33 @@ class ProductsTableComponent < ViewComponentReflex::Component end def click_sort - @sort = { column: element.dataset['sort-value'], - direction: element.dataset['sort-direction'] == "asc" ? "desc" : "asc" } + # Element is TableHeaderComponent + @sort = { + column: element.dataset['sort-value'], + direction: element.dataset['sort-direction'] == "asc" ? "desc" : "asc" + } end def toggle_per_page + # Element is SelectorComponent selected = element.dataset['value'].to_i @per_page_selected = [selected] if PER_PAGE_VALUE.include?(selected) end def toggle_category + # Element is SelectorWithFilterComponent category_clicked = element.dataset['value'] @categories_selected = toggle_selector_with_filter(category_clicked, @categories_selected) end def toggle_producer + # Element is SelectorWithFilterComponent producer_clicked = element.dataset['value'] @producers_selected = toggle_selector_with_filter(producer_clicked, @producers_selected) end def change_page + # Element is PaginationComponent page = element.dataset['page'].to_i @page = page if page > 0 end @@ -86,10 +107,13 @@ class ProductsTableComponent < ViewComponentReflex::Component private def refresh_columns - @columns = @columns_selected.map { |column| - { label: I18n.t("admin.products_page.columns.#{column}"), value: column, - sortable: SORTABLE_COLUMNS.include?(column) } - }.sort! { |a, b| a[:label] <=> b[:label] } + @columns = @columns_selected.map do |column| + { + label: I18n.t("admin.products_page.columns.#{column}"), + value: column, + sortable: SORTABLE_COLUMNS.include?(column) + } + end.sort! { |a, b| a[:label] <=> b[:label] } @columns.unshift(NAME_COLUMN) end @@ -145,8 +169,13 @@ class ProductsTableComponent < ViewComponentReflex::Component def product_query_includes [ master: [:images], - variants: [:default_price, :stock_locations, :stock_items, :variant_overrides, - { option_values: :option_type }] + variants: [ + :default_price, + :stock_locations, + :stock_items, + :variant_overrides, + { option_values: :option_type } + ] ] end end diff --git a/config/locales/en.yml b/config/locales/en.yml index 019042608e..e785b1304f 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -484,11 +484,25 @@ en: price: Price producer: Producer category: Category + sku: SKU + on_hand: "On Hand" + on_demand: "On Demand" + tax_category: "Tax Category" + inherits_properties: "Inherits Properties?" + available_on: "Available On" + import_date: "Import Date" columns_selector: unit: Unit price: Price producer: Producer category: Category + sku: SKU + on_hand: "On Hand" + on_demand: "On Demand" + tax_category: "Tax Category" + inherits_properties: "Inherits Properties?" + available_on: "Available On" + import_date: "Import Date" adjustments: skipped_changing_canceled_order: "You can't change a cancelled order." # Common properties / models diff --git a/spec/components/product_component_spec.rb b/spec/components/product_component_spec.rb new file mode 100644 index 0000000000..8695d8c8c2 --- /dev/null +++ b/spec/components/product_component_spec.rb @@ -0,0 +1,85 @@ +# frozen_string_literal: true + +require "spec_helper" + +describe ProductComponent, type: :component do + let(:product) { create(:simple_product) } + + describe 'unit' do + before do + render_inline(ProductComponent.new( + product: product, columns: [{ label: "Unit", value: "unit", sortable: false }] + )) + end + + it 'concatenates the unit value and the unit description' do + expect(page.find('.unit')).to have_content '1.0 weight' + end + end + + describe 'category' do + let(:product) do + product = create(:simple_product) + product.taxons << create(:taxon, name: 'random') + + product + end + + before do + render_inline(ProductComponent.new( + product: product, columns: [{ label: "Category", value: "category", sortable: false }] + )) + end + + it "joins the categories' name" do + expect(page.find('.category')).to have_content(product.taxons.map(&:name).join(', ')) + end + end + + describe 'on_hand' do + let(:product) { create(:simple_product, on_hand: on_hand) } + let(:on_hand) { 5 } + + before do + render_inline(ProductComponent.new( + product: product, columns: [{ label: "On Hand", value: "on_hand", sortable: false }] + )) + end + + it 'return product on_hand' do + expect(page.find('.on_hand')).to have_content(on_hand) + end + + context 'when on_hand is nil' do + let(:on_hand) { nil } + + it 'returns 0' do + expect(page.find('.on_hand')).to have_content(0.to_s) + end + end + end + + # This also covers import_date + describe 'available_on' do + let(:product) { create(:simple_product, available_on: available_on) } + let(:available_on) { Time.zone.now } + + before do + render_inline(ProductComponent.new( + product: product, columns: [{ label: "Available On", value: "available_on", sortable: false }] + )) + end + + it 'return formated available_on' do + expect(page.find('.available_on')).to have_content(available_on.strftime('%F %T')) + end + + context 'when available_on is nil' do + let(:available_on) { nil } + + it 'returns an empty string' do + expect(page.find('.available_on')).to have_content('') + end + end + end +end From 313fdab346ae2e13db5a16614a8529964e2f6ae2 Mon Sep 17 00:00:00 2001 From: Gaetan Craig-Riou Date: Wed, 8 Feb 2023 11:53:44 +1100 Subject: [PATCH 2/4] Fix rubocop warnings --- spec/components/product_component_spec.rb | 33 ++++++++++++++--------- 1 file changed, 21 insertions(+), 12 deletions(-) diff --git a/spec/components/product_component_spec.rb b/spec/components/product_component_spec.rb index 8695d8c8c2..2259cbcc35 100644 --- a/spec/components/product_component_spec.rb +++ b/spec/components/product_component_spec.rb @@ -7,9 +7,11 @@ describe ProductComponent, type: :component do describe 'unit' do before do - render_inline(ProductComponent.new( - product: product, columns: [{ label: "Unit", value: "unit", sortable: false }] - )) + render_inline( + ProductComponent.new( + product: product, columns: [{ label: "Unit", value: "unit", sortable: false }] + ) + ) end it 'concatenates the unit value and the unit description' do @@ -26,9 +28,11 @@ describe ProductComponent, type: :component do end before do - render_inline(ProductComponent.new( - product: product, columns: [{ label: "Category", value: "category", sortable: false }] - )) + render_inline( + ProductComponent.new( + product: product, columns: [{ label: "Category", value: "category", sortable: false }] + ) + ) end it "joins the categories' name" do @@ -41,9 +45,11 @@ describe ProductComponent, type: :component do let(:on_hand) { 5 } before do - render_inline(ProductComponent.new( - product: product, columns: [{ label: "On Hand", value: "on_hand", sortable: false }] - )) + render_inline( + ProductComponent.new( + product: product, columns: [{ label: "On Hand", value: "on_hand", sortable: false }] + ) + ) end it 'return product on_hand' do @@ -65,9 +71,12 @@ describe ProductComponent, type: :component do let(:available_on) { Time.zone.now } before do - render_inline(ProductComponent.new( - product: product, columns: [{ label: "Available On", value: "available_on", sortable: false }] - )) + render_inline( + ProductComponent.new( + product: product, + columns: [{ label: "Available On", value: "available_on", sortable: false }] + ) + ) end it 'return formated available_on' do From 503e0ecba17188260e182451ecc6f959e4bccfae Mon Sep 17 00:00:00 2001 From: Gaetan Craig-Riou Date: Mon, 13 Feb 2023 14:46:09 +1100 Subject: [PATCH 3/4] Fix category spec as per code review - Use a hardcoded string for the expected categories (taxons), and fix taxon step up to not use default taxon - Fix constant typo --- app/components/products_table_component.rb | 4 ++-- spec/components/product_component_spec.rb | 11 +++++++---- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/app/components/products_table_component.rb b/app/components/products_table_component.rb index 4db7749adb..44e06e8eb1 100644 --- a/app/components/products_table_component.rb +++ b/app/components/products_table_component.rb @@ -4,7 +4,7 @@ class ProductsTableComponent < ViewComponentReflex::Component include Pagy::Backend SORTABLE_COLUMNS = ['name', 'import_date'].freeze - SELECTABLE_COMUMNS = [ + SELECTABLE_COLUMNS = [ { label: I18n.t("admin.products_page.columns_selector.price"), value: "price" }, { label: I18n.t("admin.products_page.columns_selector.unit"), value: "unit" }, { label: I18n.t("admin.products_page.columns_selector.producer"), value: "producer" }, @@ -32,7 +32,7 @@ class ProductsTableComponent < ViewComponentReflex::Component def initialize(user:) super @user = user - @selectable_columns = SELECTABLE_COMUMNS + @selectable_columns = SELECTABLE_COLUMNS @columns_selected = ['unit', 'price', 'on_hand', 'category', 'import_date'] @per_page = PER_PAGE @per_page_selected = [10] diff --git a/spec/components/product_component_spec.rb b/spec/components/product_component_spec.rb index 2259cbcc35..540f4befef 100644 --- a/spec/components/product_component_spec.rb +++ b/spec/components/product_component_spec.rb @@ -22,10 +22,11 @@ describe ProductComponent, type: :component do describe 'category' do let(:product) do product = create(:simple_product) - product.taxons << create(:taxon, name: 'random') + product.taxons = taxons product end + let(:taxons) { [create(:taxon, name: 'random 1'), create(:taxon, name: 'random 2')] } before do render_inline( @@ -36,7 +37,9 @@ describe ProductComponent, type: :component do end it "joins the categories' name" do - expect(page.find('.category')).to have_content(product.taxons.map(&:name).join(', ')) + expect(page.find('.category')).to have_content( + /random 1, random 2/, exact: true, normalize_ws: true + ) end end @@ -52,7 +55,7 @@ describe ProductComponent, type: :component do ) end - it 'return product on_hand' do + it 'returns product on_hand' do expect(page.find('.on_hand')).to have_content(on_hand) end @@ -79,7 +82,7 @@ describe ProductComponent, type: :component do ) end - it 'return formated available_on' do + it 'returns formated available_on' do expect(page.find('.available_on')).to have_content(available_on.strftime('%F %T')) end From 1cf55cde4d846d04b87cc60e9e689924598a4223 Mon Sep 17 00:00:00 2001 From: Gaetan Craig-Riou Date: Mon, 13 Feb 2023 15:57:10 +1100 Subject: [PATCH 4/4] Refactor column_value, to use a giant case A giant case is more readable that the previous 'smart' solution. It also disables a couple of Rubocop checks triggered by the giant case --- app/components/product_component.rb | 30 ++++++++++++++--------------- 1 file changed, 14 insertions(+), 16 deletions(-) diff --git a/app/components/product_component.rb b/app/components/product_component.rb index 62b1019b15..5d6ce0c9fc 100644 --- a/app/components/product_component.rb +++ b/app/components/product_component.rb @@ -20,42 +20,40 @@ class ProductComponent < ViewComponentReflex::Component @product.id end + # rubocop:disable Metrics/CyclomaticComplexity, Metrics/MethodLength def column_value(column) - # unless it is a column that needs specific formatting, we just use the product method - return @product.public_send(column.to_sym) if use_product_method?(column) - case column + when 'name' + @product.name + when 'price' + @product.price when 'unit' "#{@product.unit_value} #{@product.variant_unit}" when 'producer' @product.supplier.name when 'category' @product.taxons.map(&:name).join(', ') + when 'sku' + @product.sku when 'on_hand' - return 0 if @product.on_hand.nil? - - @product.on_hand + @product.on_hand || 0 + when 'on_demand' + @product.on_demand when 'tax_category' @product.tax_category.name + when 'inherits_properties' + @product.inherits_properties when 'available_on' format_date(@product.available_on) when 'import_date' format_date(@product.import_date) end end + # rubocop:enable Metrics/CyclomaticComplexity, Metrics/MethodLength private - def use_product_method?(column) - # columns that need some specific formatting - exceptions = %w[on_hand tax_category available_on import_date] - - @product.respond_to?(column.to_sym) && !exceptions.include?(column) - end - def format_date(date) - return '' if date.nil? - - date.strftime(DATETIME_FORMAT) + date&.strftime(DATETIME_FORMAT) || '' end end