search

twomice / com.joineryhq.percentagepricesetfield

Other
5 stars 10 forks source link

Fix 'formatMoney is not defined' error #30

Closed jitendrapurohit closed 4 years ago

jitendrapurohit commented 4 years ago

We've noticed the following error on few installs(mostly non drupal sites) using this extension.

image

Not sure what exactly causes this but this PR includes the same function defined in templates//CRM/Contribute/Form/Contribution.tpl in this ext so that it is not dependent on core file anymore.

alifrumin commented 4 years ago

We had a client experience this error too!!! This solution seems sensible to me. I tested it and it resolved the js error as expected.

twomice commented 4 years ago

This seems like a wise approach, thanks @jitendrapurohit I'd feel better if the formatMoney() function were defined within the CRM.percentagepricesetfield object scope (starting on line 6 of this file), rather than the global scope. It would also mean changing the formatMoney() call (at the end of calculateTotal()) to CRM.percentagepricesetfield.formatMoney(...).

Would you mind trying it that way?

twomice commented 4 years ago

Wait, hold on @jitendrapurohit I think #31 might be a better approach here. Let me follow-up there with Ali.

twomice commented 4 years ago

OK, looking closer, I see that the core function CRM.formatMoney() has a different signature than the previous core function formatMoney(). Since we're likely to be supporting a range of versions, I don't want to fight core over function signatures. I prefer to fork the function as @jitendrapurohit does in this PR.

@jitendrapurohit If you would, please consider updating the PR with the change asked for in my comment above (https://github.com/twomice/com.joineryhq.percentagepricesetfield/pull/30#issuecomment-624686741) . Thanks!

twomice commented 4 years ago

FYI, @jitendrapurohit I'm getting enough traffic on this issue that I'm just going to go ahead and merge this, add my suggested improvements, and cut a new release. Thanks for the PR!