✨ `Marketplace`: Add `Product#tags` #2190

zspencer · 2024-02-08T16:42:18Z

🥡 Marketplace: Product#tags #2189

Products can now be tagged with things like Gluten Free or Vegan,
and will show up as such under Product listings

After

app/furniture/marketplace/management_component.html.erb

zspencer · 2024-02-08T16:44:11Z

app/furniture/marketplace/policy.rb

@@ -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)


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

zspencer · 2024-02-08T16:46:12Z

spec/furniture/marketplace/product_tags_system_spec.rb

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


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

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.

I like the solution of a marketplace trait for making instantiating a marketplace in a space with an entrance + members.

zspencer · 2024-02-08T16:46:26Z

spec/furniture/marketplace/product_tags_system_spec.rb

+    sign_in(space.members.first, space)
+  end
+
+  scenario "Adding Tags to a Product" do # rubocop:disable RSpec/Capybara/FeatureMethods,RSpec/ExampleLength


I think we should disable both of these cops in system specs. Thoughts?

Yeah, that makes sense.

zspencer · 2024-02-08T16:47:31Z

spec/furniture/marketplace/product_tags_system_spec.rb

+      super(polymorphic_path(object_or_path.location))
+    else
+      super
+    end


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

zspencer · 2024-02-08T17:01:34Z

.gitignore

@@ -33,4 +33,4 @@ app/assets/builds/

 .devcontainer/output

-TAGS


This was making us ignore the tags views folder 🤦

TIL .gitignore will look at any directories with a declaration made at the top level 😳

rosschapman · 2024-02-20T23:37:33Z

@zspencer can you provide some justification for implementing tags using a relational architecture through a bridge table vs using an array column on a Product? I generally understand the tradeoffs between these two methodologies, so was hoping you could help me understand why you might prefer one way over another for Convene.

Spitballing...I can imagine a Product -> Tags relationship using an array column, that also retains referential integrity if those tags are only chosen via a select dropdown from a pre-filled Tags record table. (Presuming referential integrity being one major benefit to the relational model.) Of course, an underlying question is whether constraining the tags to a strict taxonomy might be preferable over user-generated values?

Does my question(ing) make sense?

zspencer · 2024-02-21T01:04:46Z

To be honest, I don't really think there's enough difference to encourage one or the other in this particular use case.

My main goal here was making it so Members can decide what Tags are allowed in the Marketplace context. Rails doesn't have great built-in support for searching / filtering by array fields in Postgresql; so this is the "most boring" way to go with it.

If we were to implement it as a field-on-a-row; we'd need to get into deeper arcana than I think is necessary.

zspencer · 2024-02-22T00:02:33Z

A question I think is of greater importance here is what we call these things. label and tag are both methods defined in Rails views.

So, what should we call them? Do we call them tags even though it's a name collision? In the Journal, we call them Keywords... Maybe it's actually Product#features?

zspencer · 2024-03-04T23:56:26Z

@rosschapman - I lied! I forgot that I didn't have clarity on the lesser of N evils, regarding what we call these.

label feels too generic / collides with HTML
tag feels too generic + collides with HTML
Maybe badge? or sticker? or is that too skeumorphic?

zspencer · 2024-03-05T00:00:19Z

Or maybe we just go hella specific and call them Product#dietary_restrictions for now?

@ExMember

- #2189 Lil' quick sketch through the workflow. Only interesting things of note here are I am using `scenario` syntax (which feels better to me, and was validated by @ExMember in antoher context yesterday.) I am also doing something "weird" where I overload the `visit` method to rely on our `Record#location` method to interject a `polymorphic_path` call; saving us the burden of including the `polymorphic_path` call on our own every time we use `visit`; but adding the burden of a layer of indirection. I also did this with `within`, because omg that awkward `"##{dom_id(model)"` drives me nuts every time I type it. It makes me want to scream. I can't stand it.

Products can now be tagged with things like `Gluten Free` or `Vegan`, and will show up as such under Product listings

anaulin · 2024-03-07T02:04:43Z

@zspencer can you provide some justification for implementing tags using a relational architecture through a bridge table vs using an array column on a Product? I generally understand the tradeoffs between these two methodologies, so was hoping you could help me understand why you might prefer one way over another for Convene.

The justification is that Rails people have a bizarre kink for bridge tables. 😉

(I hate bridge tables, because I'm not a Real Rails Person.)

anaulin · 2024-03-07T02:06:55Z

Or maybe we just go hella specific and call them Product#dietary_restrictions for now?

Just to record a brief summary of the convo we just had during ensemble: seems like calling them tags gives us a bit more flexibility.

anaulin

Very nice, thank you!

anaulin · 2024-03-07T02:21:32Z

app/furniture/marketplace/tag.rb

@@ -0,0 +1,12 @@
+class Marketplace
+  class Tag < Record


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...)

I like enforcing a case insensitive uniqueness scoped to bazaar!

zspencer · 2024-03-07T02:27:55Z

The justification is that Rails people have a bizarre kink for bridge tables. 😉

Is it a kink if it's inflicted against our will through trauma... oh wait.

@ExMember

- zinc-collective#2189 Lil' quick sketch through the workflow. Only interesting things of note here are I am using `scenario` syntax (which feels better to me, and was validated by @ExMember in antoher context yesterday.) I am also doing something "weird" where I overload the `visit` method to rely on our `Record#location` method to interject a `polymorphic_path` call; saving us the burden of including the `polymorphic_path` call on our own every time we use `visit`; but adding the burden of a layer of indirection. I also did this with `within`, because omg that awkward `"##{dom_id(model)"` drives me nuts every time I type it. It makes me want to scream. I can't stand it. * ✨ `Marketplace`: Adds `Product#tags` Products can now be tagged with things like `Gluten Free` or `Vegan`, and will show up as such under Product listings * ✨ `Tag` labels are case insensitive

zspencer requested review from a team February 8, 2024 16:42

zspencer added ✨ feature Reduces Client's Burden or Grants them Benefits 🥗 test automation Adds some automated tests. V nutritious. labels Feb 8, 2024

zspencer commented Feb 8, 2024

View reviewed changes

zspencer force-pushed the marketplace/products-can-be-tagged branch from 23f6ef5 to 115a346 Compare February 8, 2024 16:50

zspencer commented Feb 8, 2024

View reviewed changes

zspencer changed the title ~~✨ Marketplace: First draft of Product#tags~~ ✨ Marketplace: First draft of Product#labels Feb 21, 2024

zspencer marked this pull request as draft February 22, 2024 00:02

zspencer changed the title ~~✨ Marketplace: First draft of Product#labels~~ ✨ Marketplace: Add Product#labels Feb 22, 2024

zspencer force-pushed the marketplace/products-can-be-tagged branch 2 times, most recently from 018ba9f to c12f3d3 Compare February 22, 2024 01:57

zspencer force-pushed the marketplace/products-can-be-tagged branch from c12f3d3 to 130e1ba Compare March 5, 2024 18:44

zspencer added 2 commits March 6, 2024 17:16

✨ Marketplace: Adds Product#tags

2c17ad2

Products can now be tagged with things like `Gluten Free` or `Vegan`, and will show up as such under Product listings

zspencer force-pushed the marketplace/products-can-be-tagged branch from 130e1ba to 2c17ad2 Compare March 7, 2024 01:16

zspencer changed the title ~~✨ Marketplace: Add Product#labels~~ ✨ Marketplace: Add Product#tags Mar 7, 2024

zspencer marked this pull request as ready for review March 7, 2024 01:51

anaulin approved these changes Mar 7, 2024

View reviewed changes

✨ Tag labels are case insensitive

d5f0cb0

zspencer merged commit 86524fe into main Mar 7, 2024
3 checks passed

zspencer deleted the marketplace/products-can-be-tagged branch March 7, 2024 02:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

✨ `Marketplace`: Add `Product#tags` #2190

✨ `Marketplace`: Add `Product#tags` #2190

zspencer commented Feb 8, 2024

zspencer Feb 8, 2024

zspencer Feb 8, 2024

anaulin Mar 7, 2024

zspencer Mar 7, 2024

zspencer Feb 8, 2024

anaulin Mar 7, 2024

zspencer Feb 8, 2024

zspencer Feb 8, 2024

rosschapman Feb 9, 2024

rosschapman commented Feb 20, 2024 •

edited

Loading

zspencer commented Feb 21, 2024

zspencer commented Feb 22, 2024

zspencer commented Mar 4, 2024

zspencer commented Mar 5, 2024

anaulin commented Mar 7, 2024 •

edited

Loading

anaulin commented Mar 7, 2024

anaulin left a comment

anaulin Mar 7, 2024

zspencer Mar 7, 2024

zspencer commented Mar 7, 2024

		@@ -33,4 +33,4 @@ app/assets/builds/

		.devcontainer/output

		TAGS

✨ Marketplace: Add Product#tags #2190

✨ Marketplace: Add Product#tags #2190

Conversation

zspencer commented Feb 8, 2024

After

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rosschapman commented Feb 20, 2024 • edited Loading

zspencer commented Feb 21, 2024

zspencer commented Feb 22, 2024

zspencer commented Mar 4, 2024

zspencer commented Mar 5, 2024

anaulin commented Mar 7, 2024 • edited Loading

anaulin commented Mar 7, 2024

anaulin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zspencer commented Mar 7, 2024

✨ `Marketplace`: Add `Product#tags` #2190

✨ `Marketplace`: Add `Product#tags` #2190

rosschapman commented Feb 20, 2024 •

edited

Loading

anaulin commented Mar 7, 2024 •

edited

Loading