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
[12.0][MIG] sale_manual_delivery #987
[12.0][MIG] sale_manual_delivery #987
Conversation
Hey @aheficent, thank you for your Pull Request. It looks like some users haven't signed our Contributor License Agreement, yet.
Appreciation of efforts, |
651413a
to
a8bd6da
Compare
a8bd6da
to
3ec5837
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.
functional ok
3ec5837
to
765c8a6
Compare
Hey @AaronHForgeFlow, thank you for your Pull Request. It looks like some users haven't signed our Contributor License Agreement, yet.
Appreciation of efforts, |
Added he possibility of changing the route when creating the delivery. This is useful when at the time of creating the order, we don't really know from warehouse and what location we are shipping. In case no route is provided then the one in the sale order line will be taken. |
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
@AaronHForgeFlow If ready, could you squash ? |
Add base for wizard wizard for manual procurements Update base computation update to OCA guidelines
Add action_manual_procurement_wizard in view
[ADD] Translation and .pot file
[IMP] Update translation files [IMP] lot's of functionnal improvments Don't add already planned or shipped lines in wizard Ensure you have one picking per manual delivery Forbid to add lines or ship more than ordered Planned date is required Improve views, naming and buttons Better Readme Default Carrier set from SO [FIX] Do not add product of type 'service' in manual delivery creation
…om SO Add product name to manual delivery lines
[FIX] call super in case of standard delivery [FIX] when multiple picking candidate available take the right one with date corresponding [FIX] revert of renaming field Add SO line description
[MIG][11] Update Readme + Manifest [MIG][11] Adapt procurement for V11 + rm api.one
Fix view name and improve tests IMP: Use a procurement group according to date planned Fix manifest keys
Hi @leemannd, in the wizard I've seen so far the possibility of changing the carrier and the planned date, but not the delivery address. It is ok to me to add that feature. Could you point me to your repo/the repo where this feature is implemented? Thanks Also, if I include the delivery_address in the wizard, Would be ok for you to continue with this Pull requests? (same in v10 and v11) |
@AaronHForgeFlow Thanks for asking. You have been removing it while helping on the migration of the V11 module -> 1f394eb#diff-ff81e585e998789b0b224c322c8cc742L19 This is present also in the original PR in V10.0 -> https://github.com/OCA/sale-workflow/pull/404/files?file-filters%5B%5D=.xml#diff-ff81e585e998789b0b224c322c8cc742R19 This is for us a blocking point as we have clients using it and needing this possibility. |
@leemannd I see. I tried to revert that removal but I missed that part. I will add it back here, and in v10/v11 |
@AaronHForgeFlow Just ping me on them, I will update my reviews right away |
46d3c5f
to
63052bf
Compare
[IMP]sale_manual_delivery. Specify route at the time of the delivery
63052bf
to
631f544
Compare
@leemannd Thanks to @mmequignon we have the delivery address back in the wizard 🎉 It would be nice if you check that everything is as expected. Thanks. |
Applied latest fixes in v11 |
class StockMove(models.Model): | ||
_inherit = "stock.move" | ||
|
||
carrier_id = fields.Many2one("delivery.carrier", string="Delivery Method") |
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.
@AaronHForgeFlow, why do we need to define the carrier at the stock.move level ?
cc @i-vyshnevska
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.
It is a requirement of the original authors of the module. #404 (comment)
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.
@AaronHForgeFlow natively Odoo defines the carrier at stock picking level (in delivery module). It confuses us if we have multiple carriers in the same picking (stock.move level), but only one deliver provider defined in picking.
In actual usage, if a picking is delivered by different providers, the picking should be splitted (partial transfer + create back order).
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.
Splits are not performed anymore with this module. In this case two separate manual deliveries will be 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.
@i-vyshnevska, do you have any idea?
@rousseldenis is this fine to be merged? |
/ocabot merge patch |
What a great day to merge this nice PR. Let's do it! |
Congratulations, your PR was merged at 3009c72. Thanks a lot for contributing to OCA. ❤️ |
Based on #983 + grindtildeath#2 at 8th November
Standard migration. Functional tests done from my side and working fine IMHO. Pending todo:
sale_stock_picking_blocking_sale_manual_delivery
between this andsale_stock_picking_blocking
(proposed in [ADD][10.0] sale_manual_delivery #935) is really needed to be ported to v12. --> not needed