Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUU] Change name of my products 🚧 #11208

Merged
merged 18 commits into from Aug 24, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
51 changes: 51 additions & 0 deletions app/reflexes/products_reflex.rb
Expand Up @@ -31,6 +31,24 @@ def clear_search
fetch_and_render_products
end

def bulk_update
product_set = product_set_from_params

product_set.collection.each { |p| authorize! :update, p }

if product_set.save
# flash[:success] = with_locale { I18n.t('.success') }
# morph_admin_flashes # ERROR: selector morph type has already been set

fetch_and_render_products
elsif product_set.errors.present?
# todo: render form with error messages
render json: { errors: product_set.errors }, status: :bad_request
else
render body: nil, status: :internal_server_error
end
end

private

def init_filters_params
Expand Down Expand Up @@ -130,4 +148,37 @@ def current_url
url.query += "&_category_id=#{@category_id}" if @category_id.present?
url.to_s
end

# Similar to spree/admin/products_controller
def product_set_from_params
# Form field names:
# '[products][<id>][name]'
#
# Resulting in params:
# "products" => {
# "<id>" => {
# "name" => "Pommes",
# }
# }

# For ModelSet, we transform to:
# {
# 0=> {:id=>"7449", "name"=>"Pommes"}
# }
#
# TO Consider: We could actually rearrange the form to suit that format more directly. eg:
# '[products][0][id]' (hidden field)
# '[products][0][name]'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess that this is necessary when you want to support creating several new products in the same form. Then you can't use the id as index. But I find this approach of stuffing all kind of data into one action unnecessarily complex. Doing lots of stuff at once has the challenge of lots of things potentially failing and providing lots of error feedback while also adding a lot of logic regarding different actions to one thing. In this case the ModelSet encapsulates it quite well but it's still complicated.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good points. At this stage, we want to provide that behaviour (updating multiple at once) so I think it makes sense to use the one form, for now. So far we haven't hit any issues with this method, so I'd avoid complicating it. For now..

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about new products or deleting products? Will there be a separate form or UX?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume, new products will continue to be with a specific form, and deleting would be one-by-one.

But if we needed to do it in bulk here, then we could.. but like you say it makes things complicated!

collection_hash = products_bulk_params[:products].map { |id, attributes|
{ id:, **attributes }
}.each_with_index.to_h { |p, i|
[i, p]
}
Sets::ProductSet.new(collection_attributes: collection_hash)
end

def products_bulk_params
params.permit(products: ::PermittedAttributes::Product.attributes)
.to_h.with_indifferent_access
end
end
138 changes: 73 additions & 65 deletions app/views/admin/products_v3/_table.html.haml
@@ -1,66 +1,74 @@
%table.products
%col{ width:"15%" }
%col{ width:"5%", style: "max-width:5em" }
%col{ width:"8%" }
%col{ width:"5%", style: "max-width:5em"}
%col{ width:"5%", style: "max-width:5em"}
%col{ width:"10%" }= # producer
%col{ width:"10%" }
%col{ width:"5%" }
%col{ width:"5%", style: "max-width:5em" }
%thead
%tr
%th.align-left= t('admin.products_page.columns.name')
%th.align-right= t('admin.products_page.columns.sku')
%th.align-right= t('admin.products_page.columns.unit')
%th.align-right= t('admin.products_page.columns.price')
%th.align-right= t('admin.products_page.columns.on_hand')
%th.align-left= t('admin.products_page.columns.producer')
%th.align-left= t('admin.products_page.columns.category')
%th.align-left= t('admin.products_page.columns.tax_category')
%th.align-left= t('admin.products_page.columns.inherits_properties')
- products.each do |product|
%tbody.relaxed
= form_with url: bulk_update_admin_products_v3_index_path, method: :patch,
html: {'data-reflex-serialize-form': true, 'data-reflex': 'submit->products#bulk_update'} do |form|
.container
.sixteen.columns.align-right
#fieldset
/ = t('.products_modified', count: 'X')
= form.submit t('.reset'), type: :reset
= form.submit t('.save')
%table.products
%col{ width:"15%" }
%col{ width:"5%", style: "max-width:5em" }
%col{ width:"8%" }
%col{ width:"5%", style: "max-width:5em"}
%col{ width:"5%", style: "max-width:5em"}
%col{ width:"10%" }= # producer
%col{ width:"10%" }
%col{ width:"5%" }
%col{ width:"5%", style: "max-width:5em" }
%thead
%tr
%td.align-left.header
.line-clamp-1= product.name
%td.align-right
.line-clamp-1= product.sku
%td.align-right
.line-clamp-1
= product.variant_unit.upcase_first
/ TODO: properly handle custom unit names
= WeightsAndMeasures::UNITS[product.variant_unit] && "(" + WeightsAndMeasures::UNITS[product.variant_unit][product.variant_unit_scale]["name"] + ")"
%td.align-right
-# empty
%td.align-right
-# TODO: new requirement "DISPLAY ON DEMAND IF ALL VARIANTS ARE ON DEMAND". And translate value
.line-clamp-1= if product.variants.all?(&:on_demand) then "On demand" else product.on_hand || 0 end
%td.align-left
.line-clamp-1= product.supplier.name
%td.align-left
.line-clamp-1= product.primary_taxon.name
%td.align-left
%td.align-left
.line-clamp-1= product.inherits_properties ? 'YES' : 'NO' #TODO: consider using https://github.com/RST-J/human_attribute_values, else use I18n.t (also below)
- product.variants.each do |variant|
%tr.condensed
%td.align-left
.line-clamp-1= variant.display_name
%td.align-right
.line-clamp-1= variant.sku
%td.align-right
.line-clamp-1= variant.unit_to_display
%td.align-right
.line-clamp-1= number_to_currency(variant.price)
%td.align-right
.line-clamp-1= variant.on_hand || 0 #TODO: spec for this according to requirements.
%td.align-left
.line-clamp-1= variant.product.supplier.name # same as product
%td.align-left
-# empty
%td.align-left
.line-clamp-1= variant.tax_category&.name || "None" # TODO: convert to dropdown, else translate hardcoded string.
%td.align-left
-# empty

%th.align-left= t('admin.products_page.columns.name')
%th.align-right= t('admin.products_page.columns.sku')
%th.align-right= t('admin.products_page.columns.unit')
%th.align-right= t('admin.products_page.columns.price')
%th.align-right= t('admin.products_page.columns.on_hand')
%th.align-left= t('admin.products_page.columns.producer')
%th.align-left= t('admin.products_page.columns.category')
%th.align-left= t('admin.products_page.columns.tax_category')
%th.align-left= t('admin.products_page.columns.inherits_properties')
- products.each do |product|
= form.fields_for(product) do |product_form|
%tbody.relaxed
%tr
%td.align-left.header
.line-clamp-1= product_form.text_field :name, name: "[products][#{product.id}][name]", id: "_product_name_#{product.id}", 'aria-label': t('admin.products_page.columns.name')
%td.align-right
.line-clamp-1= product.sku
%td.align-right
.line-clamp-1
= product.variant_unit.upcase_first
/ TODO: properly handle custom unit names
= WeightsAndMeasures::UNITS[product.variant_unit] && "(" + WeightsAndMeasures::UNITS[product.variant_unit][product.variant_unit_scale]["name"] + ")"
%td.align-right
-# empty
%td.align-right
-# TODO: new requirement "DISPLAY ON DEMAND IF ALL VARIANTS ARE ON DEMAND". And translate value
.line-clamp-1= if product.variants.all?(&:on_demand) then "On demand" else product.on_hand || 0 end
%td.align-left
.line-clamp-1= product.supplier.name
%td.align-left
.line-clamp-1= product.primary_taxon.name
%td.align-left
%td.align-left
.line-clamp-1= product.inherits_properties ? 'YES' : 'NO' #TODO: consider using https://github.com/RST-J/human_attribute_values, else use I18n.t (also below)
- product.variants.each do |variant|
%tr.condensed
%td.align-left
.line-clamp-1= variant.display_name
%td.align-right
.line-clamp-1= variant.sku
%td.align-right
.line-clamp-1= variant.unit_to_display
%td.align-right
.line-clamp-1= number_to_currency(variant.price)
%td.align-right
.line-clamp-1= variant.on_hand || 0 #TODO: spec for this according to requirements.
%td.align-left
.line-clamp-1= variant.product.supplier.name # same as product
%td.align-left
-# empty
%td.align-left
.line-clamp-1= variant.tax_category&.name || "None" # TODO: convert to dropdown, else translate hardcoded string.
%td.align-left
-# empty
5 changes: 5 additions & 0 deletions config/locales/en.yml
Expand Up @@ -818,6 +818,11 @@ en:
no_products_found: No products found
import_products: Import multiple products
no_products_found_for_search: No products found for your search criteria
table:
save: Save changes
reset: Discard changes
bulk_update:
success: "Products successfully updated"
product_import:
title: Product Import
file_not_found: File not found or could not be opened
Expand Down
4 changes: 3 additions & 1 deletion config/routes/admin.rb
Expand Up @@ -70,7 +70,9 @@
post '/product_import/reset_absent', to: 'product_import#reset_absent_products', as: 'product_import_reset_async'

