moqui / moqui-framework

Use Moqui Framework to build enterprise applications based on Java. It includes tools for databases (relational, graph, document), local and web services, web and other UI with screens and forms, security, file/resource access, scripts, templates, l10n, caching, logging, search, rules, workflow, multi-instance, and integration.
http://www.moqui.org
Other
279 stars 200 forks source link

Fixed a runtime error if Currency is BTC #555

Closed hellozhangwei closed 1 year ago

jonesde commented 1 year ago

Thank you for reporting this, yes it is a bad assumption that all uomId values are valid Java Currency IDs. This is a fairly small change that I could redo pretty quickly, but if you're up for another round of changes what this needs before going in:

  1. this is from an account that isn't in the AUTHORS file (hellozhangwei), are you the same person as 'zhangwei1979' that is in the AUTHORS file? if so maybe easier to submit under that account, if not please sign the AUTHORS file in a commit under this account with a line in each of the 2 sections (top one for CC0 copyright disclaimer, bottom for Apache 2 derived patent disclaimer)
  2. small formatting thing, space after 'if' and before the '('
  3. instead of logging an error, because this is an expected case better to log a trace message like this one:

if (logger.isTraceEnabled()) logger.trace("Ignoring NumberFormatException for Timestamp parse: " + e.toString());

Note that this has an if statement for performance reasons, so the String concatenation is not done if trace is not enabled (ie don't build the argument to a method you know will do nothing with it).

hellozhangwei commented 1 year ago

Thanks for your prompt sugguestion.