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

[Fix]stock: owner on picking not propagated on quants (Maintenance Issue: 619854) #4548

Closed
wants to merge 1 commit into from

Conversation

skh-odoo
Copy link
Contributor

@skh-odoo skh-odoo commented Jan 6, 2015

Steps to reproduce:
1 Navigate to Settings > Configuration > Warehouse, enable 'manage owner on stock', and enable Technical Features for your user
2. Create a incoming stock picking from partner = Camptocamp, with owner = Luc Maurer of 13 IPads
3. Process the picking
4. Display the IPad product and click on the button on the top right corner saying "13 on hand"

Expected:
in the displayed list, I expect to see 1 line with a quant of 13 units and owner = Camptocamp (the commercial_partner_id of Luc Maurer)

Actual:

there is no owner on the quant.

@skh-odoo
Copy link
Contributor Author

skh-odoo commented Jan 6, 2015

Pull request created for the Issue #4136

@mart-e mart-e added the OE the report is linked to a support ticket (opw-...) label Jan 6, 2015
@@ -1057,6 +1057,7 @@ def _picking_putaway_apply(product):
'product_qty': 1.0,
'location_id': pack.location_id.id,
'location_dest_id': quants_suggested_locations[pack_quants[0]],
'owner_id': picking.owner_id.id or False
Copy link
Contributor

Choose a reason for hiding this comment

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

or False is not needed

@mart-e
Copy link
Contributor

mart-e commented Jan 6, 2015

as said @lepistone the view is a different fix so should be done in another commit (each one explaining what is currently fixing)

@@ -1091,6 +1091,7 @@
<field name="create_date" invisible="1"/>
<field name="date"/>
<field name="date_expected" on_change="onchange_date(date,date_expected)"/>
<field name="restrict_partner_id" invisible ="1"/>
Copy link
Contributor

Choose a reason for hiding this comment

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

extra space

@lepistone
Copy link
Contributor

Thanks for the PR @skh-odoo @mart-e.

Something is not clear to me: we have both owner_id on the picking, and restrict_partner_id on the move. Which one is supposed to be used and when?

The changes to the view seem to add a default value on an invisible field (restrict_partner_id) so if the user adds a move and then enters the owner in the picking, it is not propagated. Since the field is hidden, do we actually need it?

Thanks!

@mart-e
Copy link
Contributor

mart-e commented Jan 6, 2015

The reported issue #4136 is fixed in the python code.
@skh-odoo is the xml changes for another issue? Could not find it.
As said Leonardo, it will work only if done is a certain order. Instead of using context hack, maybe we could set the value for restrict_partner_id at the confirmation of the move/validation of picking.

@lepistone
Copy link
Contributor

@mart-e we agree python code fixes #4136 which is owner propagation picking.in -> quant.

The other issue is respecting owner on stock reservation. According to @jco-odoo here odoo-dev@3e26a5d it is not fully implemented.

I accept that, so I developed OCA/stock-logistics-workflow/pull/44: it makes reservation always respect the owner, which I made required on quants (with proper defaults) to avoid the ambiguity of "no owner". The tests of that module should clarify things.

The thing I miss is - as I said - what's the difference between Picking.owner_id and Move.restrict_partner_id. I'm working on another module to specify the owner on sale order lines, and I doubt which field I should fill in the generated picking.

Thanks!

@jco-odoo
Copy link
Contributor

jco-odoo commented Jan 6, 2015

The restrict_partner_id could have been named owner_id. It is to have a similar naming as restrict_lot_id. Somewhere else, I stated that the restrict_partner_id on the move should be the owner of the picking and vice versa, like the owner_id on the pack operation. It is logical to have one picking with only moves of the same owner.

@lepistone
Copy link
Contributor

Thanks for the answer @jco-odoo. In my module (sale_owner_stock_sourcing) I propagate the owner from sale.order.line to the move. If it is meant like you say, I should probably create separate pickings (and procurement groups?) if two sale.order.lines have different owners.

@jco-odoo
Copy link
Contributor

jco-odoo commented Jan 6, 2015

Normally if it is a sale order, the owner is you. The owner is meant for
when your warehouse does only the logistic operations for a third party.
If you get an assignment, you could create the picking immediately with the
correct owner. Or you need to invent a special kind of sale order.

On Tue, Jan 6, 2015 at 4:19 PM, Leonardo Pistone notifications@github.com
wrote:

Thanks for the answer @jco-odoo https://github.com/jco-odoo. In my
module (sale_owner_stock_sourcing) I propagate the owner from
sale.order.line to the move. If it is meant like you say, I should probably
create separate pickings (and procurement groups?) if two sale.order.lines
have different owners.


Reply to this email directly or view it on GitHub
#4548 (comment).

@lepistone
Copy link
Contributor

@jco-odoo exactly, the "special kind of sale order" is the "logistic order" we have in the NGO verticalization:

https://github.com/OCA/vertical-ngo/blob/8.0/logistic_order/__openerp__.py#L31-L40

We are handling sale lines with owners separately in

https://github.com/OCA/sale-workflow/pull/89/files

because someone might want to use those for something other than NGOs. There I am just propagating the owner (if any) from the order line to the move (possibly splitting pickings, which is my concern).

@jco-odoo
Copy link
Contributor

jco-odoo commented Jan 6, 2015

The owner should be added in the domain of the _picking_assign method then. That way they can be separated.

@lepistone
Copy link
Contributor

Thanks a lot @jco-odoo, I hadn't found picking_assign yet!

@mart-e
Copy link
Contributor

mart-e commented Jan 7, 2015

Merged at 28833f0, thanks
The owner on the stock.move is another fix that should probably be done differently, still a reflexion in progress.

@mart-e mart-e closed this Jan 7, 2015
@mart-e mart-e deleted the 8.0-opw-619854-skh branch January 7, 2015 13:00
lepistone added a commit to lepistone/odoo that referenced this pull request Feb 9, 2015
@lepistone
Copy link
Contributor

Hi all, I found a problem with this fix. I opened issue #5165 and PRs: #5166 OCA#158.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OE the report is linked to a support ticket (opw-...)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants