[16.0][ADD] stock_picking_limit_return_qty #1424
Conversation
stock_picking_limit_return_qty/tests/test_stock_picking_return.py
Outdated
Show resolved
Hide resolved
| _( | ||
| "You can return only %(max_qty)s of %(product)s", | ||
| max_qty=rec.quantity_max, | ||
| product=rec.product_id.name, | ||
| ) |
There was a problem hiding this comment.
You need to wrap only "You can return only %(max_qty)s of %(product)s" argument.
| error = "Error" | ||
| msg = "" | ||
| try: | ||
| line.quantity = line.quantity + 1 | ||
| except ValidationError as e: | ||
| error = str(e) | ||
| msg = "You can return only %s of %s" % ( | ||
| line.quantity_max, | ||
| line.product_id.name, | ||
| ) | ||
| self.assertEqual(error, msg) |
There was a problem hiding this comment.
Maybe self.assertRaises(ValidationError) instead?
Limit quantity of items that can be returned in the same picking
DemchukM
force-pushed
the
16.0-t2735-add-stock_picking_limit_return_qty-2
branch
from
43c3f44
to
a510b42
Compare
|
@em230418 we have pushed the updates. Could you please do a code review again? Thank you in advance! |
There was a problem hiding this comment.
Mostly good. View definition could be simpler if I'm not mistaken.
can you take a look at my PR? #1432
| <field name="inherit_id" ref="stock.view_stock_return_picking_form" /> | ||
| <field name="arch" type="xml"> | ||
| <xpath | ||
| expr="//field[@name='product_return_moves']/tree/field[@name='quantity']" |
There was a problem hiding this comment.
I only see one quantity in the view, why not use the simpler field selector ?
<field name="quantity" position="after" />
<field name="quantity_max" invisible="1" />
</field>or even
<form position="inside" />
<field name="quantity_max" invisible="1" />
</form>There was a problem hiding this comment.
Hi @robinkeunen thank you for your feedback! We opted for keeping them more explicit in case this module would be installed on some system which already has some modifications done in the same views.
To make it more capable of survival in a harsh module conditions)
There was a problem hiding this comment.
Well, I think it's actually more robust to be more generic : <form> or <field name="quantity"> have more chance to survive the future than //field[@name='product_return_moves']/tree/field[@name='quantity'].
But ok :-)
|
This PR has the |
| )._prepare_stock_return_picking_line_vals_from_move(stock_move) | ||
|
|
||
| # Store maximum quantity that is possible to return | ||
| res.update( |
There was a problem hiding this comment.
As this is breaking the default Odoo's behaviour, I would have added a configuration parameter to activate this.
Either a global one, either one on stock.picking.type object (I don't know if it is defined on stock move values at this point)
There was a problem hiding this comment.
@rousseldenis thank you for mentioning this. Sounds reasonable. The only thing we didn't add it is because this is actually the only feature this module is adding. So this setting is actually if the module installed or not. Don't want this behaviour? Just uninstall it.
However the concern was that this module will be installed on the runboat bu default. So it might cause some confusion. It could be a good idea if this module could be installed as using checkbox in settings like this is done with some core Odoo modules. However I didn't manage to find to do the same for the third party modules.
Would be great if you could give some hints.
|
There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days. |
github-actions
bot
added
the
stale
Limit quantity of items that can be returned in the same picking
Before this PR:
User can return any amount of products in picking return. Even if this amount is more tha initial amount in the original picking.
After this PR:
if user tries to return bigger qty ot a product than it was in the original picking ValidationError is raised. This includes quantities already returned.