purchase_delivery_address #46
Conversation
lepistone
force-pushed
the
purchase_delivery_address
branch
from
099abe8
to
1f3b1a4
Compare
|
User res_users_giulia in tests needs some group assignation -> Purchases/Manager 2014-11-03 07:41:54,453 23678 INFO openerp_test openerp.modules.loading: loading purchase_delivery_address/test/test_propagate_address.yml 2014-11-03 07:41:55,562 23678 WARNING openerp_test openerp.addons.base.ir.ir_model: Access Denied by ACLs for operation: read, uid: 7, model: framework.agreement |
|
@yvaucher she has not the right to read the framework agreement in the tests of purchase_delivery_address. Being an interaction between the two, I split the modules in the travis config. |
|
@lepistone green 👍 |
|
as discussed, it seems that a "dest_address_id" field exists in purchase core. |
The new delivery address in the purchase order is propagated to the picking.
lepistone
force-pushed
the
purchase_delivery_address
branch
from
d8edd99
to
764dec0
Compare
|
We solved the conflict and the test, but I have to check @rdeheele's remark. He is probably right. |
|
The field mentioned by @rdeheele exists but is hidden, see odoo/odoo/issues/2950. I adapted this module to use it, and extend/fix the existing functionality to get the same result as before. I updated the readme and the tests to reflect that. Thanks! |
|
No, the partner address is already in the core, in a field called I'm marking this one as WIP, and will see tomorrow that do to here. Thanks @rdeheele for the heads up. |
See the updated README for more infomation about the new logic. The idea is to have in the picking information about both the supplier and the customer address. The test test_propagate_supplier_and_chosen_address_to_picking show also clarify what is going on.
|
I uploaded version 47.8 of the pull request, with an update of the readme and tests. Thanks for the review! |
|
according to the readme explanation, it's 👍 |
| new_picktype = self.env['stock.picking.type'].search([ | ||
| ('default_location_src_id.usage', '=', 'supplier'), | ||
| ('default_location_dest_id.usage', '=', 'customer'), | ||
| ], limit=1) |
There was a problem hiding this comment.
I wonder if for this one we shouldn't have stock_dropshipping in dependancies and search it by xml id as I did here:
https://github.com/OCA/purchase-workflow/pull/47/files#r19790441
In anyway we must be consistent between both modules.
There was a problem hiding this comment.
A second opinion would be nice. To know in which PR we should change this part.
There was a problem hiding this comment.
I have no real preference here. I may add that searching by xml id does not necessarily mean adding a dependency on sale_dropshipping (as long as the exception is handled correctly)
There was a problem hiding this comment.
OTOH, since it is possible to get a dropshipping config without installing the sale_dropshipping module, this version is probably nicer.
There was a problem hiding this comment.
and then again... there are a number of cases in the official addons where such things are handled with an xmlid lookup + a search as a backup in case the xmlid was not found.
There was a problem hiding this comment.
If we don't use xmlid, thus stock_dropshipping poor little module will be sad as considered useless.
EDIT: adding poor and little before module 😿
There was a problem hiding this comment.
I agree that we can't hardcode picking type. We should find it with this technique.
There was a problem hiding this comment.
@pedrobaeza picking type dropshipping is a data created by an official module. And as said @gurneyalex on his last comment, for location in official addons customer and supplier locations are taken with xml id.
There was a problem hiding this comment.
You can have multiple cases that doesn't apply the search by XML-ID:
- You are in a multi-company environment and you define manually dropshipping picking type for that company.
- You duplicate dropshipping picking type and remove the first one without updating module.
- ...
So I think this must be this way, but adding company_id parameter.
There was a problem hiding this comment.
- dropshipping picking type is not related to a company.
- I agree there should be a mechanism to warn the user he is deleting a useful data.
- ... <-- I didn't thought of this one 😁
|
I changed the domain as suggested by @pedrobaeza. As for the visibility of the field, I think for the interface we have now we should always show it. The whole point of having the onchange is that the user can fill in the delivery address, and the system will find an appropriate picking type and destination for you. Thanks! |
|
To be clear, I argue with XML id lookup but as it will work this is still a 👍 for me |
This module adds a delivery address to the purchase order, which is
automatically propagated to the generated picking.
Moreover, when a sale order generates a purchase in drop shipping mode, the
generated purchase has the delivery address set.
The idea is that this way dropshipping pickings have a trace of the desired
delivery address. The partner_id field of the picking is already used for the
supplier.