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

feat: better cart and order item attributes, with labels #5253

Merged
merged 5 commits into from
Jul 1, 2019

Conversation

aldeed
Copy link
Contributor

@aldeed aldeed commented Jun 29, 2019

Resolves #5208
Impact: minor
Type: feature

Issue

CartItem and OrderItem have attributes array, but label was null and value was pulled from the wrong product field. OrderItem always had empty array.

One underlying problem is that we had no label for variants, which is why we have null attribute labels.

A related problem is general confusion around title vs. optionTitle fields for variants.

Solution

  • Added optional attributeLabel string field to Products of type "variant"
  • Added required attributeLabel string field to all CatalogProductVariant
  • Added an operator UI field to the variant edit form, "Attribute label". Also generally improved the labels, placeholder text, and help text for the attributeLabel, optionTitle, and title fields on that form, to help avoid more confusion.
  • Properly set attributes array on OrderItems when they are created
  • Properly set attributes array on CartItems when they are created
  • Migration to set attributeLabel to a default value in Products and Catalog collections. For top-level variants, set to "Variant". For option variants, set to "Option".
  • Migration to set label for all attributes on all CartItem to a default value in Cart collection. For first attribute, set to "Variant". For second attribute, if present, set to "Option".

NOTE: I opted not to do #5206 in this PR to avoid it becoming too large.

Breaking changes

As long as migrations run, nothing should break. If client code is using cart item attributes and depending on what the value of them currently is (the full title), it may need to be updated.

Testing

  1. Test the new and updated fields at the top of the variant edit form. Test for top-level variant and for option variant. Verify you cannot publish without an attribute label set.
  2. Verify you can query GraphQL for attributeLabel field for any variant or option in a CatalogItemProduct.
  3. Test the migration and verify that all data is migrated as described.

Required to publish to catalog.
Use this to set cart item attribute labels.

Signed-off-by: Eric Dobbertin <[email protected]>
@aldeed aldeed self-assigned this Jun 29, 2019
@aldeed aldeed marked this pull request as ready for review June 30, 2019 14:54
Copy link
Member

@mikemurray mikemurray left a comment

Choose a reason for hiding this comment

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

👍

Copy link
Member

@willopez willopez left a comment

Choose a reason for hiding this comment

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

👍

@willopez willopez merged commit ac71059 into develop Jul 1, 2019
@willopez willopez deleted the fix-cart-item-attributes branch July 1, 2019 22:04
@jeffcorpuz jeffcorpuz mentioned this pull request Jul 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants