Skip to content

Conversation

sreichel
Copy link
Contributor

@sreichel sreichel commented Mar 14, 2025

Description (*)

Allow price rounding up to 4 digits.

Used that over years for a shop that wanted to display prices incl/excl per storewiew and we run into multiple rounding issues. That solved it for our cases.

Related Issues

Did not test ...

@github-actions github-actions bot added Component: Core Relates to Mage_Core Component: Catalog Relates to Mage_Catalog Component: Adminhtml Relates to Mage_Adminhtml labels Mar 14, 2025
kiatng
kiatng previously approved these changes Mar 17, 2025
Copy link
Contributor

@kiatng kiatng left a comment

Choose a reason for hiding this comment

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

LGTM

@Axepih
Copy link

Axepih commented Mar 23, 2025

just a typo on the label

Catalog Price Rounding Crecision

kiatng
kiatng previously approved these changes Mar 24, 2025
@kiatng
Copy link
Contributor

kiatng commented Mar 24, 2025

just a typo on the label

Catalog Price Rounding Crecision

Thanks for the review. However, it's much better if you had added a review comment, in this way, your contribution would have been logged in Git when your suggestion is committed.

@Axepih Please help to approve the PR after your review. We will acknowledge you as a contributor.

@kiatng
Copy link
Contributor

kiatng commented Mar 26, 2025

@all-contributors please add @Axepih for review

Copy link
Contributor

@kiatng

I've put up a pull request to add @Axepih! 🎉

Copy link

@sreichel sreichel marked this pull request as draft April 20, 2025 07:28
@sreichel sreichel closed this Apr 23, 2025
@sreichel sreichel reopened this Oct 11, 2025
@sreichel sreichel marked this pull request as ready for review October 11, 2025 20:08
@Copilot Copilot AI review requested due to automatic review settings October 11, 2025 20:08
Copy link
Contributor

@Copilot 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 pull request enables configurable price rounding precision between 0 and 4 decimal places instead of the hardcoded 2 decimal places. This addresses rounding issues that can occur when displaying prices with different precision requirements across store views.

Key changes:

  • Introduces a new system configuration option for catalog price rounding precision
  • Replaces hardcoded 2-decimal rounding with configurable precision
  • Adds helper class to manage rounding precision settings

Reviewed Changes

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

Show a summary per file
File Description
app/code/core/Mage/Core/Model/Store.php Updates roundPrice() method to use configurable precision instead of hardcoded 2 decimals
app/code/core/Mage/Catalog/etc/system.xml Adds new system configuration field for rounding precision with validation
app/code/core/Mage/Catalog/etc/config.xml Sets default rounding precision value to 2 in configuration
app/code/core/Mage/Catalog/Helper/Price.php New helper class that manages rounding precision configuration and validation
app/code/core/Mage/Adminhtml/Block/Catalog/Product/Helper/Form/Price.php Updates price formatting in admin forms to use configurable precision

@sreichel sreichel requested review from Axepih, addison74 and kiatng and removed request for Axepih October 11, 2025 20:14
kiatng
kiatng previously approved these changes Oct 14, 2025
@sreichel
Copy link
Contributor Author

Merged with one green. In production for years.

@sreichel sreichel merged commit b439e2f into OpenMage:main Oct 14, 2025
20 checks passed
@sreichel sreichel deleted the price-rounding branch October 14, 2025 04:50
Copy link

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

Labels

Component: Adminhtml Relates to Mage_Adminhtml Component: Catalog Relates to Mage_Catalog Component: Core Relates to Mage_Core new feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants