Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[11.0] [MIG] stock orderpoint uom #386

Merged
merged 14 commits into from
Mar 8, 2018

Conversation

bodedra
Copy link
Member

@bodedra bodedra commented Feb 27, 2018

No description provided.

@grindtildeath
Copy link
Contributor

grindtildeath commented Feb 28, 2018

@bodedra I wanted to try out the module and do a review, but I keep having a weird behaviour.
I increased the tests which are now failing with this commit : grindtildeath@7415e1a
But I have no idea why do I get :

test_stock_orderpoint_procure_uom.py:85: in _assert_purchase_generated
    self.assertEquals(lines.product_qty, 9)
E   AssertionError: 1.0 != 9

Although the value here is 9 : https://github.com/OCA/stock-logistics-warehouse/pull/386/files#diff-ace9f064ade2bd0ba24d6a8148f9c701R16

Any idea ?

Bugs are tracked on `GitHub Issues
<https://github.com/OCA/stock-logistics-warehouse/issues>`_. In case of trouble, please
check there if your issue has already been reported. If you spotted it first,
help us smashing it by providing a detailed and welcomed feedback.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/smashing/smash

[('orderpoint_id', '=', orderpoint.id),
('order_id', '=', purchase.id)])
self.assertEquals(len(purchase_line), 1)
self.assertEqual(purchase_line.product_qty, 2.0)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use assertAlmostEqual for float comparsions

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have changed from 2.0 to 2

"version": "11.0.1.0.0",
"author": "Eficent, "
"Odoo Community Association (OCA)",
"website": "https://www.odoo-community.org",
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Corresponded. Thank you.

Copy link
Contributor

@grindtildeath grindtildeath left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well it took me a while to figure it out, so please cherry-pick this commit grindtildeath@e520445 (as I can't open a PR on your repo) to go faster.

product_qty = orderpoint.product_uom._compute_quantity(
product_qty, orderpoint.procure_uom_id)
return super(ProcurementGroup, self).run(product_id, product_qty,
product_uom, location_id,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be orderpoint.procure_uom_id ⚠️
Because product_qty here is updated to the new UOM while product_uom stays the same...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Corresponded. Thank you.

sched = self.env['procurement.order']
sched.run_scheduler()
proc = sched.search([('product_id', '=', self.productA.id)])
self.assertEqual(proc.product_uom, self.uom_dozen)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, had you kept this replacing proc with purchase_line, the test would have failed because of the uom of the purchase is not the procure_uom of the orderpoint. 👎

@bodedra bodedra force-pushed the 11.0-mig-stock_orderpoint_uom branch 3 times, most recently from b36b739 to bb2056f Compare March 5, 2018 17:36
@bodedra bodedra force-pushed the 11.0-mig-stock_orderpoint_uom branch from bb2056f to ba59b52 Compare March 7, 2018 12:47
"version": "11.0.1.0.0",
"author": "Eficent, "
"Odoo Community Association (OCA)",
"website": "https://www.odoo-community.org",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@max3903 max3903 merged commit 79df2ff into OCA:11.0 Mar 8, 2018
@max3903 max3903 deleted the 11.0-mig-stock_orderpoint_uom branch March 8, 2018 10:11
@max3903 max3903 mentioned this pull request Mar 15, 2018
21 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants