Merge pull request #6270 from chaimann/admin-product-form-improvements

[Admin] Product form improvements

Author: tvdeyen

admin/app/components/solidus_admin/products/show/component.html.erb

@@ -26,13 +26,13 @@
   <%= form_for @product, url: solidus_admin.product_path(@product), html: { id: form_id } do |f| %>
     <%= page_with_sidebar do %>
       <%= page_with_sidebar_main do %>
-        <%= render component('ui/panel').new do %>
+        <%= render component("ui/panel").new do %>
           <%= render component("ui/forms/field").text_field(f, :name) %>
           <%= render component("ui/forms/field").text_field(f, :slug) %>
           <%= render component("ui/forms/field").text_area(f, :description) %>
         <% end %>
 
-        <%= render component('ui/panel').new(title: 'SEO') do %>
+        <%= render component("ui/panel").new(title: t(".seo")) do %>
           <%= render component("ui/forms/field").text_field(f, :meta_title) %>
           <%= render component("ui/forms/field").text_field(f, :meta_description) %>
           <%= render component("ui/forms/field").text_area(f, :meta_keywords) %>
@@ -41,25 +41,25 @@
             f,
             :condition,
             condition_options,
-            include_blank: t('spree.unset'),
+            include_blank: t("spree.unset"),
           ) %>
         <% end %>
-        <%= render component('ui/panel').new(title: "Media") do |panel| %>
+        <%= render component("ui/panel").new(title: t(".media")) do |panel| %>
           <% panel.with_action(
             name: t(".manage_images"),
             href: spree.admin_product_images_path(@product)
           ) %>
         <% end %>
 
-        <%= render component('ui/panel').new(title: 'Pricing') do %>
+        <%= render component("ui/panel").new(title: t(".pricing")) do %>
           <%= render component("ui/forms/field").text_field(f, :price) %>
           <div class="flex gap-4 justify-items-stretch">
             <%= render component("ui/forms/field").text_field(f, :cost_price) %>
             <%= render component("ui/forms/field").text_field(f, :cost_currency) %>
           </div>
         <% end %>
 
-        <%= render component('ui/panel').new(title: 'Stock') do |panel| %>
+        <%= render component("ui/panel").new(title: t(".stock")) do |panel| %>
           <%= render component("ui/forms/field").text_field(f, :sku) %>
 
           <% panel.with_action(
@@ -68,25 +68,22 @@
           ) %>
         <% end %>
 
-        <%= render component('ui/panel').new(title: 'Shipping') do %>
+        <%= render component("ui/panel").new(title: t(".shipping")) do %>
           <%= render component("ui/forms/field").select(
             f,
             :shipping_category_id,
             [[t(".none"), nil]] + Spree::ShippingCategory.order(:name).pluck(:name, :id),
-            tip: t(".hints.shipping_category_html"),
+            tip: t(".hints.shipping_category_html")
           ) %>
           <%= render component("ui/forms/field").select(
             f,
             :tax_category_id,
             [[t(".none"), nil]] + Spree::TaxCategory.order(:name).pluck(:name, :id),
-            tip: t(
-              ".hints.tax_category_html",
-              default_tax_category: Spree::TaxCategory.default&.name
-            ),
+            tip: t(".hints.tax_category_html")
           ) %>
         <% end %>
 
-        <%= render component('ui/panel').new(title: "Options") do %>
+        <%= render component("ui/panel").new(title: t(".options")) do %>
           <%= render component("ui/forms/field").select(
             f,
             :option_type_ids,
@@ -96,7 +93,7 @@
           ) %>
         <% end %>
 
-        <%= render component('ui/panel').new(title: "Specifications") do |panel| %>
+        <%= render component("ui/panel").new(title: t(".specifications")) do |panel| %>
           <% panel.with_action(
             name: t(".manage_properties"),
             href: spree.admin_product_product_properties_path(@product)
@@ -105,18 +102,18 @@
       <% end %>
 
       <%= page_with_sidebar_aside do %>
-        <%= render component('ui/panel').new(title: "Publishing") do %>
+        <%= render component("ui/panel").new(title: t(".publishing")) do %>
           <%= render component("ui/forms/field").text_field(
             f,
             :available_on,
-            hint: t(".available_on_html"),
+            hint: t(".hints.available_on_html"),
             type: :date,
             value: f.object.available_on&.to_date
           ) %>
           <%= render component("ui/forms/field").text_field(
             f,
             :discontinue_on,
-            hint: t(".discontinue_on_html"),
+            hint: t(".hints.discontinue_on_html"),
             type: :date,
             value: f.object.discontinue_on&.to_date
           ) %>
@@ -130,7 +127,7 @@
           </label>
         <% end %>
 
-        <%= render component('ui/panel').new(title: "Product organization") do %>
+        <%= render component("ui/panel").new(title: t(".product_organization")) do %>
           <%= render component("ui/forms/field").select(
             f,
             :taxon_ids,

admin/app/components/solidus_admin/products/show/component.yml

@@ -4,14 +4,23 @@ en:
   duplicate: "Duplicate"
   view: "View online"
   delete: "Delete"
-  none: "None"
+  delete_confirmation: "Are you sure you want to delete this product?"
   manage_images: "Manage images"
   manage_properties: "Manage product specifications"
   manage_stock: "Manage stock"
-  delete_confirmation: "Are you sure you want to delete this product?"
-  available_on_html: 'Product availability starts from the set date.<br> Empty date indicates no availability.'
-  discontinue_on_html: 'Product availability ends from the set date.<br> Empty date indicates continuous availability.'
+  media: "Media"
+  none: "None"
+  options: "Options"
+  pricing: "Pricing"
+  product_organization: "Product organization"
+  publishing: "Publishing"
+  seo: "SEO"
+  stock: "Stock"
+  shipping: "Shipping"
+  specifications: "Specifications"
   hints:
-    promotionable_html: Promotions can apply to this product
-    shipping_category_html: Manage Shipping in Settings
-    tax_category_html: Manage Taxes in Settings
+    available_on_html: "Product availability starts from the set date.<br> Empty date indicates no availability."
+    discontinue_on_html: "Product availability ends from the set date.<br> Empty date indicates continuous availability."
+    promotionable_html: "Promotions can apply to this product"
+    shipping_category_html: "Manage Shipping in Settings"
+    tax_category_html: "Manage Taxes in Settings"

admin/app/components/solidus_admin/ui/panel/component.rb

@@ -32,6 +32,7 @@ def initialize(title: nil, title_hint: nil)
   def render_section(wide: false, high: false, **args, &block)
     tag.section(**args, class: "
       border-gray-100 border-t w-full first-of-type:border-t-0
+      flex flex-col gap-6
       #{'px-6' unless wide}
       #{'py-4' unless high}
       #{args[:class]}

admin/app/controllers/solidus_admin/products_controller.rb

@@ -11,8 +11,6 @@ class ProductsController < SolidusAdmin::BaseController
     search_scope(:in_stock) { _1.where(id: Spree::Variant.in_stock.distinct.select(:product_id)) }
     search_scope(:out_of_stock) { _1.where.not(id: Spree::Variant.in_stock.distinct.select(:product_id)) }
 
-    before_action :split_params, only: [:update]
-
     def index
       products = apply_search_to(
         Spree::Product.includes(:master, :variants),
@@ -44,7 +42,7 @@ def show
     def update
       @product = Spree::Product.friendly.find(params[:id])
 
-      if @product.update(params.require(:product).permit!)
+      if @product.update(product_params)
         flash[:success] = t('spree.successfully_updated', resource: [
           Spree::Product.model_name.human,
           @product.name.inspect,
@@ -101,13 +99,12 @@ def activate
       redirect_to products_path, status: :see_other
     end
 
-    def split_params
-      if params[:product][:taxon_ids].present?
-        params[:product][:taxon_ids] = params[:product][:taxon_ids].split(',')
-      end
-      if params[:product][:option_type_ids].present?
-        params[:product][:option_type_ids] = params[:product][:option_type_ids].split(',')
-      end
+    private
+
+    def product_params
+      params.require(:product).permit(:name, :slug, :description, :meta_title, :meta_description, :meta_keywords, :gtin,
+        :condition, :price, :cost_price, :cost_currency, :sku, :shipping_category_id, :tax_category_id,
+        :available_on, :discontinue_on, :promotionable, option_type_ids: [], taxon_ids: [])
     end
   end
 end

admin/spec/requests/solidus_admin/products_spec.rb

@@ -0,0 +1,47 @@
+# frozen_string_literal: true
+
+require "spec_helper"
+
+RSpec.describe "SolidusAdmin::PropertiesController", type: :request do
+  let(:admin_user) { create(:admin_user) }
+
+  before do
+    allow_any_instance_of(SolidusAdmin::BaseController).to receive(:spree_current_user).and_return(admin_user)
+  end
+
+  describe "PATCH #update" do
+    let(:product) { create(:product) }
+    let(:params) do
+      {
+        name: "T-Shirt",
+        description: "Nice T-Shirt",
+        slug: "nice-t-shirt",
+        meta_title: "Nice T-Shirt",
+        meta_description: "It is a really nice T-Shirt",
+        meta_keywords: "tshirt, tee",
+        gtin: "12345",
+        condition: "new",
+        price: 100,
+        cost_price: 100,
+        cost_currency: "USD",
+        sku: "T123",
+        shipping_category_id: create(:shipping_category).id,
+        tax_category_id: create(:tax_category).id,
+        available_on: "2025-05-28".to_date,
+        discontinue_on: "2026-01-06".to_date,
+        promotionable: true,
+        option_type_ids: [create(:option_type).id, create(:option_type).id],
+        taxon_ids: [create(:taxon).id, create(:taxon).id],
+      }
+    end
+
+    it "updates product" do
+      patch solidus_admin.product_path(product), params: { product: params }
+      expect(response).to have_http_status(:see_other)
+      expect(product.reload).to have_attributes(params.except(Spree::Product::MASTER_ATTRIBUTES))
+      %i[gtin condition price cost_price cost_currency sku].each do |attr|
+        expect(product.public_send(attr)).to eq(params[attr])
+      end
+    end
+  end
+end