-
-
Notifications
You must be signed in to change notification settings - Fork 730
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_quant_manual_assign #361
Conversation
… the wizard of "quants Manual" and check availability.
* Extended usage in README
Conforms to the latest README template: bugtracker, runbot etc. Fixes bugtracker URL on some modules. States OCA as maintainer, removes other contributors from the 'Maintainer' section.
* Fix uom issue * Convert move quantity in the default product uom to use the same uom as quant
[FIX] test_flake8 [FIX] - use the already existing field product_qty - fix picking edition by warehouse user due to related none readonly field
You can review other PRs and ask in exchange that they review yours |
Hi, 2 other Travis-CI report is relative to the "test_stock_quant_manual_assign.py" which is from v9.0 -> v10.0. I just don't know how to use them. is there any document about the it? Is that good to remove them? |
You should fix everything, and test shouldn't be removed, but fixed. Tests assures that everything goes correct. Tests can fail because:
|
@fanha99 The quantity field in stock.quant model has changed. It's called 'quantity' now (old name : qty). |
@rousseldenis, thanks. I can run the test now. but there are still bugs |
fb31d5e
to
340afb7
Compare
@DbTechGit you can try this module to "lock" assigned serial |
340afb7
to
7850ab9
Compare
@fanha99 Is this ok ? |
@rousseldenis : i'm using this module in our production Odoo. I think it is ok for us. :)
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code review
self.env['stock.quant'].quants_reserve(quants, move) | ||
move._do_unreserve() | ||
for line in self.quants_lines: | ||
if line.qty > 0: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@fanha99 Please use float_compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should I use math.isclose(line.qty, 0.0, rel_tol=1e-6)
the math.isclose is with the python 3.5
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, Odoo orm uses a layer on top of some float fields to interpret what is stored in database. These fields are defined with decimal precision. So, to interpret values, you have to use orm-integrated functions as float_compare. Other libraries are useless.
move.product_id, | ||
move.location_id, | ||
lot_id=line.lot_id) | ||
if available_quantity <= 0: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe a float_compare on this should be used too
line['qty'] = 0 | ||
move_lines = move.move_line_ids.filtered( | ||
lambda ml: | ||
ml.location_id == x.location_id and ml.lot_id == x.lot_id) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is too restrictive (or I don't understand the flow) ? IMHO you have to add condition as you maybe want to match quants that don't have lot if move line has no lot.
... and (ml.lot_id == False and x.lot_id = False or ml.lot_id == x.lot_id)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
multi locations?
when the delivery order is form location A, you could not choice the product from location B to send out.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On move lines, you can have multiple locations if products come from children of move location.
Try to store product A in STOCK > Shelf 1 and product B in STOCK > Shelf 2. Create a picking from stock to out with product A and product B.
Moves are from stock to out and move lines are from children locations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these lines of code is getting the correct reserved quant/lot info (form previous action) and display to the wizard form.
wizard.lines_qty) | ||
for quant in self.move.reserved_quant_ids: | ||
self.assertTrue(quant in selected_quants) | ||
# self.assertEqual(len(wizard.quants_lines.filtered('selected')), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove comments if they are not needed
<field name="in_date" /> | ||
<field name="lot_id" /> | ||
<field name="package_id" /> | ||
<field name="lot_id" readonly="1"/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why changing to readonly from v10 ? Is the flow changed ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these fields were set to readonly in the python code. Just not remember why need to set readonly the the xml view too. maybe there are bugs or somethings else.
will double check.
there are 10 months since the last commit (Dec 28, 2017).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, no problem. Try to make this merged.
@fanha99 can I push to this repository and finish this pull request? |
@cubells : yes, please :) |
aea7c2e
to
4a3a6d2
Compare
@fanha99 Thanks for the work here! I just tested it on runbot and it does not work as I expected if you select quants in sublocations. Example: In the example above I select the quant in the small refrigerator and confirm. After that the quantity reserved is still 0. |
This is because there's no stock.move.line created. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code review. LGTM. Could you attend @lreficent remarks?
@@ -0,0 +1,5 @@ | |||
# -*- coding: utf-8 -*- | |||
############################################################################## | |||
# For copyright and license notices, see __openerp__.py file in root directory |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not relevant
@rousseldenis I don't think this is working properly. See our previous comments. |
I'm working on a pr to approve this pr. |
Superseded by #508 |
[11.0] [MIG] stock quant manual assign
please help to check, thx.
NOTE: in v11, the stock_quant is take different role in compare to v10.
This module must be stock lot/serial manual assign