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

[16.0][MIG] delivery_package_fee: Migrate to version 16.0 #693

Merged
merged 15 commits into from
Oct 3, 2023

Conversation

tuantrantg
Copy link
Contributor

Note for the reviewer:

  • The price field (of product.product model) in 14.0 is replaced by the _get_contextual_price function (of model product.product) in 16.0. You can see the details in here

guewen and others added 14 commits August 25, 2023 07:43
A list of Package Fees can be added on shipping methods.
When a outgoing transfer is done, for each package fee configured on the
shipping method, a new sale order line is created with:

* The product selected on the Package Fee
* The product name with the number of the transfer in the line's description
  (e.g. "Service Fee (WH/OUT/00036)")
* The quantity equal to the number of packages in the transfer
* The unit price equal to the price set on the product's pricelist (so it can be
  different per customer and even have different pricing depending on the number
  of packages)
* The taxes configured on the product, fiscal position applies if any.

Package Fee lines are added only if their quantity and price is above zero.
Flag package fee order lines with a new boolean 'is_delivery_package_fee'.
Standard method '_is_delivery' will also consider this field to exclude
the lines from the computation of the 'expected_date' in 'sale' module.
'order_line' can be empty when SO is copied
from direct call to copy with False as default.
@rousseldenis
Copy link
Sponsor Contributor

/ocabot migration delivery_package_fee

@@ -81,7 +81,7 @@ def _create_sale(cls):
def _add_sale_carrier(cls, sale, carrier):
delivery_wizard = Form(
cls.env["choose.delivery.carrier"].with_context(
{"default_order_id": sale.id, "default_carrier_id": carrier.id}
**{"default_order_id": sale.id, "default_carrier_id": carrier.id}
Copy link
Sponsor Contributor

Choose a reason for hiding this comment

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

You can avoid such notation setting parameters directly:

(...).with_context(default_order_id=sale.id, ...)

Copy link
Sponsor Contributor

@rousseldenis rousseldenis left a comment

Choose a reason for hiding this comment

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

Code review. Minor change

@OCA-git-bot
Copy link
Contributor

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

@yvaucher
Copy link
Member

yvaucher commented Oct 2, 2023

@tuantrantg Please see @rousseldenis comment

@tuantrantg tuantrantg force-pushed the 16.0-mig-delivery_package_fee branch from e9372ff to f43aa90 Compare October 3, 2023 07:55
@tuantrantg tuantrantg force-pushed the 16.0-mig-delivery_package_fee branch from f43aa90 to 6b1e242 Compare October 3, 2023 07:56
@tuantrantg
Copy link
Contributor Author

@tuantrantg Please see @rousseldenis comment

Hi @rousseldenis , the PR updated, please review it again. Thank you

@rousseldenis
Copy link
Sponsor Contributor

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

Hey, thanks for contributing! Proceeding to merge this for you.
Prepared branch 16.0-ocabot-merge-pr-693-by-rousseldenis-bump-nobump, awaiting test results.

OCA-git-bot added a commit that referenced this pull request Oct 3, 2023
Signed-off-by rousseldenis
@OCA-git-bot
Copy link
Contributor

@rousseldenis your merge command was aborted due to failed check(s), which you can inspect on this commit of 16.0-ocabot-merge-pr-693-by-rousseldenis-bump-nobump.

After fixing the problem, you can re-issue a merge command. Please refrain from merging manually as it will most probably make the target branch red.

@rousseldenis
Copy link
Sponsor Contributor

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

On my way to merge this fine PR!
Prepared branch 16.0-ocabot-merge-pr-693-by-rousseldenis-bump-nobump, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit 28ebba2 into OCA:16.0 Oct 3, 2023
7 checks passed
@OCA-git-bot
Copy link
Contributor

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

This pull request was closed.
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.