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.

Sign up for GitHub

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

Jump to bottom

Migrate stock_available_sale to v8 #81

Merged
merged 3 commits into from Nov 20, 2015
Merged

Migrate stock_available_sale to v8 #81

merged 3 commits into from Nov 20, 2015

Conversation

clonedagain
Copy link

Correctly segregates templates and variants.
Rewritten in new API.
Uses the ORM instead of SQL calls.
Conforms better to OCA standards

You may be interested in this other PR too: #93

@clonedagain
Copy link
Author

@lbellier for your information

@clonedagain clonedagain force-pushed the 8.0-stock-available-sale branch 5 times, most recently from e77892c to 7bff33f Compare September 10, 2015 13:57
@clonedagain
Copy link
Author

The remaining failures seem unrelated to these changes, so I'd say this is ready for review.

@clonedagain clonedagain mentioned this pull request Sep 10, 2015
Closed
2 tasks
moylop260
moylop260 reviewed Sep 10, 2015
View reviewed changes
stock_available_sale/views/product_template_view.xml
<field name="name">product.form.quoted_qty</field>
<field name="model">product.product</field>
<field name="name">Quoted quantity on product form</field>
<field name="model">product.template</field>
Copy link

Choose a reason for hiding this comment

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

Why not product.product?

Copy link
Author

Choose a reason for hiding this comment

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

If I'm not mistaken, this view is reused for both product templates and variants so doing it on product.template serves both purposes at once.
The tree view on the other hand seems to have been specifically inherited for product.product in the standard, but I'm not adding the new field there.

@moylop260
Copy link

@clonedagain
travis red. Is a know error?

@clonedagain
Copy link
Author

@moylop260 The error seems unrelated but please double-check if you can.
The main 8.0 is red too, but I don't know if someone is working to make it green.

@clonedagain
Copy link
Author

@moylop260 if you care about this PR you may want to test #80 too, which makes the product/variant situation more correct.

@gurneyalex
Copy link
Member

@clonedagain build should be fixed by #82

This was referenced Sep 15, 2015
Closed
Closed
Closed
@gurneyalex
Copy link
Member

👍

if you have the opportunity to add an automated test using the location / warehouse context it would be a very nice addition.

@gurneyalex
Copy link
Member

@clonedagain can you rebase this PR and resolve the merge conflict?

@clonedagain
Copy link
Author

@gurneyalex I merged the latest 8.0 branch, I hope I didn't break anything.

@clonedagain
Copy link
Author

@gurneyalex I've improved the tests, hopefully now it's going to get the coverall quota.

@clonedagain
Copy link
Author

I've also removed the @api.one decorators like was done recently in stock_available_immediately.

@clonedagain clonedagain force-pushed the 8.0-stock-available-sale branch 3 times, most recently from 7f58b9d to 348d420 Compare October 30, 2015 20:37
@clonedagain clonedagain mentioned this pull request Nov 5, 2015
Closed
27 tasks
@clonedagain clonedagain force-pushed the 8.0-stock-available-sale branch 2 times, most recently from e4daf3d to c2de162 Compare November 6, 2015 16:36
@clonedagain
Copy link
Author

I had to rebase this on another PR: #100
This is not needed for this PR by itself, but I fear that the changes demanded by #100 will be forgotten if I don't include them right now.

Anyway, if runbot & travis are still green, this still needs review.

@clonedagain clonedagain mentioned this pull request Nov 6, 2015
Merged
7 tasks
@clonedagain clonedagain force-pushed the 8.0-stock-available-sale branch 4 times, most recently from f849506 to fea9755 Compare November 9, 2015 14:49
@clonedagain
Copy link
Author

Travis is red on a Transifex error. Anything I can do about it?

@pedrobaeza
Copy link
Member

Retry as you have done.

Lionel Sausin added 2 commits November 9, 2015 17:55
[IMP] Decouple the quantity for templates and variants
0ba7264 
There are cases where we dot NOT want to simply sum the quantities of all the variants. For example when dealing with manufacturing capacities, we may have to chose between variants because we can't make ALL of them with the same components.
So instead of a simple non-modular implementation, we'll let each module define his own implementation of how to compute the product template's quantity available for sale.
Simplify copyright headers
8360f92
@clonedagain clonedagain force-pushed the 8.0-stock-available-sale branch 2 times, most recently from a6f2e42 to 09c7eb8 Compare November 9, 2015 17:04
clonedagain
clonedagain reviewed Nov 19, 2015
View reviewed changes
stock_available_sale/models/product_product.py
# Compute the quoted quantity in the product's UoM
# Rounding is OK since small values have not been squashed before
results += Counter({
product_id: self.env['product.uom'].sudo()._compute_qty(
Copy link
Author

Choose a reason for hiding this comment

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

@lmignon I'm doing a sudo also in stock_available_sale, do you think it could break multi-company setups like it did in stock_available_mrp?

Copy link
Sponsor Contributor

Choose a reason for hiding this comment

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

@clonedagain Why do you need to use sudo() here? product.uom can be read by base.group_user. Moreover, each time you use sudo(), you build a new empty environment and the _compute_qty will need to query the database to retrieve the product, the uom, etc... IMO, you introduce here a performance issue. What do you think?

@clonedagain clonedagain mentioned this pull request Nov 20, 2015
Closed
Migrate stock_available_sale to v8
449071b 
Correctly segregates templates and variants.
Rewritten in new API.
Uses the ORM instead of SQL calls.
Conforms better to OCA standards
'Quoted qty' is now positive. Explain UoM feature drop.
Added directions to block quotations using sale_exceptions
Rewrote the tests in python and added new tests regarding warehouses, locations, variants and templates ; and now test the quantity available for sale.
Don't use SUPERUSER privileges anymore (it's not needed, it prevents cache reuse, it may breaks multicompany features)
Simplify copyright headers
@clonedagain
Copy link
Author

@lmignon I changed as you suggested.

@lmignon
Copy link
Sponsor Contributor

lmignon commented Nov 20, 2015

👍 (Code review) Tahnk you @clonedagain

@moylop260
Copy link

👍 (w/o test)

moylop260 added a commit that referenced this pull request Nov 20, 2015
@moylop260
Merge pull request #81 from numerigraphe/8.0-stock-available-sale
bcf19ba 
Migrate stock_available_sale to v8
@moylop260 moylop260 merged commit bcf19ba into OCA:8.0 Nov 20, 2015
@clonedagain clonedagain deleted the 8.0-stock-available-sale branch November 20, 2015 16:56
gschrott-osi pushed a commit to ursais/stock-logistics-warehouse that referenced this pull request Jun 5, 2023
@auroraeosrose
Merge pull request OCA#81 from burroaktoolinc/rl-oak-purchase
746e5d3 
[MIG] oak_purchase
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@clonedagain @moylop260 @gurneyalex @pedrobaeza @lmignon @yvaucher