openerp-community team mailing list archive

[Lionel Sausin - Numérigraphe <ls@xxxxxxxxxxxxxxxx>]

Review: Needs Fixing

Functional: Worked for me, no problem.
I only wish it was possible to mix fixed price rules and standard ones, so we could get rules like
- product A = 1€
- product B = 3€
- other products = sale price + 10%
But this is something that we can change later, it should not block this MP.

GUI:
The form layout for the fixed price list version is slightly wrong (the number of cols must be wrong I guess).
Would you mind making the list view of pricelist lines editable? That would make it much faster to enter.

Code:
I suggest you merge the content of "model/" in a single file at the upper level directory to make consistent with the other modules.

There are spaces at the end of lots of lines, would you care to remove them please?

Can you please make the docstring for the check_*() functions more explicit that "Raise exception when error found", like maybe "Ensure the fixed prices are in fixed pricelists"?

The docstring states that "_compute_vals()" mutates the argument "vals", so I'd be more comfortable if it was called "_adapt_vals()" or something likely explicit.

In _compute_vals you don't use the context, so please remove "context = context or {}"

In write(),the context is missing from the browse() call.

The python files lack copyright comments, is that OK?

-- 
https://code.launchpad.net/~therp-nl/openerp-product-attributes/7.0_lp1272282_fixed_price/+merge/203348
Your team OpenERP Community is subscribed to branch lp:openerp-product-attributes.