Skip to content
This repository has been archived by the owner on Nov 14, 2023. It is now read-only.

track sold out products INREL-3676 #101

Open
wants to merge 1 commit into
base: 8.x-3.x-dev
Choose a base branch
from

Conversation

mgoedecke
Copy link
Contributor

No description provided.

@@ -197,7 +197,8 @@ function infinite_preprocess_advertising_product(&$variables) {
->setAttribute('data-price', $product->product_price->value)
->setAttribute('data-currency', $product->product_currency->value)
->setAttribute('data-uuid', $product->uuid->value)
->setAttribute('data-product-id', $product->product_id->value);
->setAttribute('data-product-id', $product->product_id->value)
->setAttribute('data-sold-out', (string)$product->product_sold_out->value === "1" ? 'true' : 'false');
Copy link
Contributor

Choose a reason for hiding this comment

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

How about we leave it clean without transforming the data? Something like: ->setAttribute('data-sold-out', (string)$product->product_sold_out->value

@@ -9,11 +9,6 @@
initialize: function (pOptions) {
BaseView.prototype.initialize.call(this, pOptions);

if (this.$el.hasClass('item-product--sold-out')) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure why this has to be removed. Can you clarify?

@@ -198,7 +193,8 @@
brand: this.model.get('brand'),
provider: this.model.get('provider'),
productCategory: this.model.get('productCategory'),
containerType: this.model.get('containerType') || ''
containerType: this.model.get('containerType') || '',
soldOut: this.$el.attr('data-sold-out') === 'true'
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of getting the data here we could set the data on the model directly (and also transform data there):
this.model.set('sold-out', this.$el.data('sold-out') === 1 ? 'true' : 'false'). See Line 85.

Copy link
Contributor

Choose a reason for hiding this comment

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

Afterwards it's just a simple soldOut: this.model.get('sold-out')

Copy link
Contributor

@saviomuc saviomuc left a comment

Choose a reason for hiding this comment

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

I have some small suggestions. Please clarify what value we need to pass: Number (1,0), Boolean or String.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants