OCA / pos

GNU Affero General Public License v3.0
272 stars 600 forks source link

[12.0]POS ORder Return is not computing correctly fianancial parts #508

Closed flotho closed 4 years ago

flotho commented 4 years ago

Just made a test here : http://3426910-480-76ca7e.runbot2-3.odoo-community.org/web#id=2&action=315&model=pos.order&view_type=form&menu_id=176 As you can see :+1: image picte_subtotal and price_subtotal_incl are not negative , this is not the native behaviour and cause great troubles with cash control because account.move are no more balanced.

flotho commented 4 years ago

Looks like that part https://github.com/OCA/pos/blob/12.0/pos_order_return/models/pos_order.py#L72 is not setting the price_subtotal. It's not the native behaviour there : https://github.com/odoo/odoo/blob/12.0/addons/point_of_sale/models/pos_order.py#L1022

flotho commented 4 years ago

@legalsylvain , what's your point view do you confirm the unexpected behaviour?

flotho commented 4 years ago

@chienandalu , what's your point of view on this one ?

legalsylvain commented 4 years ago

Thanks @flotho for your report. Indeed the subotal seems wrong. (also the margin are wrong, BTW, as it depends on the subtotal)

@chienandalu : could you take a look ? It seems you introduced this code during a migration.

thanks !

chienandalu commented 4 years ago

No problem, although although I can't promise to attend it until middle of july since I'm currently finshing tasks berfore holidays...

flotho commented 4 years ago

No problem, I will propose a patch for this, Iwas just looking for confirmations from your side.

Regards

chienandalu commented 4 years ago

Looks like a bug to me :)

ivantodorovich commented 4 years ago

Yeap, we're having the same issue.

In fact it only happens with full refunds. Partial refund (even by completing all the quantities) works ok.

I believe it's because in partial_refund we're calling _onchange_amount_line_all, which is updating price_subtotal_incl and price_subtotal.

In the standard module this is not done because they're just copying the values. But in our case we are filtering some lines so we should probably call it, and also _onchange_amount_all, like it was done here: https://github.com/OCA/pos/blame/12.0/pos_order_return/models/pos_order.py#L99

@flotho are you working on it ? I need to fix it for a customer asap so I would gladly do it if you haven't started yet

ivantodorovich commented 4 years ago

Well, it was as easy as that, so I've just gone ahead and fixed it: https://github.com/OCA/pos/pull/509