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

Fix currency formatting #814

Merged
merged 5 commits into from
Jan 17, 2024
Merged

Conversation

wernerkrauss
Copy link
Contributor

somehow related to #779 some more formatting for different prices in the backend.

For formatting payments see silverstripe/silverstripe-omnipay#244

@wilr and/or @forsdahl please have a look at this PR and merge if possible. I disabled OrderModifier's Amount() method which should be fine IMHO but could possibly break some stuff.

@wilr
Copy link
Contributor

wilr commented Jan 17, 2024

All checks are green so I'm happy

@wilr wilr merged commit a4d0b02 into silvershop:3 Jan 17, 2024
9 checks passed
@chromos33
Copy link

disabling the Amount method breaks the ShowInTable() function in OrderDiscountModifier in the Discount Module.

@wernerkrauss
Copy link
Contributor Author

@chromos33 Thanks for reporting this. Can you please try changing OrderDiscountModifier's ShowInTable() method to:

    public function ShowInTable()
    {
        return $this->Amount > 0;
    }

so using the magic DB-Field value directly instead of the getter method

wernerkrauss added a commit that referenced this pull request Feb 20, 2024
* Order: helper method for nice formatting of Total in GridFields

See #811

Can be removed if Total() works like other DB Fields

* OrderModifier: disable Amount() method

See #811

* OrderModifier: show short class name in summary fields

* shop reports: format currency values as currency

* fix linting errors
wernerkrauss added a commit to silvershop/silvershop-discounts that referenced this pull request Feb 26, 2024
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.

3 participants