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

Marketplace: Add Product#tags #2190

Merged
merged 3 commits into from
Mar 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions app/furniture/marketplace/bazaar.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ class Marketplace
class Bazaar < ::Space
has_many :marketplaces, through: :rooms, source: :gizmos, inverse_of: :bazaar, class_name: "Marketplace"
has_many :tax_rates, inverse_of: :bazaar, dependent: :destroy
has_many :tags, inverse_of: :bazaar, dependent: :destroy

def space
becomes(Space)
Expand Down
10 changes: 10 additions & 0 deletions app/furniture/marketplace/breadcrumbs.rb
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,16 @@
link t("marketplace.stripe_accounts.show.link_to"), marketplace.location(child: :stripe_account)
end

crumb :marketplace_tags do |marketplace|
parent :edit_marketplace, marketplace
link(t("marketplace.tags.index.link_to"), marketplace.location(child: :tags))
end

crumb :new_marketplace_tag do |tag|
parent :marketplace_tags, tag.marketplace
link t("marketplace.tags.new.link_to"), marketplace.location(:new, child: :tag)
end

crumb :marketplace_tax_rates do |marketplace|
parent :edit_marketplace, marketplace
link t("marketplace.tax_rates.index.link_to"), marketplace.location(child: :tax_rates)
Expand Down
5 changes: 5 additions & 0 deletions app/furniture/marketplace/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -136,3 +136,8 @@ en:
payment_settings:
index:
link_to: "Payment Settings"
tags:
index:
link_to: "Tags"
new:
link_to: "Add Tag"
1 change: 1 addition & 0 deletions app/furniture/marketplace/management_component.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
<%= link_to_child(:notification_methods, icon: :bell_alert) if policy(marketplace.notification_methods).index? %>
<%= link_to_child(:flyer, icon: :qr_code) if policy(marketplace.flyer).show? %>
<%= link_to_child(:vendor_representatives, icon: :building_storefront) if policy(marketplace.vendor_representatives).index? %>
<%= link_to_child(:tags, icon: :tag) if policy(marketplace.tags).index? %>
</nav>
<% end %>
<% end %>
Expand Down
2 changes: 2 additions & 0 deletions app/furniture/marketplace/marketplace.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ class Marketplace < Furniture
# @todo replace with through :bazaar
has_many :tax_rates, inverse_of: :marketplace

has_many :tags, through: :bazaar

has_many :products, inverse_of: :marketplace, dependent: :destroy
has_many :carts, inverse_of: :marketplace, dependent: :destroy
has_many :orders, inverse_of: :marketplace
Expand Down
3 changes: 2 additions & 1 deletion app/furniture/marketplace/policy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ class Marketplace
class Policy < ApplicationPolicy
def create?
return true if current_person.operator?
return true if current_person.member_of?(marketplace.space)
return true if current_person.member_of?(space)
Copy link
Member Author

Choose a reason for hiding this comment

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

This allows us to have Models within the Marketplace that do not reference a Marketplace directly, such as TaxRate and Tag


return true if shopper&.person.blank? && !current_person.authenticated?

Expand All @@ -23,6 +23,7 @@ def marketplace

object.marketplace if object.respond_to?(:marketplace)
end
delegate :space, to: :marketplace

module SpecFactories
def self.included(spec)
Expand Down
3 changes: 3 additions & 0 deletions app/furniture/marketplace/product.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ class Product < Record
has_many :ordered_products, inverse_of: :product, dependent: :destroy
has_many :orders, -> { checked_out }, through: :ordered_products, inverse_of: :products

has_many :product_tags, inverse_of: :product, dependent: :destroy
has_many :tags, through: :product_tags, inverse_of: :products

has_many :product_tax_rates, inverse_of: :product, dependent: :destroy
has_many :tax_rates, through: :product_tax_rates, inverse_of: :products

Expand Down
4 changes: 4 additions & 0 deletions app/furniture/marketplace/product/title_component.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,7 @@
<%- if servings.present? %>
<p class="text-sm">Serves <%= servings %></p>
<%- end %>

<%- if product.tags.present? %>
<p>(<%= product.tags.pluck(:label).join(", ") %>)</p>
<%- end %>
2 changes: 1 addition & 1 deletion app/furniture/marketplace/product_policy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ class Marketplace
class ProductPolicy < Policy
alias_method :product, :object
def permitted_attributes(_params = nil)
%i[name description price_cents price_currency price photo restore servings] + [tax_rate_ids: []]
%i[name description price_cents price_currency price photo restore servings] + [tag_ids: [], tax_rate_ids: []]
end

def update?
Expand Down
8 changes: 8 additions & 0 deletions app/furniture/marketplace/product_tag.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
class Marketplace
class ProductTag < Record
self.table_name = :marketplace_product_tags

belongs_to :product, inverse_of: :product_tags
belongs_to :tag, inverse_of: :product_tags
end
end
2 changes: 2 additions & 0 deletions app/furniture/marketplace/products/_form.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
<%= render "number_field", { attribute: :servings, form: f, min: 0, step: 1} %>

<%= render "money_field", { attribute: :price, form: f, min: 0, step: 0.01} %>
<%= render "collection_check_boxes", { attribute: :tag_ids, collection: bazaar.tags, value_method: :id,
text_method: :label, form: f} %>
<%= render "collection_check_boxes", { attribute: :tax_rate_ids, collection: bazaar.tax_rates, value_method: :id, text_method: :label, form: f} %>
<%- if product.photo.present? %>
<div>
Expand Down
1 change: 1 addition & 0 deletions app/furniture/marketplace/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ def self.append_routes(router)
router.resources :products
router.resource :stripe_account, only: [:show, :new, :create]
router.resources :stripe_events
router.resources :tags
router.resources :tax_rates
router.resources :vendor_representatives
router.resources :payment_settings, only: [:index]
Expand Down
14 changes: 14 additions & 0 deletions app/furniture/marketplace/tag.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
class Marketplace
class Tag < Record
Copy link
Member

Choose a reason for hiding this comment

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

Not a blocker, but one way to help users keep their tags tidy would be to add a case-insensitive uniqueness constraint here, e.g:

validates :label, uniqueness: true, scope: { :bazaar, case_insensitive: true }

This would prevent adding "Gluten-Free" if "gluten-free" already exists.

(Also a good excuse to start a smol tag_spec.rb...)

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 like enforcing a case insensitive uniqueness scoped to bazaar!

self.table_name = "marketplace_tags"

belongs_to :bazaar, inverse_of: :tags
has_many :product_tags, inverse_of: :tag, dependent: :destroy
has_many :products, through: :product_tags, inverse_of: :tags

validates :label, uniqueness: {case_sensitive: false, scope: :bazaar_id}

attr_accessor :marketplace
location(parent: :marketplace)
end
end
32 changes: 32 additions & 0 deletions app/furniture/marketplace/tag_policy.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
# frozen_string_literal: true

class Marketplace
class TagPolicy < Policy
alias_method :tag, :object
def space
tag.bazaar
end

def permitted_attributes(_params = nil)
%i[label]
end

def update?
return false unless current_person.authenticated?

super
end

alias_method :create?, :update?

def show?
true
end

class Scope < ApplicationScope
def resolve
scope.all
end
end
end
end
6 changes: 6 additions & 0 deletions app/furniture/marketplace/tags/_form.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
<%= form_with(model: tag.location) do |form| %>

<%= render "text_field", attribute: :label, form: form %>

<%= form.submit %>
<%- end %>
20 changes: 20 additions & 0 deletions app/furniture/marketplace/tags/index.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
<%- breadcrumb :marketplace_tags, marketplace %>

<%= render CardComponent.new do |card| %>

<%- marketplace.tags.each do |tag| %>
<%- tag.marketplace = marketplace %>
<div id="<%= dom_id(tag)%>" class="flex flex-row gap-3">
<span class="flex-grow">
<%= tag.label %>
</span>
</div>
<%- end %>

<%- card.with_footer(variant: :action_bar) do %>
<%- new_tag = bazaar.tags.new %>
<%- if policy(new_tag).create? %>
<%= link_to t("marketplace.tags.new.link_to"), marketplace.location(:new, child: :tag), class: "button w-full" %>
<%- end %>
<%- end %>
<%- end %>
2 changes: 2 additions & 0 deletions app/furniture/marketplace/tags/new.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
<%- breadcrumb :new_marketplace_tag, mtag %>
<%= render "form", tag: mtag %>
31 changes: 31 additions & 0 deletions app/furniture/marketplace/tags_controller.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
# frozen_string_literal: true

class Marketplace
class TagsController < Controller
# Apparently, `tag` is used inside of `turbo_frame_tag`, so if we define a
# helper_method named `tag` our method gets called when it really shouldn't
# be... so `mtag` it is. For now.
expose :mtag, scope: -> { tags }, model: Tag
expose :tags, -> { policy_scope(bazaar.tags.create_with(marketplace: marketplace)) }

def new
authorize(mtag)
end

def create
if authorize(mtag).save
redirect_to marketplace.location(child: :tags)
else
render :new
end
end

def index
skip_authorization
end

def mtag_params
policy(Tag).permit(params.require(:tag))
end
end
end
17 changes: 17 additions & 0 deletions db/migrate/20240208025354_create_marketplace_product_tags.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
class CreateMarketplaceProductTags < ActiveRecord::Migration[7.1]
def change
create_table :marketplace_tags, id: :uuid do |t|
t.references :bazaar, type: :uuid
t.string :label

t.timestamps
end

create_table :marketplace_product_tags, id: :uuid do |t|
t.references :product, type: :uuid, foreign_key: {to_table: :marketplace_products}
t.references :tag, type: :uuid, foreign_key: {to_table: :marketplace_tags}

t.timestamps
end
end
end
19 changes: 19 additions & 0 deletions db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,15 @@
t.index ["shopper_id"], name: "index_marketplace_orders_on_shopper_id"
end

create_table "marketplace_product_tags", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t|
t.uuid "product_id"
t.uuid "tag_id"
t.datetime "created_at", null: false
t.datetime "updated_at", null: false
t.index ["product_id"], name: "index_marketplace_product_tags_on_product_id"
t.index ["tag_id"], name: "index_marketplace_product_tags_on_tag_id"
end

create_table "marketplace_product_tax_rates", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t|
t.uuid "tax_rate_id"
t.uuid "product_id"
Expand Down Expand Up @@ -231,6 +240,14 @@
t.index ["person_id"], name: "index_marketplace_shoppers_on_person_id", unique: true
end

create_table "marketplace_tags", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t|
t.uuid "bazaar_id"
t.string "label"
t.datetime "created_at", null: false
t.datetime "updated_at", null: false
t.index ["bazaar_id"], name: "index_marketplace_tags_on_bazaar_id"
end

create_table "marketplace_tax_rates", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t|
t.float "tax_rate"
t.string "label"
Expand Down Expand Up @@ -337,6 +354,8 @@
add_foreign_key "marketplace_notification_methods", "furnitures", column: "marketplace_id"
add_foreign_key "marketplace_orders", "marketplace_delivery_areas", column: "delivery_area_id"
add_foreign_key "marketplace_orders", "marketplace_shoppers", column: "shopper_id"
add_foreign_key "marketplace_product_tags", "marketplace_products", column: "product_id"
add_foreign_key "marketplace_product_tags", "marketplace_tags", column: "tag_id"
add_foreign_key "marketplace_product_tax_rates", "marketplace_products", column: "product_id"
add_foreign_key "marketplace_product_tax_rates", "marketplace_tax_rates", column: "tax_rate_id"
add_foreign_key "marketplace_products", "furnitures", column: "marketplace_id"
Expand Down
49 changes: 49 additions & 0 deletions spec/furniture/marketplace/product_tags_system_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
require "rails_helper"

describe "Product Tags", type: :system do
let(:space) { create(:space, :with_entrance, :with_members) }
let(:marketplace) { create(:marketplace, :ready_for_shopping, room: space.entrance) }
Copy link
Member Author

Choose a reason for hiding this comment

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

These two lines are repeated in pretty much every marketplace/*_system_spec.rb; maybe we pull them to a mixin?

Copy link
Member

Choose a reason for hiding this comment

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

I don't have a strong opinion either way.

For tests like these, I don't mind duplicating a couple of lines, and I like seeing at a glance how data is being setup for the test. An intermediate solution would be to have a trait like :with_full_space on the Marketplace factory that creates a Marketplace with a fully-setup and populated space.

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 like the solution of a marketplace trait for making instantiating a marketplace in a space with an entrance + members.


before do
sign_in(space.members.first, space)
end

scenario "Adding Tags to a Product" do # rubocop:disable RSpec/Capybara/FeatureMethods,RSpec/ExampleLength
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 think we should disable both of these cops in system specs. Thoughts?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, that makes sense.

muffins = create(:marketplace_product, marketplace:, name: "Mazin' Muffins", description: "Buttery corn muffins")

visit(marketplace)
click_link("Tags")

click_link("Add Tag")

fill_in("Label", with: "🚫🌾 Gluten Free")

click_button("Create")

click_link("Products")
within(muffins) do
click_link("⚙️ Edit")
end

check("🚫🌾 Gluten Free")
click_button("Save")

visit(marketplace)

within(muffins) do
expect(page).to have_content("🚫🌾 Gluten Free")
end
end

def visit(object_or_path)
if object_or_path.respond_to?(:location)
super(polymorphic_path(object_or_path.location))
else
super
end
Copy link
Member Author

Choose a reason for hiding this comment

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

Technically we don't need this else in this case; since everything that is visited has a location but oh well.

end

def within(model, *, **, &block)
page.within("##{dom_id(model)}", *, **, &block)
end
end
5 changes: 5 additions & 0 deletions spec/furniture/marketplace/tag_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
require "rails_helper"

RSpec.describe Marketplace::Tag, type: :model do
it { is_expected.to validate_uniqueness_of(:label).case_insensitive.scoped_to(:bazaar_id) }
end
Loading