sale-workflow
sale-workflow copied to clipboard
OCA
[16.0][ADD]sale_order_line_multi_warehouse: Split sale order in multiple pickings
Split sale order lines into multiple sub-lines assigned to different warehouses. When a sale order is confirmed, multiple pickings can be created depending on the warehouses selected.
@ValentinVinagre @HaraldPanten @luis-ron
T-6563
Isn't this the same as https://github.com/OCA/sale-workflow/tree/16.0/sale_sourced_by_line ?
@pedrobaeza Not exactly. sale_sourced_by_line lets you select a warehouse in a sale order line. Our module lets you split a single sale order line into multiple sublines, each one related to a different warehouse.
I think there's a shared code on how to do such split when confirming the sales order, so it should be at least an expansion module. cc @rousseldenis
I think there's a shared code on how to do such split when confirming the sales order, so it should be at least an expansion module. cc @rousseldenis
Hi Pedro, thanks for your comments.
Could you give us some examples of the shared code that you mention? The modules have different purposes and we don't want to create a "too complex" module, but we are open to improve our work. You know...
I was just talking from the "feature" perspective, as both should split the pickings (at procurement level). I haven't dug deeper, as I'm not using any of them. If you think there's no overlap, OK then.
I was just talking from the "feature" perspective, as both should split the pickings (at procurement level). I haven't dug deeper, as I'm not using any of them. If you think there's no overlap, OK then.
@manuelregidor @ValentinVinagre Could you deeper analise Pedro's comments and verify that we're not overlaping anything? As we're not using sale_sourced_by_line maybe there's something that we are not taking in consideration.
Denis's comments would be appreciated, as well 😉
THX!
@pedrobaeza It is kind of similar from a concept perspective. However, in this proposal not only a sale order line can be assigned to a specific warehouse, but they can also be split into separate warehouses. Because of that, it has been necessary to create a new model (sale.order.line.warehouse). Its class instances are related to a sale order line, and a sale order line can have multiple instances of sale.order.line.warehouse, so the logic to separate the sale order into multiple warehouses follows a different and much more complex path, as it shifts from sale.order.line model to sale.order.line.warehouse model.
Basically, in module sale_sourced_by_line the way of groupping lines in multiple procurement groups is done in the sale order line level, so using the methods to apply additional rules (a warehouse assignment in this case) is enough to group the sale order lines into different pickings. In our proposal, we have to achieve the separation from another level.
That is why an expansion module would be possible, yet the methods in sale_sourced_by_line module would be strongly modified as they should be adapted to the sale.order.line.warehouse level. Another issue is that module sale_sourced_by_line depends on sale_procurement_group_by_line, which contains methods also used in this module; again, those inheritances would be pretty much ignored for the same reason.
Using float_compare everywhere a field with decimal precision is used should be done.
@rousseldenis Thanks for your feedback. I'll have a look as soon as I can.
@rousseldenis @pedrobaeza This module is not compatible with sale_procurement_group_by_line. In the future, we could develope a module to make them compatible. However, we'd like to know what's the best way to proceed in these situations, as we cannot include sale_procurement_group_by_line in the "excludes" list in manifest. Thank you.
Well, I know by experience that such future may not come. On other modules, I did some checks against config["test_enable"] and detect if there's an injected context to only activate the behavior in other cases, but I know others like Roussel doesn't like this. What I totally dislike is to have more splits in the CI (we already have 2 splits) to test them apart.
but I know others like Roussel doesn't like this. What I totally dislike is to have more splits in the CI (we already have 2 splits) to test them apart.
And the solution that introduce a configuration parameter to activate the feature avoid both :-)
but I know others like Roussel doesn't like this. What I totally dislike is to have more splits in the CI (we already have 2 splits) to test them apart.
And the solution that introduce a configuration parameter to activate the feature avoid both :-)
I don't know exactly what you mean by "introduce a configuration parameter". There is already a configuration field in res.company to activate the features introduced by this module. What kind of additional parameter do we need here? Thank you.
but I know others like Roussel doesn't like this. What I totally dislike is to have more splits in the CI (we already have 2 splits) to test them apart.
And the solution that introduce a configuration parameter to activate the feature avoid both :-)
I don't know exactly what you mean by "introduce a configuration parameter". There is already a configuration field in res.company to activate the features introduced by this module. What kind of additional parameter do we need here? Thank you.
@rousseldenis Could you give us more details, please?
@rousseldenis Could you give some feedback to us? We're not sure about how to continue with that.
THX!
@rousseldenis Could you give us some advice? We are waiting to finalize this PR based on your comments. Thanks 😄
@rousseldenis I've removed the incompatibility with sale_procurement_group_by_line from the manifest and explained in roadmap why both modules should be used together. I don't think there's need to create a system parameter to activate the features provided by this module as there is already a field in company to activate them.
@rousseldenis I applied the changes you suggested and added a new test. Could have a look, please? Thank you.
@rousseldenis There is currently a very similar case to this module in https://github.com/OCA/sale-workflow/pull/3598/files. The solution has been to change the OCA CI in the repository for the incompatible modules. Don't you think that would be the solution to apply? I mean that it avoids creating parameters in odoo, etc. only by not adapting the CI. If you agree, we will apply it.
@rousseldenis @Daniel-Melian Tests passed. Could you have a look? Thank you
@rousseldenis you asked for changes some weeks ago. Could you have a look?
THX!
This PR looks fantastic, let's merge it! Prepared branch 16.0-ocabot-merge-pr-3357-by-rousseldenis-bump-nobump, awaiting test results.
Congratulations, your PR was merged at f04c027e3ee70ff23b499cd6d9cc771bfff4bc5d. Thanks a lot for contributing to OCA. ❤️