From 37f217b4ff166a414b655f5e35edfe886520a8fb Mon Sep 17 00:00:00 2001
From: Zee <50284+zspencer@users.noreply.github.com>
Date: Sun, 9 Apr 2023 18:16:43 -0700
Subject: [PATCH] =?UTF-8?q?=F0=9F=A7=B9=F0=9F=90=9E=F0=9F=8C=B8=20`Marketp?=
 =?UTF-8?q?lace`:=20Tidy=20up=20`Product#index`=20(#1337)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* 🧹🐞 `Marketplace`: Fix broken link on `Product#edit` page

- https://github.com/zinc-collective/convene/issues/1187
- https://github.com/zinc-collective/convene/issues/1324

There is no `Product#show` 🙃

Other Changes:
- 🧹`Marketplace`: Use `ButtonComponent` instead of `buttons/edit`
- 🧹`Components`: Remove `buttons/edit`

* 🌸`Marketplace`: `Product#index` is a Grid of Cards

- https://github.com/zinc-collective/convene/issues/1324

The table layout was not great on mobile, and this is slightly better.

* 🧹`Components`: Inline `buttons/new` partial

* 🌸`Marketplace`: Increase shown precision of `TaxRate`

* 🧹 `Marketplace`: Include better words in the `Product#destroy` button

* 🌸 `Marketplace`: `Products#index` is single column on small devices

* 🛠️`Components`: `ApplicationComponent` supports `dom_id`

If a `dom_id` is set on the component, it will populate the elements
`id` field with the `dom_id`.

It it's not, but `dom_id` is called with arguments it will delegate that
to `ViewComponent::Base.dom_id`

* ✨ `Marketplace`: `Products#create` shows validation errors

* 🌸✨ `Marketplace`: `Product#destroy` removes elements
---
 app/components/application_component.rb       | 13 ++++--
 app/furniture/marketplace/breadcrumbs.rb      |  4 +-
 app/furniture/marketplace/locales/en.yml      | 13 +++---
 .../marketplace/products/_product.html.erb    | 40 +++++++++----------
 .../marketplace/products/index.html.erb       | 38 +++++-------------
 .../marketplace/products/new.html.erb         |  5 +--
 .../marketplace/products_controller.rb        | 13 ++++--
 .../marketplace/tax_rates/_tax_rate.html.erb  |  2 +-
 app/views/buttons/_edit.html.erb              | 10 -----
 app/views/buttons/_new.html.erb               |  6 ---
 .../products_controller_request_spec.rb       |  7 ++++
 11 files changed, 69 insertions(+), 82 deletions(-)
 delete mode 100644 app/views/buttons/_edit.html.erb
 delete mode 100644 app/views/buttons/_new.html.erb

diff --git a/app/components/application_component.rb b/app/components/application_component.rb
index 067399aca..eddcfd62a 100644
--- a/app/components/application_component.rb
+++ b/app/components/application_component.rb
@@ -1,19 +1,26 @@
 class ApplicationComponent < ViewComponent::Base
   attr_accessor :data
-  attr_writer :classes
+  attr_writer :classes, :dom_id
 
-  def initialize(data: {}, classes: "")
+  def initialize(data: {}, dom_id: nil, classes: "")
     self.data = data
     self.classes = classes
+    self.dom_id = dom_id
   end
 
   # @todo this should gracefully merge left passed in data
   def attributes(classes: "")
-    tag_builder.tag_options(data: data, class: self.classes(classes))
+    tag_builder.tag_options(id: dom_id, data: data, class: self.classes(classes))
   end
 
   # @todo how do we want to handle when tailwind is given conflicting classes? i.e. `mt-5 mt-3`
   def classes(others = "")
     "#{@classes} #{others}".strip.split.compact.uniq.join(" ")
   end
+
+  def dom_id(*args, **kwargs)
+    return @dom_id if @dom_id
+    return if args.blank?
+    super
+  end
 end
diff --git a/app/furniture/marketplace/breadcrumbs.rb b/app/furniture/marketplace/breadcrumbs.rb
index 3436bd855..8064a7534 100644
--- a/app/furniture/marketplace/breadcrumbs.rb
+++ b/app/furniture/marketplace/breadcrumbs.rb
@@ -43,8 +43,8 @@
 end
 
 crumb :edit_marketplace_product do |product|
-  parent :marketplace_product, product
-  link "Edit", product.location(:edit)
+  parent :marketplace_products, product.marketplace
+  link t("marketplace.products.edit.link_to", name: product.name), product.location(:edit)
 end
 
 crumb :marketplace_delivery_areas do |marketplace|
diff --git a/app/furniture/marketplace/locales/en.yml b/app/furniture/marketplace/locales/en.yml
index af96ea920..7b7f43c89 100644
--- a/app/furniture/marketplace/locales/en.yml
+++ b/app/furniture/marketplace/locales/en.yml
@@ -50,11 +50,14 @@ en:
     products:
       index:
         link_to: "Configure Products"
-        new: Add a Product
-      product:
-        add: Add Product
-        remove: Remove Product
-        edit: Edit Product
+      new:
+        link_to: Add a Product
+      create:
+        success: "Added Product '%{name}'"
+      edit:
+        link_to: "Edit Product '%{name}'"
+      destroy:
+        link_to: "Remove Product '%{name}"
     tax_rates:
       index:
         link_to: "Tax Rates"
diff --git a/app/furniture/marketplace/products/_product.html.erb b/app/furniture/marketplace/products/_product.html.erb
index b19bb3d10..d4471ee42 100644
--- a/app/furniture/marketplace/products/_product.html.erb
+++ b/app/furniture/marketplace/products/_product.html.erb
@@ -1,24 +1,20 @@
-<tr id="<%=dom_id(product)%>">
-  <td class="w-full max-w-0 py-4 pl-4 pr-3 text-sm font-medium text-gray-900 sm:w-auto sm:max-w-none sm:pl-6">
-    <%= product.name %>
-    <dl class="font-normal lg:hidden">
-      <dt class="sr-only"><%= product.class.human_attribute_name(:description) %></dt>
-      <dd class="mt-1 truncate text-gray-700"><%= product.description %></dd>
-    </dl>
-  </td>
-  <td class="hidden px-3 py-4 text-sm text-gray-500 lg:table-cell">
-    <%= product.description %>
-  </td>
 
-  <td class="hidden px-3 py-4 text-sm text-gray-500 sm:table-cell">
-    <%= render product.tax_rates %>
-  </td>
+<%= render CardComponent.new(dom_id: dom_id(product)) do %>
+  <div class="flex justify-between">
+    <h3 class="py-2"><%= product.name %></h3>
+    <span class="flex justify-between">
+      <%= render ButtonComponent.new(label: "#{t('icons.pencil')}",  title: t('marketplace.products.edit.link_to', name: product.name), href: product.location(:edit), method: :get) %>
+      <%= render ButtonComponent.new(label: t('icons.remove'), title: t('marketplace.products.destroy.link_to', name: product.name), href: product.location, method: :delete) %>
+    </span>
+  </div>
+  <div class="text-sm italic">
+    <%= product.description %>
+  </div>
 
-  <td class="hidden px-3 py-4 text-sm text-gray-500 sm:table-cell">
-    <%= humanized_money_with_symbol(product.price) %>
-  </td>
-  <td class="py-4 pl-3 pr-4 text-right text-sm font-medium sm:pr-6">
-    <%= render "buttons/edit", label: "#{t('icons.pencil')}",  title: t('.edit'), href: product.location(:edit), method: :get %>
-    <%= render ButtonComponent.new(label: t('icons.remove'), title: t('.remove'), href: product.location, method: :delete) %>
-  </td>
-</tr>
+  <div class="text-right">
+    <p><%= humanized_money_with_symbol(product.price) %></p>
+    <div class="text-xs italic">
+      <%= product.tax_rates.map { |tax_rate| "#{tax_rate.label} #{number_to_percentage(tax_rate.tax_rate, precision: 2)}" }.to_sentence %>
+    </div>
+  </div>
+<%- end %>
diff --git a/app/furniture/marketplace/products/index.html.erb b/app/furniture/marketplace/products/index.html.erb
index 5ff01ff5c..86ed9cb80 100644
--- a/app/furniture/marketplace/products/index.html.erb
+++ b/app/furniture/marketplace/products/index.html.erb
@@ -1,30 +1,14 @@
 <%- breadcrumb :marketplace_products, marketplace%>
-<div class="-mx-4 mt-8 overflow-hidden shadow ring-1 ring-black ring-opacity-5 sm:-mx-6 md:mx-0 md:rounded-lg">
-  <table class="min-w-full divide-y divide-gray-300">
-    <thead class="bg-gray-50">
-      <tr>
-        <th scope="col" class="py-3.5 pl-4 pr-3 text-left text-sm font-semibold text-gray-900 sm:pl-6">
-          <%= Marketplace::Product.human_attribute_name(:name) %>
-        </th>
-        <th scope="col" class="hidden px-3 py-3.5 text-left text-sm font-semibold text-gray-900 lg:table-cell">
-          <%= Marketplace::Product.human_attribute_name(:description) %>
-        </th>
+<div class="grid grid-cols-1 gap-4 sm:grid-cols-2 mt-2 sm:mt-4 max-w-prose mx-auto">
+  <%= render marketplace.products %>
 
-        <th scope="col" class="hidden px-3 py-3.5 text-left text-sm font-semibold text-gray-900 sm:table-cell">
-          <%= Marketplace::Product.human_attribute_name(:tax_rates) %>
-        </th>
-
-        <th scope="col" class="hidden px-3 py-3.5 text-left text-sm font-semibold text-gray-900 sm:table-cell">
-          <%= Marketplace::Product.human_attribute_name(:price) %>
-        </th>
-      </tr>
-    </thead>
-    <tbody class="divide-y divide-gray-200 bg-white">
-      <%= render marketplace.products %>
-    </tbody>
-  </table>
+  <div class="sm:col-span-2 text-right">
+    <%- new_product = marketplace.products.new %>
+    <%- if policy(new_product).create? %>
+      <%= render ButtonComponent.new(
+        label: "#{t('marketplace.products.new.link_to')} #{t('icons.new')}",
+        title: t('marketplace.products.new.link_to'),
+        href: marketplace.location(:new, child: :product), method: :get) %>
+    <%- end %>
+  </div>
 </div>
-<%- new_product = marketplace.products.new %>
-<%- if policy(new_product).create? %>
-  <%= render "buttons/new", label: "#{t('.new')} #{t('icons.new')}", title: t('.new'), href: marketplace.location(:new, child: :product) %>
-<%- end %>
diff --git a/app/furniture/marketplace/products/new.html.erb b/app/furniture/marketplace/products/new.html.erb
index 9b7d915a2..361bde067 100644
--- a/app/furniture/marketplace/products/new.html.erb
+++ b/app/furniture/marketplace/products/new.html.erb
@@ -1,3 +1,2 @@
-<%- new_product = marketplace.products.new %>
-<%- breadcrumb :new_marketplace_product, new_product %>
-<%= render "form", product: marketplace.products.new %>
+<%- breadcrumb :new_marketplace_product, product %>
+<%= render "form", product: product %>
diff --git a/app/furniture/marketplace/products_controller.rb b/app/furniture/marketplace/products_controller.rb
index bf6f8d08c..e14bdd59d 100644
--- a/app/furniture/marketplace/products_controller.rb
+++ b/app/furniture/marketplace/products_controller.rb
@@ -3,15 +3,20 @@
 class Marketplace
   class ProductsController < Controller
     def new
-      authorize(marketplace.products.new)
     end
 
     def create
       authorize(product)
-      product.save!
+      product.save
 
       respond_to do |format|
-        format.html { redirect_to marketplace.location(child: :products) }
+        format.html do
+          if product.persisted?
+            redirect_to marketplace.location(child: :products), notice: t(".success", name: product.name)
+          else
+            render :new, status: :unprocessable_entity
+          end
+        end
       end
     end
 
@@ -46,6 +51,8 @@ def update
         policy_scope(marketplace.products).find(params[:id])
       elsif params[:product]
         marketplace.products.new(product_params)
+      else
+        marketplace.products.new
       end.tap { |product| authorize(product) }
     end
 
diff --git a/app/furniture/marketplace/tax_rates/_tax_rate.html.erb b/app/furniture/marketplace/tax_rates/_tax_rate.html.erb
index bbea64277..be287c825 100644
--- a/app/furniture/marketplace/tax_rates/_tax_rate.html.erb
+++ b/app/furniture/marketplace/tax_rates/_tax_rate.html.erb
@@ -1,3 +1,3 @@
 <%= link_to tax_rate.location(:edit) do %>
-  <%= tax_rate.label %>: <%= number_to_percentage(tax_rate.tax_rate, precision: 1) %>
+  <%= tax_rate.label %>: <%= number_to_percentage(tax_rate.tax_rate, precision: 2) %>
 <%- end %>
diff --git a/app/views/buttons/_edit.html.erb b/app/views/buttons/_edit.html.erb
deleted file mode 100644
index 5e0d3b726..000000000
--- a/app/views/buttons/_edit.html.erb
+++ /dev/null
@@ -1,10 +0,0 @@
-<%- label = local_assigns.fetch(:label, t('icons.edit_pencil')) %>
-<%- title = local_assigns.fetch(:title) %>
-<%- href = local_assigns.fetch(:href) %>
-<%- method = local_assigns.fetch(:method, :put)%>
-<%- data = { turbo_method: method, turbo: true } %>
-<%- data = data.merge(turbo_confirm: local_assigns[:confirm], confirm: local_assigns[:confirm]) if local_assigns.key?(:confirm) %>
-
-<%= link_to label, href, title: title, method: method,
-              data: data,
-              class: 'no-underline bg-transparent hover:bg-primary-100 button' %>
diff --git a/app/views/buttons/_new.html.erb b/app/views/buttons/_new.html.erb
deleted file mode 100644
index 70afe93a8..000000000
--- a/app/views/buttons/_new.html.erb
+++ /dev/null
@@ -1,6 +0,0 @@
-<%- label = local_assigns.fetch(:label, t('icons.new')) %>
-<%- title = local_assigns.fetch(:title) %>
-<%- href = local_assigns.fetch(:href) %>
-
-<%= link_to label, href, title: title,
-              class: 'no-underline hover:bg-primary-400 bg-primary-500 font-bold py-2 px-4 my-1 rounded text-white text-center transition ease-in-out duration-150 ' %>
diff --git a/spec/furniture/marketplace/products_controller_request_spec.rb b/spec/furniture/marketplace/products_controller_request_spec.rb
index e36deb342..7aab38fac 100644
--- a/spec/furniture/marketplace/products_controller_request_spec.rb
+++ b/spec/furniture/marketplace/products_controller_request_spec.rb
@@ -10,6 +10,7 @@
     subject(:perform_request) do
       post polymorphic_path([space, room, marketplace, :products]),
         params: {product: product_attributes}
+      response
     end
 
     let(:tax_rate) { create(:marketplace_tax_rate, marketplace: marketplace) }
@@ -32,6 +33,12 @@
       specify { expect(created_product.price_currency).to eql(Money.default_currency.to_s) }
       specify { expect(created_product.tax_rates).to include(tax_rate) }
     end
+
+    describe "when product is invalid" do
+      let(:product_attributes) { {} }
+
+      it { is_expected.to have_rendered(:new) }
+    end
   end
 
   describe "#edit" do