Reference UoM for category should be checked for uniqueness

Bug #731035 reported by Eric Caudal - www.elico-corp.com
40
This bug affects 5 people
Affects Status Importance Assigned to Milestone
Odoo Addons (MOVED TO GITHUB)
Fix Released
Low
OpenERP R&D Addons Team 2

Bug Description

Currently any UoM can be set as a reference for a given category which seems to be wrong (meaning one category can have several UoM references). By conception it is dubious and actually it makes some reports crashes like account_invoice_report with the following error message:

Traceback (most recent call last):
  File "/usr/local/lib/python2.6/dist-packages/openerp-server_9000/netsvc.py", line 489, in dispatch
    result = ExportService.getService(service_name).dispatch(method, auth, params)
  File "/usr/local/lib/python2.6/dist-packages/openerp-server_9000/service/web_services.py", line 599, in dispatch
    res = fn(db, uid, *params)
  File "/usr/local/lib/python2.6/dist-packages/openerp-server_9000/osv/osv.py", line 122, in wrapper
    return f(self, dbname, *args, **kwargs)
  File "/usr/local/lib/python2.6/dist-packages/openerp-server_9000/osv/osv.py", line 176, in execute
    res = self.execute_cr(cr, uid, obj, method, *args, **kw)
  File "/usr/local/lib/python2.6/dist-packages/openerp-server_9000/osv/osv.py", line 167, in execute_cr
    return getattr(object, method)(cr, uid, *args, **kw)
  File "/usr/local/lib/python2.6/dist-packages/openerp-server_9000/osv/orm.py", line 1735, in search
    return self._search(cr, user, args, offset=offset, limit=limit, order=order, context=context, count=count)
  File "/usr/local/lib/python2.6/dist-packages/openerp-server_9000/osv/orm.py", line 3981, in _search
    cr.execute('SELECT "%s".id FROM ' % self._table + from_clause + where_str + order_by + limit_str + offset_str, where_clause_params)Traceback (most recent call last):
  File "/usr/local/lib/python2.6/dist-packages/openerp-server_9000/netsvc.py", line 489, in dispatch
    result = ExportService.getService(service_name).dispatch(method, auth, params)
  File "/usr/local/lib/python2.6/dist-packages/openerp-server_9000/service/web_services.py", line 599, in dispatch
    res = fn(db, uid, *params)
  File "/usr/local/lib/python2.6/dist-packages/openerp-server_9000/osv/osv.py", line 122, in wrapper
    return f(self, dbname, *args, **kwargs)
  File "/usr/local/lib/python2.6/dist-packages/openerp-server_9000/osv/osv.py", line 176, in execute
    res = self.execute_cr(cr, uid, obj, method, *args, **kw)
  File "/usr/local/lib/python2.6/dist-packages/openerp-server_9000/osv/osv.py", line 167, in execute_cr
    return getattr(object, method)(cr, uid, *args, **kw)
  File "/usr/local/lib/python2.6/dist-packages/openerp-server_9000/osv/orm.py", line 1735, in search
    return self._search(cr, user, args, offset=offset, limit=limit, order=order, context=context, count=count)
  File "/usr/local/lib/python2.6/dist-packages/openerp-server_9000/osv/orm.py", line 3981, in _search
    cr.execute('SELECT "%s".id FROM ' % self._table + from_clause + where_str + order_by + limit_str + offset_str, where_clause_params)
  File "/usr/local/lib/python2.6/dist-packages/openerp-server_9000/sql_db.py", line 78, in wrapper
    return f(self, *args, **kwargs)
  File "/usr/local/lib/python2.6/dist-packages/openerp-server_9000/sql_db.py", line 131, in execute
    res = self._obj.execute(query, params)
ProgrammingError: more than one row returned by a subquery used as an expression

  File "/usr/local/lib/python2.6/dist-packages/openerp-server_9000/sql_db.py", line 78, in wrapper
    return f(self, *args, **kwargs)
  File "/usr/local/lib/python2.6/dist-packages/openerp-server_9000/sql_db.py", line 131, in execute
    res = self._obj.execute(query, params)
ProgrammingError: more than one row returned by a subquery used as an expression

Uniqueness should be checked when the UoM is created/modified. Even more, you should not be able to delete the reference of a category if other UoM depend on it.

Related branches

Revision history for this message
Olivier Dony (Odoo) (odo-openerp) wrote :

Indeed, the crash of reports due to multiple reference UoM for a category needs to be prevented or fixed.

I guess it is never really necessary to have multiple units in the same UoM category that have the same conversion rate, as that makes them equal to each other. We could add a unique constraint on (category_id, factor) so that only one UoM in each category is allowed to have the same conversion rate. This will also prevent having 2 reference UoMs with factor = 1.0.

