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

[14.0] delivery_auto_refresh: migration to v14 #473

Merged
merged 39 commits into from
Apr 6, 2022

Conversation

ilyasProgrammer
Copy link
Member

@ilyasProgrammer ilyasProgrammer commented Apr 5, 2022

based on #466
This one is replacement of #472

pedrobaeza and others added 30 commits February 24, 2022 11:58
Auto-refresh delivery price in sales orders
===========================================

This module automates the delivery price handling for the following cases:

* If you change any line in your draft sales order (SO), when saving, the
  delivery price will be adjusted without having to click on "→ Set price".
* If specified in the system parameter, the delivery line can be also
  auto-added when creating/saving.
* If you deliver a different quantity than the ordered one, the delivery price
  is adjusted on the linked SO when the picking is transferred.

Configuration
=============

* Activate developer mode.
* Go to *Settings > Technical > Parameters > System Parameters*.
* Locate the setting with key "delivery_auto_refresh.auto_add_delivery_line"
  or create a new one if not exists.
* Put a non Falsy value (1, True...) if you want to add automatically the
  delivery line on save.

Known issues / Roadmap
======================

* After confirming the sales order, the price of the delivery line (if exists)
  will be only updated after the picking is transferred, but not when you
  might modify the order lines.
* There can be some duplicate delivery unset/set for assuring that the refresh
  is done on all cases.
* On multiple deliveries, second and successive pickings update the delivery
  price, but you can't invoice the new delivery price.
* This is only working from user interface, as there's no way of making
  compatible the auto-refresh intercepting create/write methods from sale order
  lines.
Currently translated at 100.0% (2 of 2 strings)

Translation: delivery-carrier-12.0/delivery-carrier-12.0-delivery_auto_refresh
Translate-URL: https://translation.odoo-community.org/projects/delivery-carrier-12-0/delivery-carrier-12-0-delivery_auto_refresh/zh_CN/
Currently translated at 100.0% (2 of 2 strings)

Translation: delivery-carrier-12.0/delivery-carrier-12.0-delivery_auto_refresh
Translate-URL: https://translation.odoo-community.org/projects/delivery-carrier-12-0/delivery-carrier-12-0-delivery_auto_refresh/zh_CN/
Currently translated at 100.0% (2 of 2 strings)

Translation: delivery-carrier-12.0/delivery-carrier-12.0-delivery_auto_refresh
Translate-URL: https://translation.odoo-community.org/projects/delivery-carrier-12-0/delivery-carrier-12-0-delivery_auto_refresh/pt_BR/
…+ add on create

As the used method now has a check for not being used when the order is confirmed, so
it collapses when you want to add the delivery line in that moment.

Previous code didn't take into account the direct creation of the sales order.
For not having bad interaction with the rest of the tests.
new() doesn't invoke defaults, so you can get an SO without company for example,
and interacts bad with other modules.
Currently translated at 100.0% (2 of 2 strings)

Translation: delivery-carrier-12.0/delivery-carrier-12.0-delivery_auto_refresh
Translate-URL: https://translation.odoo-community.org/projects/delivery-carrier-12-0/delivery-carrier-12-0-delivery_auto_refresh/sl/
In the core the onchange is now aligned with `partner_shipping_id`
instead of `partner_id`.
odoo/odoo@b6f91e3
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: delivery-carrier-13.0/delivery-carrier-13.0-delivery_auto_refresh
Translate-URL: https://translation.odoo-community.org/projects/delivery-carrier-13-0/delivery-carrier-13-0-delivery_auto_refresh/
When the company has different currency than the pricelist asociated to the partner. The final price is converted to the currency of the pricelist.

With this changes we achieve to avoid this recomputation and solve this problems on tests.

TT26914
…n website

The steps to reproduce the problem are these:
    - Set delivery_auto_refresh.auto_add_delivery_line.
    - Set a default delivery method on the user's partner.
    - Place a new ecommerce order with that user adding to the cart.

Doing this steps we get the error:
    Record does not exist or has been deleted.
    (Record: sale.order.line(63,), User: 1)
We should get as the same default carrier as Odoo does.

TT34020
Added new config paramerter that automates de voiding of the delivery
line for the case of returning a picking that just didn't go away from
the warehouse (due to several causes like the customer cancelling the
sale order in the last minute).

This will only be performed if the order wasn't already invoiced and the
return was set to refund.

TT30359
simahawk and others added 4 commits February 24, 2022 14:22
If there were more than one delivery line, we'd get a singleton error
when trying to refresh due to the need of taking the discount of the
former lines.

TT35025
When choosing the sale carrier from the header of the order form we
should filter those carriers not available for the given shipping
address the in the same way the wizard does it.

TT35200
@ilyasProgrammer ilyasProgrammer marked this pull request as ready for review April 5, 2022 11:16
@ilyasProgrammer
Copy link
Member Author

@chienandalu @simahawk

Copy link
Member

@chienandalu chienandalu left a comment

Choose a reason for hiding this comment

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

Thanks @ilyasProgrammer Tested functionaly and working Can you squash @simahawk migration commits into the single one?

@ilyasProgrammer
Copy link
Member Author

Thanks @ilyasProgrammer Tested functionaly and working Can you squash @simahawk migration commits into the single one?

@chienandalu All commits of this branch? Or maybe you could specify range of commits hashes to squash because im not sure which has to be sqashed.

@simahawk
Copy link
Contributor

simahawk commented Apr 5, 2022

you don't have to squash my commits: they are improvements 😉
If you have to touch commits pls rewrite the last one to something like:

develiry_auto_refresh: enable via config params

Added parameter set_default_carrier to avoid conflicts with other modules.

@@ -2,6 +2,10 @@
<!-- Copyright 2018 Tecnativa - Pedro M. Baeza
License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl). -->
<odoo noupdate="1">
<record id="set_default_carrier" model="ir.config_parameter">
Copy link
Contributor

Choose a reason for hiding this comment

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

this param should be set from res.config.settings IMO but as other params here are not.... I understand if you don't do it ;)

@ilyasProgrammer
Copy link
Member Author

@simahawk @chienandalu So should I squash commits or not? Which ones should I squash?

@simahawk
Copy link
Contributor

simahawk commented Apr 6, 2022

From my POV no need to squash, just rewrite your last commit 🙏

@chienandalu
Copy link
Member

👍

Added parameter set_default_carrier to avoid conflicts with other modules.
@ilyasProgrammer ilyasProgrammer force-pushed the 14.0-delivery-auto-refresh_3 branch from 4ab316b to eeb17fb Compare April 6, 2022 09:23
@simahawk simahawk changed the title 14.0 delivery auto refresh 3 [14.0] delivery_auto_refresh Apr 6, 2022
@simahawk simahawk changed the title [14.0] delivery_auto_refresh [14.0] delivery_auto_refresh: migration to v14 Apr 6, 2022
@simahawk
Copy link
Contributor

simahawk commented Apr 6, 2022

/ocabot merge minor

@OCA-git-bot
Copy link
Contributor

This PR looks fantastic, let's merge it!
Prepared branch 14.0-ocabot-merge-pr-473-by-simahawk-bump-minor, awaiting test results.

@chienandalu
Copy link
Member

/ocabot merge minor

Shouldn't it be nobump?

@OCA-git-bot OCA-git-bot merged commit a329a0f into OCA:14.0 Apr 6, 2022
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at 99f67e6. Thanks a lot for contributing to OCA. ❤️

@simahawk
Copy link
Contributor

simahawk commented Apr 6, 2022

/ocabot merge minor

Shouldn't it be nobump?

there were several changes. IMO it's preferable to "tag" those changes in another version (which BTW is better if someone was already using 14.0.1.0.0).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.