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

sync to Yith products to CTB #52

Merged
merged 4 commits into from
Feb 27, 2024
Merged

sync to Yith products to CTB #52

merged 4 commits into from
Feb 27, 2024

Conversation

manikantakailasa
Copy link
Contributor

@manikantakailasa manikantakailasa commented Feb 15, 2024

Currently in the branded plugin, we are redirecting customers to the YITH plugin pages to purchase premium plugins. We need to use the YITH CTBs instead.
https://confluence.newfold.com/display/SF/Yith+Plugins

story : https://jira.newfold.com/browse/PRESS4-462

@manikantakailasa manikantakailasa requested a review from a team February 21, 2024 06:55
}

// Show the filtered products on the page
showProducts(products) {
Copy link
Member

Choose a reason for hiding this comment

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

This supportsCTB shouldn't be needed. We can safely add the CTB attributes and the ctb module will detect whether it is supported or not.

FYI, there is a legacy ctb module and a newer global ctb module. They both will detect the same attributes and which one (if any) is supported.

href="${product.primaryUrl}"
class="button button-primary nfd-ctb-btn"
${supportsCTB && product.clickToBuyId ? 'data-action="load-nfd-ctb" data-ctb-id="' + product.clickToBuyId + '"' : ''}>Buy $${product.price}</a>`;
${
supportsCTB && product?.clickToBuyId
Copy link
Member

Choose a reason for hiding this comment

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

Here we can safely add the ctb attributes and don't need to check the supportsCTB value.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Copy link
Member

@circlecube circlecube left a comment

Choose a reason for hiding this comment

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

This looks like just formatting changes now that the Yith IDs are not being included and mapped here.

The change here is that it will always add the ctb attributes to the products (without checking if the site supports CTB, which is what we want since the ctb modules manage that logic anyways), so we should include this, and it will add the attrs to all these products (YITH and others) properly all the time. The issue I believe was that this code checked if legacy ctb was supported, and that is being replaced for the most part by new global ctb - which this code didn't get updated to account for that.

@circlecube circlecube merged commit 3b1864b into main Feb 27, 2024
5 of 6 checks passed
@circlecube circlecube deleted the PRESS4-462 branch February 27, 2024 17:01
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.

4 participants