I don't agree about the second part where you say that it should be forbidden to delete a reference UoM if there are other UoM. The reference UoM does not need to be explicitly defined, as per definition its factor is 1.0. If you have 2 UoM in the same category, let's say A (factor=0.5) and B (factor=0.1) you can easily convert from one to the other because you know their factor (against the implicit reference 1.0), without needing an explicit reference UoM. For example: 5.0 A = ((5.0/0.5)*0.1) B = 1.0 B
This is more convenient if the reference UoM should never be used directly.

Changed in openobject-addons:
assignee: nobody → OpenERP R&D Addons Team 2 (openerp-dev-addons2)
importance: Undecided → Low
status: New → Confirmed
Revision history for this message
Graeme Gellatly (gdgellatly) wrote : Re: [Bug 731035] Re: Reference UoM for category should be checked for uniqueness
Download full text (6.2 KiB)

I disagree with your agreement to the first part :) that it isn't necessary,
for multiple reference UOM especially in the unit case.

Roll, PCE, EA, Unit all mean the same mathematically, but some make no sense
when applied to other products. And in length, metre and lineal metre
while they mean the same, convention dictates some products sold by the
metre, others by the lineal.

e.g a roll of building paper vs a piece of building paper. just as a piece
of furniture vs a roll of furniture. makes no sense to be restricted to one
choice.

Would a better answer be to change the wording to "Equal to Reference Unit"
and then raise the bug on the one report expecting a single result.

On Tue, Mar 8, 2011 at 10:54 PM, Olivier Dony (OpenERP) <
<email address hidden>> wrote:

> Indeed, the crash of reports due to multiple reference UoM for a
> category needs to be prevented or fixed.
>
> I guess it is never really necessary to have multiple units in the same
> UoM category that have the same conversion rate, as that makes them
> equal to each other. We could add a unique constraint on (category_id,
> factor) so that only one UoM in each category is allowed to have the
> same conversion rate. This will also prevent having 2 reference UoMs
> with factor = 1.0.
>
> I don't agree about the second part where you say that it should be
> forbidden to delete a reference UoM if there are other UoM. The reference
> UoM does not need to be explicitly defined, as per definition its factor is
> 1.0. If you have 2 UoM in the same category, let's say A (factor=0.5) and B
> (factor=0.1) you can easily convert from one to the other because you know
> their factor (against the implicit reference 1.0), without needing an
> explicit reference UoM. For example: 5.0 A = ((5.0/0.5)*0.1) B = 1.0 B
> This is more convenient if the reference UoM should never be used directly.
>
> ** Changed in: openobject-addons
> Importance: Undecided => Low
>
> ** Changed in: openobject-addons
> Status: New => Confirmed
>
> ** Changed in: openobject-addons
> Assignee: (unassigned) => OpenERP R&D Addons Team 2
> (openerp-dev-addons2)
>
> --
> You received this bug notification because you are subscribed to OpenERP
> Addons.
> https://bugs.launchpad.net/bugs/731035
>
> Title:
> Reference UoM for category should be checked for uniqueness
>
> Status in OpenERP Modules (addons):
> Confirmed
>
> Bug description:
> Currently any UoM can be set as a reference for a given category which
> seems to be wrong (meaning one category can have several UoM
> references). By conception it is dubious and actually it makes some
> reports crashes like account_invoice_report with the following error
> message:
>
> Traceback (most recent call last):
> File
> "/usr/local/lib/python2.6/dist-packages/openerp-server_9000/netsvc.py", line
> 489, in dispatch
> result = ExportService.getService(service_name).dispatch(method, auth,
> params)
> File
> "/usr/local/lib/python2.6/dist-packages/openerp-server_9000/service/web_services.py",
> line 599, in dispatch
> res = fn(db, uid, *params)
> File
> "/usr/local/lib/python2.6/dist-packages/openerp-server_9000/osv/osv.py",
> line 12...

Read more...

Revision history for this message
Eric Caudal - www.elico-corp.com (elicoidal) wrote :

Hi Olivier,
I agree on the unique constraint you propose but I still feel that
having a mandatory reference per Category is necessary.

Maybe I understand it wrong but currently if you have one UoM in a
category it can be of 2 main types:
- reference ('Absolute' reference)
- bigger/smaller than the reference ('relative' meaning compared with
something)

Currently the system has no field for the 'reference' UoM in the
relative UoM definition so you cannot specify several 'reference' UoM
for one category . If you have 2 references which one should we compared
with? In other systems I saw, there was complete and separate
conversion factor tables which allow you several references (actually
whatever UoM could be a reference)

Now let's take the process logically in case we keep your philosophy
(uniqueness but no constraint on having a mandatory reference UoM and no
'reference' field in UoM definition)
- If you have 1 UoM in a category, it should be 'reference' type by
default (since there is nothing to compare). It doesnot matter actually
to much as it is unique in the category but there is no point of a
semantic lie (bigger than something that does not exist) .
- If you have 2 UoM, one should be reference and the other relative. 2
absolute reference are not possible (as you cannot then refer to them in
the relative UoM) and 2 relative UoM are not possible neither (even if
you calculation is right, the meaning is incorrect and leads to
misunderstanding in the user's head). Still could be acceptable even if
borderline.
- Problem is imagine that you have 3 or more UoM with no 'reference'
type, how do you tell the UoM that the conversion factor for the UoM1 is
relative to UoM2 or UoM3?

That's why I am in favor of having 1 reference mandatory per category
and only one. If you have different UoM in meaning (metric tons or metre
linear), you should create different categories (length, length lineal,
etc...) or create the correct relative connection (eg: 1 ROLL=100 m).

Where it becomes tricky is that you cannot force a mandatory reference
UoM for a category by SQL since the category is needed in the UoM and
the UoM would be needed in the category. I think we need to find a way
to check that at deletion/creation/modification, there is always at
least one UoM of 'reference' type in the category.

Another option of course can be to have several references per category,
create a field and specify the reference UoM in the relative UoM
definition (for relative UoM) and of course fix the bug on the reports.
I am not sure I like this option mainly because of the issue of circular
references and more complex design.

--
Eric

Revision history for this message
Atik Agewan(OpenERP) (aag-openerp) wrote :

Hello Eric,

Thanks for reporting,
It has been fixed in lp:~openerp-dev/openobject-addons/trunk-bug-731035-aag
Revision ID: <email address hidden>
Revision num: 4486.
It will be available in trunk soon,

Changed in openobject-addons:
status: Confirmed → In Progress
status: In Progress → Fix Released
Revision history for this message
Kyle Waid (midwest) wrote :

So these fixes are not pushed to the 6.0 branch? I just downloaded sources today and got the programming error. Why would a development branch get fixes to issues that are in "stable" branches.

Revision history for this message
Olivier Dony (Odoo) (odo-openerp) wrote :

I'm reopening this bug, in light of bug 734686 and on the comments on the current bug.

I think Graeme has a point in comment #2: we may indeed need multiple UoM with the same factor, even perhaps multiple ones with factor = 1.0, i.e multiple reference units. And I don't think this would be an issue.

What is critical here is that we only allow a single "reference quantity" in each category, which is the quantity that is equivalent to 1 x reference UoM. If you decide to have multiple UoM with factor = 1.0, they effectively become synonyms for each other, which is fine. As Graeme says, maybe you want to have 1 PCE = 1 UNIT = 1 ROLL, etc. It doesn't matter because they all represent the same quantity and can be compared correctly.

Eric, we do not need and do not want to explicitly specific the UoM that is used as reference for relative UoM, because the reference is always very clear: the quantity represented by factor or ratio = 1.0. So if a category only contains "Kg" of type "bigger than reference" and ratio = 1000, then it is implicit that this category has a reference unit (unnamed) that has ratio = 1.0, i.e 1/1000 of Kg. And if I define a second UoM named "mg" of type "smaller than reference" with ratio = 1000, then I am defining it relatively to that implicit reference UoM, 1 mg will in fact be equal to 1/1,000,000 of a Kg. I cannot define relative UoMs otherwise than that.
I think this gives me maximum flexibility (if I don't want "gram" to be used anywhere in the system, I don't need to define it) without the complexity of conversion tables, etc.
And if I want to define multiple UoMs with ratio = 1 in the same category, I don't think this is an issue, because by definition they are all exactly identical in term of relative quantities. This means that if you have 1 PCE = 1 ROLL, then you will have BOX-OF-10-PCE = BOX-OF-10-ROLLS, by definition. If you don't want that, then you should put ROLL and PCE in different categories.

Now, in order to avoid confusion, and as bug 734686 mentions, we should also enforce that UoM names are unique, but that is a separate bug.

My suggestion is therefore to revert the patch mentioned in comment #4, because it removes flexibility, and instead fix the report mentioned in the description, to make it more robust and support all the different cases: no explicit reference UoM, multiple reference UoM, all reference UoM inactive, etc.
A partial fix may have been provided in 6.0 with http://bazaar.launchpad.net/~openerp/openobject-addons/6.0/revision/4512.2.1

Changed in openobject-addons:
status: Fix Released → Confirmed
Revision history for this message
Eric Caudal - www.elico-corp.com (elicoidal) wrote :

Hi Olivier,
This is fine for me.

One question is what if you delete the last UoM reference in one category? How is the system gonna behave?
If it finds a UoM = 1000 x reference and there is no reference, what result should expect? 1?

Revision history for this message
Marco Dieckhoff (dieck) wrote :

I think the simpliest way would be to add reference as required many2one to the category, not as marker on the unit.

That way, existence is enforced, and it can only be one reference per category.

(We have to find a way to avoid the obvious chicken-and-egg problem)

Revision history for this message
Olivier Dony (Odoo) (odo-openerp) wrote : Re: [Bug 731035] Re: Reference UoM for category should be checked for uniqueness

On 05/31/2011 02:12 PM, Eric Caudal - www.elico-corp.com wrote:
> One question is what if you delete the last UoM reference in one
> category? How is the system gonna behave? If it finds a UoM = 1000 x
> reference and there is no reference, what result should expect? 1?

It doesn't matter if you delete the last reference unit, because its
existence is implicit. It would be strange, but if you only have one
"Kg" UoM in a category, and this UoM is not a reference one (e.g it's
1000x bigger), everything will be expressed in terms of Kg.
So all stock moves should record quantities in terms of Kg, all
computations should be done in Kg, etc. The reference UoM does not
matter at all in that case.
It only starts to matter when you have more than one UoM, and you want
to be able to convert between them: in that case you start considering
their size as compared to that virtual "reference UoM".
But this reference is really virtual, it does not need to be defined
explicitly, and it can also be defined multiple times with synonym
names. If I say that Kg is 1000x bigger than my reference unit, I don't
need to define it explicitly, but I know that other UoMs will need to be
defined relatively to 1/1000 of a Kg.

On 05/31/2011 02:38 PM, Marco Dieckhoff wrote:
> I think the simpliest way would be to add reference as required
> many2one to the category, not as marker on the unit.

This would only help if we have multiple units with factor=1.0 in a
category, and if we want to be able to choose the main one, right?
I don't think there are many cases where we need to do that (the report
mentioned in the bug being one). Most of the time the reference unit is
used implicitly, and we probably don't need to make the model more
complicated than it is (the chicken and egg problem being one complication)
Would it really be an issue if we have multiple synonym UoMs in the same
category, all with factor = 1.0? This would not be a common case, but if
they have been defined that way, I guess they all represent the same
concept, and we can choose anyone in the report.

What do you think?

Isn't it more flexible if we are free to define UoMs as we want, as long
as we know they are all expressed relatively to a "virtual reference
UoM" with factor = 1.0. You can have 0, 1, or more "aliases" defined for
that virtual UoM, it doesn't matter.

Revision history for this message
Eric Caudal - www.elico-corp.com (elicoidal) wrote :

It seems fine too me (until I can bullet-proof it on customer case ;))
It would deserve some note in the manual though.

Revision history for this message
OpenBMS JSC (openbmsjsc) wrote :

I agree that each category should have only 1 "reference" UoM, but it should not prevent the users to define different UoMs with the same factor. E.g. I'm implementing OpenERP for a Medical Center, and they have medicaments that are packaged in different forms, e.g. capsules in plastic package or capsules in a bottle. These packages may have the same factors (number of capsules in it). So in this case how can I define different packages without having the same factor for different forms?

Revision history for this message
Atik Agewan(OpenERP) (aag-openerp) wrote :

Hello Eric,

Thanks for reporting,
It has been fixed in lp:~openerp-dev/openobject-addons/trunk-bug-731035-aag
Revision ID: <email address hidden>
Revision num: 4782.
It will be available in trunk soon,

Changed in openobject-addons:
status: Confirmed → In Progress
status: In Progress → Fix Committed
Revision history for this message
Nathan (nathan-bowden-kiwi) wrote :

Guys, we prefer this: unique name constraint AND per category, refernce should only be one.

Revision history for this message
Fabien (Open ERP) (fp-tinyerp) wrote :

Hello,

1. We can have several UoM that are the reference in one category. See Graeme's comment.
2. The bug in sale/purchase report are real bug and not related to this issue:
   - reports should never convert UoM/Quantities to the reference UoM but to the UoM set on the product
   - so, the way we compute eports in SQL is wrong.

For an end user, the reference UoM of a product is the one set on the product form, not the one having factor=1. So, every report should convert to the UoM set on the product form by default. (it's a requirement if you need to compute correct cost prices too)

Revision history for this message
Fabien (Open ERP) (fp-tinyerp) wrote :

I fixed sale & purchase report to avoid UoM troubles r5162 in trunk.

Changed in openobject-addons:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Duplicates of this bug

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.