constraints FeatureToggleConstraint.new(:admin_style_v3) do
resources :products_v3, only: :index
resources :products_v3, as: :products_v3, only: :index do
patch :bulk_update, on: :collection
end
end

resources :variant_overrides do
Expand Down
27 changes: 27 additions & 0 deletions spec/reflexes/products_reflex_spec.rb
Expand Up @@ -32,4 +32,31 @@
end
end
end

describe '#bulk_update' do
let!(:product_z) { create(:simple_product, name: "Zucchini") }
let!(:product_a) { create(:simple_product, name: "Apples") }

it "saves valid changes" do
params = {
# '[products][<id>][name]'
"products" => {
product_a.id.to_s => {
"name" => "Pommes",
}
}
}

expect{
reflex(:bulk_update, params:)
product_a.reload
}.to change(product_a, :name).to("Pommes")
dacook marked this conversation as resolved.
Show resolved Hide resolved
end
end
end

# Build and run a reflex using the context
# Parameters can be added with params: option
def reflex(method_name, opts = {})
build_reflex(method_name:, **context.merge(opts)).run(method_name)
end
35 changes: 31 additions & 4 deletions spec/system/admin/products_v3/products_spec.rb
Expand Up @@ -24,15 +24,21 @@
end

describe "sorting" do
let!(:product_z) { create(:simple_product, name: "Bananas") }
let!(:product_b) { create(:simple_product, name: "Bananas") }
let!(:product_a) { create(:simple_product, name: "Apples") }

before do
visit admin_products_v3_index_url
end

it "Should sort products alphabetically by default" do
expect(page).to have_content /Apples.*Bananas/
within "table.products" do
# Gather input values, because page.content doesn't include them.
input_content = page.find_all('input[type=text]').map(&:value).join

# Products are in correct order.
expect(input_content).to match /Apples.*Bananas/
end
end
end

Expand Down Expand Up @@ -91,7 +97,7 @@
search_for "searchable product"
expect(page).to have_field "search_term", with: "searchable product"
expect_products_count_to_be 1
expect(page).to have_selector "table.products tbody tr td", text: product_by_name.name
expect(page).to have_field "Name", with: product_by_name.name

click_link "Clear search"
expect(page).to have_field "search_term", with: ""
Expand Down Expand Up @@ -130,11 +136,32 @@

expect(page).to have_select "category_id", selected: "Category 1"
expect_products_count_to_be 1
expect(page).to have_selector "table.products tbody tr td", text: product_by_category.name
expect(page).to have_field "Name", with: product_by_category.name
end
end
end

describe "updating" do
before do
visit admin_products_v3_index_url
end

it "can update product fields" do
fill_in id: "_product_name_#{product_1.id}", with: "An updated name"

expect {
click_button "Save changes"
product_1.reload
}.to(
change { product_1.name }.to("An updated name")
)

expect(page).to have_field id: "_product_name_#{product_1.id}", with: "An updated name"
pending
expect(page).to have_content "Changes saved"
end
end

def expect_page_to_be(page_number)
expect(page).to have_selector ".pagination span.page.current", text: page_number.to_s
end
Expand Down