foodcoops / foodsoft

Web-based software to manage a non-profit food coop (product catalog, ordering, accounting, job scheduling).
https://foodcoops.net/
Other
326 stars 146 forks source link

Synchronization from external database failed when unit value differs #1014

Closed kidhab closed 1 year ago

kidhab commented 1 year ago

This is actually a users feedback that I was able to reproduce via our demo instance.

Trying to synchronise the supplier database with the external source failed with a ZeroDivisionError:

Completed 500 Internal Server Error in 69ms (ActiveRecord: 36.5ms | Allocations: 6684)
[...]
FATAL -- : [95b7f1ef-f999-43e3-aa1e-8b52f315acf9]   
ZeroDivisionError (ZeroDivisionError):
[95b7f1ef-f999-43e3-aa1e-8b52f315acf9] app/models/article.rb:215:in `convert_units'
[95b7f1ef-f999-43e3-aa1e-8b52f315acf9] app/models/article.rb:137:in `unequal_attributes'
[95b7f1ef-f999-43e3-aa1e-8b52f315acf9] app/models/article.rb:117:in `shared_article_changed?'
[95b7f1ef-f999-43e3-aa1e-8b52f315acf9] app/models/supplier.rb:50:in `block in sync_all'
[95b7f1ef-f999-43e3-aa1e-8b52f315acf9] app/models/supplier.rb:44:in `sync_all'

The error occurs when the unit field contains a comma seperated weight (e.g. "kg" or "g") value which differs from the value of the external database - if it also contains a weight specification.

There's no error if you remove the weight specification. So I assume it's caused by the conversion.

You can reproduce here.

The screenshot shows the problematic unit field of "Article1":

Bildschirmfoto vom 2023-06-21 22-05-19

And the related Article in the shared database:

Bildschirmfoto vom 2023-06-21 22-10-09

yksflip commented 1 year ago

awesome bug hunting kidhab! Could it be possible that in conversion_factor = (supplier_unit / fc_unit).to_base.to_r fc_unit is zero ? wait .. shouldn't unit just be kg or g without a numeric value? I'm irritated :D I'll have a closer look tomorrow

yksflip commented 1 year ago

soo for some reason Unit.new("0,9kg") becomes 0 ... I fixed it by adding zero checks to the convert_units function. The same exception could be reproduced by simply setting Unit to "0kg" ... Not sure if we'd want to maybe replace "," to "." or explain the user that only "." is allowed