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

[PLUGIN1CC-4024] Magic buy now logic update based on config #514

Open
wants to merge 2 commits into
base: magic_integration_latest
Choose a base branch
from

Conversation

ChetanGN
Copy link
Contributor

Note :- Please follow the below points while attaching test cases document link below:

- If label Tested is added then test cases document URL is mandatory.

- Link added should be a valid URL and accessible throughout the org.

- If the branch name contains hotfix / revert by default the BVT workflow check will pass.

Test Case Document URL
Please paste test case document link here....

@ChetanGN ChetanGN changed the title Magic buy now logic update based on config [PLUGIN1CC-4024] Magic buy now logic update based on config Dec 12, 2024
@ChetanGN ChetanGN added tested TestingNotRequired TestingNotRequired label for BVT and removed tested labels Dec 12, 2024
Copy link

@lbajsarowicz lbajsarowicz 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 noticed a few significant issues to address.

Comment on lines +184 to +189
$buyNowCartPushAction = $this->config->getBuyNowAction();
if ($buyNowCartPushAction === '1') {
$quote = $quoteBuilder->createOrUpdateQuote();
} else {
$quote = $quoteBuilder->createQuote();
}

Choose a reason for hiding this comment

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

This can be simplified to:

Suggested change
$buyNowCartPushAction = $this->config->getBuyNowAction();
if ($buyNowCartPushAction === '1') {
$quote = $quoteBuilder->createOrUpdateQuote();
} else {
$quote = $quoteBuilder->createQuote();
}
$quote = $this->config->getBuyNowAction()
? $quoteBuilder->createOrUpdateQuote()
: $quoteBuilder->createQuote();

@@ -231,10 +236,10 @@ public function execute()
$productImageUrl = $store->getBaseUrl(\Magento\Framework\UrlInterface::URL_TYPE_MEDIA) . 'catalog/product' . $product->getImage();
}

$imagewidth=200;
$imageheight=200;
$imagewidth = 200;

Choose a reason for hiding this comment

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

It's not a part of your change, but long-term if we really need to use Magento\Catalog\Helper\Image, I'd rather use Magento\Catalog\Helper\ImageFactory than ObjectManager.

ItemBuilderFactory $itemBuilderFactory
) {
Session $session,
CheckoutSession $checkoutSession,

Choose a reason for hiding this comment

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

Every single time you introduce Session or CheckoutSession (or Backend Session) to the class, make sure that in corresponding di.xml there's a \Proxy passed through the argument, to make sure that session is not tried to be initialized in CLI or Cron context.

In order to address this requirement, open di.xml and add:

<type name="Razorpay\Magento\Model\QuoteBuilder">
    <arguments>
        <argument name="session" xsi:type="object">Magento\Customer\Model\Session\Proxy</argument>
        <argument name="checkoutSession" xsi:type="object">Magento\Checkout\Model\Session\Proxy</argument>
    </arguments>
</type>

https://developer.adobe.com/commerce/php/development/components/proxies/


return $quote;
}

}

Choose a reason for hiding this comment

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

We work in *nix context, make sure to keep empty line at the end of file.

@@ -124,7 +124,19 @@
</depends>
</field>

<field id="activate_magic_mini_cart" translate="label" type="select" sortOrder="27" showInDefault="0" showInWebsite="0" showInStore="0">
<field id="magic_buy_now_cart_push" translate="label" type="select" sortOrder="27" showInDefault="1" showInWebsite="1" showInStore="1">
<label>Update Product to Existing Cart on Buy Now </label>

Choose a reason for hiding this comment

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

Suggested change
<label>Update Product to Existing Cart on Buy Now </label>
<label>Update Product to Existing Cart on Buy Now</label>

</field>


<field id="activate_magic_mini_cart" translate="label" type="select" sortOrder="28" showInDefault="0" showInWebsite="0" showInStore="0">

Choose a reason for hiding this comment

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

When one of these values is 0 then the attribute is redundant.

Suggested change
<field id="activate_magic_mini_cart" translate="label" type="select" sortOrder="28" showInDefault="0" showInWebsite="0" showInStore="0">
<field id="activate_magic_mini_cart" translate="label" type="select" sortOrder="28">

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
TestingNotRequired TestingNotRequired label for BVT
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants