Support limiting options for product #43

sldblog · 2023-03-13T13:18:11Z

Extends the type: Product schema to allow a new attribute, only.

We want to allow creators to choose the relevant types of products for blocks using this type.

Example:

Experiences:
  group: content
  label: Select featured experiences
  type: Product
  array: true
  default:
    - random
    - random
    - random
  only:
    - experiences

sldblog · 2023-03-13T13:21:29Z

Enable specifying accommodations vs experiences on product and variant variables

sldblog · 2023-03-13T13:29:48Z

❓ I decided not to validate the values as if we need to expand that, that would require a schema change. Semantically, however, it can result in excluding everything if there's a typo like expereince. What would you suggest?

eballantine · 2023-03-13T13:46:35Z

❓ I decided not to validate the values as if we need to expand that, that would require a schema change. Semantically, however, it can result in excluding everything if there's a typo like expereince. What would you suggest?

So this canvas gem is used for theme developers to lint the theme before making changes, and therefore personally I think it should validate the exact values of 'experience' or 'accommodation' so they're not committing code that won't work.

I've just added another example to the Trello ticket for how this should also be an option on the variant type. But appreciate if you want to tackle that separately.

I'm also just going to say 'extras' as I'm not sure if we've fully considered them for this work. We maybe we need to have refinement of the ticket to be clear what's in scope, let me know when you're free to discuss cc:@ianmooney

eballantine

Hey @sldblog thanks for the rubocop clearup along the way.

I wasn't sure how to go about testing this and either I still haven't figured it out or, this is not behaving as we'd like it to. Please correct my approach:

I've checked out this branch
Run canvas lint inside a local version of a theme (alchemist in my case).

If I update a block schema to use only in the way we've outlined, then I get the below failure.

sldblog · 2023-03-13T14:51:37Z

Thanks @eballantine, I will add the choice validation here. I'm unsure what the options would be for variants, can we catch up offline about that, please?

ianmooney

Nice one, David. I just left a couple comments in there.

ianmooney · 2023-03-13T15:34:50Z

lib/canvas/validators/schema_attributes/product.rb

        end

+        def optional_keys
+          super.merge("only_show" => Array)


Now I'm looking at it, I wonder if only reads better than only_show.

What do you think?

I think that we should validate that the values passed to the only_show option are from an allowed list of options (experiences, accommodations and extras). I think these should be lower case but I don't have a strong opinion whether they should be pluralised or not.

This is because this list of allowed options is unlikely to change and if it did it would warrant a update to this canvas gem.

done in 5ec1e53

Warning: obsolete parameter `IgnoredMethods` (for `Style/BlockDelimiters`) found in .rubocop.yml `IgnoredMethods` has been renamed to `AllowedMethods` and/or `AllowedPatterns`. Warning: Style/BlockDelimiters does not support IgnoredMethods parameter.

The new attribute is 1. optional 2. has to be a non-empty array The intent is to provide a way for creators and theme developers to limit the available options to either experiences or accommodation, as appropriate. I decided not to validate the allowed values as I thought that would be too restrictive in the schema.

sldblog · 2023-03-13T18:54:21Z

@ianmooney @eballantine 👍 Thank you for your reviews!
Renamed to only in 7c36d16 and added the restrictions in 5ec1e53

To avoid accidental typos and misuse in the user input

(This is a new, non-breaking feature)

README.md

It's generally quicker than installing the gem, but installing the gem is better for final testing

ianmooney

Works great 🚀

ianmooney · 2023-03-14T12:52:28Z

README.md

+```
+cd alchemist_theme
+../canvas/bin/canvas lint
+```


Nice one for updating these docs with a quicker way to test things 🚀

sldblog · 2023-03-14T13:41:35Z

🎩 Thank you!

sldblog requested review from a team, eballantine and ianmooney and removed request for a team March 13, 2023 13:18

eballantine self-assigned this Mar 13, 2023

eballantine reviewed Mar 13, 2023

View reviewed changes

ianmooney reviewed Mar 13, 2023

View reviewed changes

sldblog added 2 commits March 13, 2023 18:37

Fix Rubocop deprecation warning

f193c6d

Warning: obsolete parameter `IgnoredMethods` (for `Style/BlockDelimiters`) found in .rubocop.yml `IgnoredMethods` has been renamed to `AllowedMethods` and/or `AllowedPatterns`. Warning: Style/BlockDelimiters does not support IgnoredMethods parameter.

sldblog force-pushed the support-limiting-options-for-product branch 2 times, most recently from b0c8634 to ea407d4 Compare March 13, 2023 18:53

sldblog requested review from eballantine and ianmooney March 14, 2023 11:21

sldblog force-pushed the support-limiting-options-for-product branch from 0e28164 to db14753 Compare March 14, 2023 11:47

sldblog added 2 commits March 14, 2023 11:48

Restrict values to known settings

12faeaa

To avoid accidental typos and misuse in the user input

Bump minor version to 3.3.0

6d82e5a

(This is a new, non-breaking feature)

sldblog force-pushed the support-limiting-options-for-product branch from db14753 to f4e5214 Compare March 14, 2023 11:48

CatalinaTudor reviewed Mar 14, 2023

View reviewed changes

README.md Show resolved Hide resolved

sldblog added 2 commits March 14, 2023 12:17

Add instructions for local testing via bin script

70e1e37

It's generally quicker than installing the gem, but installing the gem is better for final testing

Use percent literals instead of quoting strings

e5ab567

sldblog force-pushed the support-limiting-options-for-product branch from f4e5214 to e5ab567 Compare March 14, 2023 12:17

ianmooney self-assigned this Mar 14, 2023

ianmooney approved these changes Mar 14, 2023

View reviewed changes

sldblog merged commit 7e6f65f into main Mar 14, 2023

sldblog deleted the support-limiting-options-for-product branch March 14, 2023 13:41

This was referenced Mar 16, 2023

Remove "extras" from product attribute restrictions #45

Merged

Support limiting product options for variant type #46

Merged

Expose allowed attribute keys #48

Closed

Support limiting options for product #43

Support limiting options for product #43

Uh oh!

Conversation

sldblog commented Mar 13, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sldblog commented Mar 13, 2023

Uh oh!

sldblog commented Mar 13, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

eballantine commented Mar 13, 2023

Uh oh!

eballantine left a comment

Choose a reason for hiding this comment

Uh oh!

sldblog commented Mar 13, 2023

Uh oh!

ianmooney left a comment

Choose a reason for hiding this comment

Uh oh!

ianmooney Mar 13, 2023

Choose a reason for hiding this comment

Uh oh!

ianmooney Mar 13, 2023

Choose a reason for hiding this comment

Uh oh!

sldblog Mar 13, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sldblog commented Mar 13, 2023

Uh oh!

Uh oh!

ianmooney left a comment

Choose a reason for hiding this comment

Uh oh!

ianmooney Mar 14, 2023

Choose a reason for hiding this comment

Uh oh!

sldblog commented Mar 14, 2023

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

5 participants

sldblog commented Mar 13, 2023 •

edited

Loading

sldblog commented Mar 13, 2023 •

edited

Loading

sldblog Mar 13, 2023 •

edited

Loading