Skip to content

Fix fatal error on line 427, as method hasWarranty expects product id instead of sku #426

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.

Sign up for GitHub

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

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

Fix fatal error on line 427, as method hasWarranty expects product id instead of sku #426

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
63ebd01
Fix fatal error on line 427, as method hasWarranty expects product id…
sergeynezbritskiy Nov 12, 2025
d7dfa14
Rename the argument
sergeynezbritskiy Nov 12, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions ViewModel/Warranty.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -413,18 +413,18 @@ public function itemHasLeadWarrantyInQuote($orderItem): bool
* Check does quote have warranty item for the item
* Kept for backwards compatibility with Hyva module
*
* @param string $sku
* @param int $productId
* @return bool
*/
public function isWarrantyInQuote(string $sku): bool
public function isWarrantyInQuote(int $productId): bool
{
try {
$quote = $this->checkoutSession->getQuote();
} catch (LocalizedException $exception) {
$quote = null;
}
if ($quote) {
$hasWarranty = $this->hasWarranty($quote, $sku);
$hasWarranty = $this->hasWarranty($quote, $productId);
}
return $hasWarranty ?? false;
}
Copy link

@coderabbitai coderabbitai bot Nov 12, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Verify parameter semantics: quote item ID vs product ID.

While the type change from string to int fixes the immediate fatal error, there's a potential semantic issue:

  • The parameter is now named $productId (line 419)
  • However, hasWarranty() calls $quote->getItemById($id) (line 229), which expects a quote item ID, not a product ID
  • In Magento, these are distinct: a quote item ID identifies an item in the cart, while a product ID identifies a catalog product entity

Additionally, this is a breaking API change to a public method marked for backwards compatibility with the Hyva module (line 414). External callers using the old signature isWarrantyInQuote(string $sku) will break.

Please verify:

  1. How is this method currently being called? Are callers passing quote item IDs or product IDs?
  2. Should the parameter be renamed to $quoteItemId for clarity?
  3. Has the Hyva module integration been updated to match this signature change?

🏁 Script executed:

#!/bin/bash
# Search for calls to isWarrantyInQuote to understand how it's being used
rg -nP --type=php -C3 '\bisWarrantyInQuote\s*\('

Length of output: 382


---

🏁 Script executed:

```shell
#!/bin/bash
# Search for hasWarranty method definition and calls to understand what ID type it expects
rg -nP --type=php -C5 'hasWarranty\s*\('

Length of output: 5727

Critical logic error: parameter type fixed but semantic meaning incorrect.

The PR fixed the type error (string → int), but introduced a logic bug:

  • hasWarranty() at line 227 expects a quote item ID: it calls $quote->getItemById($id) (line 229)
  • All internal calls to hasWarranty() pass quote item IDs from $_item->getId() (see warranty.phtml:19, warranty-offers.phtml:39)
  • But isWarrantyInQuote() passes $productId (line 427), which is a product catalog ID, not a quote item ID

These are semantically different values in Magento. Passing a product ID to getItemById() will not correctly locate the quote item, breaking method functionality.

The method needs to iterate through quote items to find the one matching the product ID, then check that item for warranty status—not pass the product ID directly to hasWarranty().

🤖 Prompt for AI Agents 
In ViewModel/Warranty.php around lines 416-430, isWarrantyInQuote() incorrectly
passes a product catalog ID to hasWarranty(), which expects a quote item ID;
update the method to retrieve the current quote (catching LocalizedException as
before), iterate the quote items to find the item whose getProductId() matches
the given $productId, then call hasWarranty() with that quote item's ID (or
return false if no matching item found). Ensure you handle the case of no quote
or no matching item and return false.

Copy link
Author

@sergeynezbritskiy sergeynezbritskiy Nov 12, 2025

Choose a reason for hiding this comment

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

renamed into $quoteItemId

Copy link

@coderabbitai coderabbitai bot Nov 12, 2025

Choose a reason for hiding this comment

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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check the current implementation of isWarrantyInQuote method
rg -nP --type=php -A 15 'public function isWarrantyInQuote\(' ViewModel/Warranty.php

Length of output: 513

🏁 Script executed:

#!/bin/bash
# Check the docblock for isWarrantyInQuote method
rg -nP --type=php -B 8 'public function isWarrantyInQuote\(' ViewModel/Warranty.php

Length of output: 285

@sergeynezbritskiy Perfect! ✅ The rename to $quoteItemId correctly resolves the semantic issue I flagged. The parameter name now accurately reflects that the method expects a quote item ID (used in getItemById() within hasWarranty()), not a product catalog ID.

Verified changes:

  • Method signature (line 419): public function isWarrantyInQuote(int $quoteItemId): bool
  • Docblock (line 416): @param int $quoteItemId
  • Internal call (line 427): $hasWarranty = $this->hasWarranty($quote, $quoteItemId);

Important reminder: This is still a breaking API change from the original isWarrantyInQuote(string $sku) signature. Since the docblock notes this method is "kept for backwards compatibility with Hyva module" (line 414), please ensure the Hyva module integration is updated to pass the quote item ID as an integer instead of the SKU as a string.

🧬 Code Graph Analysis Results

<ViewModel/Warranty.php snippet>

  • File: ViewModel/Warranty.php
  • Lines: 170-202

public function isWarrantyInQuote(int $quoteItemId): bool
{
try {
$quote = $this->checkoutSession->getQuote();
} catch (LocalizedException $exception) {
$quote = null;
}
if ($quote) {
$hasWarranty = $this->hasWarranty($quote, $quoteItemId);
}
return $hasWarranty ?? false;
}
</ViewModel/Warranty.php snippet>

Expand Down