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_multi_destination: Migration to version 16.0 #766

Merged
merged 23 commits into from
Apr 1, 2024

Conversation

carolinafernandez-tecnativa
Copy link
Contributor

@carolinafernandez-tecnativa carolinafernandez-tecnativa commented Jan 25, 2024

@Tecnativa
TT46562

@pedrobaeza @victoralmau

pedrobaeza and others added 22 commits January 25, 2024 09:14
==================================================
Multiple destinations for the same delivery method
==================================================

Module `delivery` in version 8 allows to set different price rules depending
on the destination. This is what is called a delivery grid.

In version 9, for simplifying delivery methods, Odoo has plained the structure,
lowering destinations at delivery method level, and removing delivery grid
model.

This is not usable when you have different prices according the destination
of your delivery.

This module restores the same concept, reusing the same model for nesting
several "children" delivery methods, one per possible destination. It has been
designed to reuse all possible extensions to the base delivery, without the
need to create a glue module for having multiple destinations.

This module also handles if you're migrating from version 8 and you had
`delivery` module installed, to keep the delivery grids.

Installation
============

If you installed the module on a version 8 migrated database, some operations
will be done for recovering delivery grids. If so, you need to have
**openupgradelib** library installed.

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

To configure delivery methods with multiple destinations:

* Go to Inventory > Configuration > Delivery > Delivery Methods
* Create or edit an existing record.
* Select "Destination type" = "Multiple destinations".
* Introduce a line for each destination in the new tab "Destinations"
* Lines have priority, so you have to put first the lines with more restricted
  destinations.

Usage
=====

* When using the delivery method in a Sales order, delivery address will be
  used for computing the delivery price according introduced destinations.
OCA Transbot updated translations from Transifex
…#148)

* Don't show children carriers on many2one selections
* Don't search by default children carriers
* Add demo data

OCA Transbot updated translations from Transifex
* Standard procedure
* README by fragments
* Code changed to follow v11 logic
* Tests adapted and expanded
…lements

On a multi-destination carrier, that fields shouldn't be shown
…main one

When testing available carriers, we must return the main one if one of the
children matches, not returning the children itself.
…h multi

Handle the step of sending the shipping (triggered on picking validation) for
multi destination delivery. This means to manually check for fixing prices (as
the implementation doesn't check this part and always take the main carrier price,
not the subcarrier one.

Test for this use case done, although the rest of the cases are not covered by
tests yet.
[UPD] Update delivery_multi_destination.pot

[UPD] README.rst
[UPD] Update delivery_multi_destination.pot

[UPD] README.rst
When there is no carrier, destination_type is false and can cause error.
In the case, `one` destination_type should be used.

delivery_multi_destination 14.0.1.0.1
This reverts commit 0e28544.

This ismixing in the same commit other things for a non related problem that has
been fixed in a better way in the previous version, so we are reverting it and
applying the rest of the patches.
…sed on rules

If the destination carrier line is based on rules, the price is not
correctly fetched, as it's hardcoded to call `_get_price_available`
using picking's carrier, no matter the recordset from which you call it
(the self argument).

Thus, the only solution to get the proper value is to temporarily
replace the carrier on the picking on the calls chain, to restore it
before returning.

TT42862
… other companies

In the context where `carrier.child_ids` is being examined, all
existing subdestinations, no matter the company they have, are shown as
being in a sudo environment, so we need to filter them out those from
other companies.

TT43596
@carolinafernandez-tecnativa carolinafernandez-tecnativa force-pushed the 16.0-mig-delivery_multi_destination branch 4 times, most recently from 6c102ca to 5314a23 Compare January 25, 2024 14:02
@carolinafernandez-tecnativa carolinafernandez-tecnativa changed the title 16.0 mig delivery multi destination [16.0][MIG] delivery_multi_destination: Migration to version 16.0 Jan 25, 2024
@carolinafernandez-tecnativa carolinafernandez-tecnativa marked this pull request as ready for review January 25, 2024 14:12
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.

An issue I found in a functional test:

  • I create a multi destination carrier
  • With a filtered destination subcarrier
  • Then I go to a sale order with a partner no in the filtered destination
  • Force choosing that destination in the wizard:
Traceback (most recent call last):
  File "/opt/odoo/odoo/http.py", line 1591, in _serve_db
    return service_model.retrying(self._serve_ir_http, self.env)
  File "/opt/odoo/odoo/service/model.py", line 133, in retrying
    result = func()
  File "/opt/odoo/odoo/http.py", line 1618, in _serve_ir_http
    response = self.dispatcher.dispatch(rule.endpoint, args)
  File "/opt/odoo/odoo/http.py", line 1822, in dispatch
    result = self.request.registry['ir.http']._dispatch(endpoint)
  File "/opt/odoo/odoo/addons/base/models/ir_http.py", line 154, in _dispatch
    result = endpoint(**request.params)
  File "/opt/odoo/odoo/http.py", line 697, in route_wrapper
    result = endpoint(self, *args, **params_ok)
  File "/opt/odoo/addons/web/controllers/dataset.py", line 42, in call_kw
    return self._call_kw(model, method, args, kwargs)
  File "/opt/odoo/addons/web/controllers/dataset.py", line 33, in _call_kw
    return call_kw(request.env[model], method, args, kwargs)
  File "/opt/odoo/odoo/api.py", line 468, in call_kw
    result = _call_kw_multi(method, model, args, kwargs)
  File "/opt/odoo/odoo/api.py", line 453, in _call_kw_multi
    result = method(recs, *args, **kwargs)
  File "/opt/odoo/odoo/models.py", line 6571, in onchange
    record._onchange_eval(name, field_onchange[name], result)
  File "/opt/odoo/odoo/models.py", line 6282, in _onchange_eval
    method_res = method(self)
  File "/opt/odoo/addons/delivery/wizard/choose_delivery_carrier.py", line 32, in _onchange_carrier_id
    vals = self._get_shipment_rate()
  File "/opt/odoo/addons/delivery/wizard/choose_delivery_carrier.py", line 68, in _get_shipment_rate
    if vals.get('success'):
AttributeError: 'NoneType' object has no attribute 'get'

The above server error caused the following client error:
null

@rousseldenis
Copy link
Sponsor Contributor

/ocabot migration delivery_multi_destination

@carolinafernandez-tecnativa
Copy link
Contributor Author

An issue I found in a functional test:

  • I create a multi destination carrier
  • With a filtered destination subcarrier
  • Then I go to a sale order with a partner no in the filtered destination
  • Force choosing that destination in the wizard:
Traceback (most recent call last):
  File "/opt/odoo/odoo/http.py", line 1591, in _serve_db
    return service_model.retrying(self._serve_ir_http, self.env)
  File "/opt/odoo/odoo/service/model.py", line 133, in retrying
    result = func()
  File "/opt/odoo/odoo/http.py", line 1618, in _serve_ir_http
    response = self.dispatcher.dispatch(rule.endpoint, args)
  File "/opt/odoo/odoo/http.py", line 1822, in dispatch
    result = self.request.registry['ir.http']._dispatch(endpoint)
  File "/opt/odoo/odoo/addons/base/models/ir_http.py", line 154, in _dispatch
    result = endpoint(**request.params)
  File "/opt/odoo/odoo/http.py", line 697, in route_wrapper
    result = endpoint(self, *args, **params_ok)
  File "/opt/odoo/addons/web/controllers/dataset.py", line 42, in call_kw
    return self._call_kw(model, method, args, kwargs)
  File "/opt/odoo/addons/web/controllers/dataset.py", line 33, in _call_kw
    return call_kw(request.env[model], method, args, kwargs)
  File "/opt/odoo/odoo/api.py", line 468, in call_kw
    result = _call_kw_multi(method, model, args, kwargs)
  File "/opt/odoo/odoo/api.py", line 453, in _call_kw_multi
    result = method(recs, *args, **kwargs)
  File "/opt/odoo/odoo/models.py", line 6571, in onchange
    record._onchange_eval(name, field_onchange[name], result)
  File "/opt/odoo/odoo/models.py", line 6282, in _onchange_eval
    method_res = method(self)
  File "/opt/odoo/addons/delivery/wizard/choose_delivery_carrier.py", line 32, in _onchange_carrier_id
    vals = self._get_shipment_rate()
  File "/opt/odoo/addons/delivery/wizard/choose_delivery_carrier.py", line 68, in _get_shipment_rate
    if vals.get('success'):
AttributeError: 'NoneType' object has no attribute 'get'

The above server error caused the following client error:
null

I checked this and now it seems to work correctly, could you please check? Thanks!!

Copy link
Member

@pedrobaeza pedrobaeza left a comment

Choose a reason for hiding this comment

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

The dependency of the order of the assigned values is a bit weird, but let's not block this more.

/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-766-by-pedrobaeza-bump-nobump, awaiting test results.

OCA-git-bot added a commit that referenced this pull request Feb 8, 2024
Signed-off-by pedrobaeza
@carolinafernandez-tecnativa
Copy link
Contributor Author

ping @chienandalu

@pedrobaeza
Copy link
Member

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

What a great day to merge this nice PR. Let's do it!
Prepared branch 16.0-ocabot-merge-pr-766-by-pedrobaeza-bump-nobump, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit 4b67932 into OCA:16.0 Apr 1, 2024
5 of 7 checks passed
@OCA-git-bot
Copy link
Contributor

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

@pedrobaeza pedrobaeza deleted the 16.0-mig-delivery_multi_destination branch April 1, 2024 18:24
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.

None yet