Skip to content

Conversation

@sreichel
Copy link
Contributor

@sreichel sreichel commented Nov 8, 2025

In Mage_Catalog_Model_Product_Image::setSize($size) the $size parameter had to be a string value, seperated by a x for width and heigth.

Everything else leads to undifined offset 1 error.

Also, for phpstan (or later strict_types) extracted dimensions had to be integer values, no strings.

Refactored setSize() to deal with different values.

  • 100x100 (default required pattern)
  • 100x and x100 for setting one dimension
  • 100 for both dimensions
  • added test

Copilot AI review requested due to automatic review settings November 8, 2025 00:46
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors the setSize() method in the Mage_Catalog_Model_Product_Image class to handle edge cases better and adds comprehensive unit tests. The refactored logic handles empty strings, zero values, and various dimension formats more consistently.

  • Improved the setSize() method to handle edge cases like empty strings, "0x", "x0", and non-numeric values
  • Added unit tests with comprehensive test coverage for different size format inputs
  • Fixed PHPStan type errors by updating parameter type hints

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.

File Description
tests/unit/Traits/DataProvider/Mage/Catalog/Model/Product/ImageTrait.php New trait providing test data for various size format inputs
tests/unit/Mage/Catalog/Model/Product/ImageTest.php New unit test file testing the setSize() method with data provider
app/code/core/Mage/Catalog/Model/Product/Image.php Refactored setSize() method with improved edge case handling and updated type hints
.phpstan.dist.baseline.neon Removed 3 PHPStan errors that are now fixed

@sonarqubecloud
Copy link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants