[IMP/REF] *: pickup point delivery cleanup #160187

mohammadhossam · 2024-04-02T15:08:58Z

*: delivery, sale, sale_management, sale_stock, stock_delivery, website_sale

This PR introduces several changes and improvements to pickup point delivery feature.

Behavior before this PR:

Users could choose a pickup address on checkout, if a proper delivery method is enabled.
Users could only see pickup locations close to their registered address.
Data of this address is attached to the sales order in a json field and the delivery address is set to the customer's address as in normal delivery.
On confirming the sale order, a new address for the pickup point is created as a child to the customer's contact. Its data is retrieved from the above mentioned json field.
On confirming the delivery order, the address data to be passed to the shipping provider to process the shipment is retrieved from the related sales order (i.e. pickup delivery orders must have a related sales order).
It was not possible to change the pickup address from the backend after the user confirms the order.

Behavior after this PR:

The json field is removed from the sales order.
Users can search for any pickup address by postal code, not necessarily close to their address.
When the user confirms an order with a pickup delivery, a new address is created with the point data and is attached to the customer as a child address (if there's no existing address with the same data).
A new json field is added to res.partner to capture the info needed by the shipping provider later when processing the shipment.
Pickup address can be changed from sales/delivery order through a special wizard that can be opened only when the delivery is pickup.
Pickup addresses can be searched by postal code from this wizard to enable getting all points from the provider, not necessarily ones close to the customer.

Enterprise PR: odoo/enterprise#59935
Upgrade PR: odoo/upgrade#5920

Task-3645144

I confirm I have signed the CLA and read the PR guidelines at www.odoo.com/submit-pr

robodoo · 2024-04-02T15:10:37Z

Pull request status dashboard.

amoyaux · 2024-04-26T08:56:06Z

addons/delivery/static/src/js/fedex_locations_template.js

+
+import { Component } from "@odoo/owl";
+
+export class FedexLocations extends Component {


Those template should be in enterprise no?

addons/delivery/models/sale_order.py

amoyaux

The comment I put are only some details. However there is a bigger issue in the current state. It would be great to have a single template for each carrier.
The idea would be to define a set of variable in the ChoosePickupPointForm and the server would have to do the matching between data received by the carrier and the date required by the form view (so in carrier_get_close_location)

That way we will avoid the annoying part of creating a new template by carrier and the majority of the code would be in 2 functions. The ones that receive data from the carrier and format it to a single syntax used by our client. And the other part that receive the formated data of our client to the carrier when we ask for a rate or a shipment

amoyaux · 2024-04-26T09:00:33Z

addons/delivery/models/res_partner.py

@@ -7,3 +7,4 @@ class ResPartner(models.Model):
    _inherit = 'res.partner'

    property_delivery_carrier_id = fields.Many2one('delivery.carrier', company_dependent=True, string="Delivery Method", help="Default delivery method used in sales orders.")
+    external_id = fields.Json(string="External Identifier", help="Information needed by shipping providers in case of pickup point addresses.")


pickup_point_address no? is it really an id stored in it?

It is different from a provider to another. In case of Sendcloud, it's just and id used by the provider later to process the shipment when the delivery is validated. In case of Fedex, it's an id in addition to other fields.

addons/delivery/models/sale_order.py

amoyaux · 2024-04-26T09:01:08Z

addons/delivery/models/sale_order.py

@@ -25,6 +34,24 @@ def _compute_amount_total_without_delivery(self):
        delivery_cost = sum([l.price_total for l in self.order_line if l.is_delivery])
        return self.amount_total - delivery_cost

+    @api.depends('carrier_id')
+    def _compute_shipping_method(self):


Should we really need a compute for it?

I.M.O it's not because the carrier allow to select a pickup point that we have to define it as a default

As far as I understand, if the delivery carrier has <carrier>_use_locations field and if it is true at the same time, then it's always used as a pickup delivery. Isn't this what happens here or am I missing something?

addons/delivery/models/sale_order.py

amoyaux · 2024-04-26T09:03:08Z

addons/delivery/models/sale_order.py

@@ -88,6 +115,23 @@ def action_open_delivery_wizard(self):
            }
        }

+    def action_open_pickup_point_wizard(self):
+        if not self.partner_shipping_id.country_id:


Only allow it if the carrier allow to use pickup point

Alright, but isn't it enough to hide the button using this action when the carrier is not applicable?

amoyaux · 2024-04-26T09:05:30Z

addons/delivery/static/src/js/sendcloud_locations_template.js

+
+import { Component } from "@odoo/owl";
+
+export class SendcloudLocations extends Component {


should be in sendcloud module

amoyaux · 2024-04-26T09:06:40Z

addons/delivery/static/src/xml/choose_pickup_point_form.xml

+                        <t t-if="this.carrier[0].delivery_type === 'sendcloud'">
+                            <SendcloudLocations locations="this.pickupPoints.data" showDistance="!this.searchByZipCode.value" onSelectPickupPoint.bind="this.onSelectPickupPoint"/>
+                        </t>
+                        <t t-else="">
+                            <FedexLocations locations="this.pickupPoints.data" onSelectPickupPoint.bind="this.onSelectPickupPoint"/>
+                        </t>


Should be in appropriate module. Maybe the slot could be useful in order to extend that

This commit adds the ability to search for pickup points to choose from in shipment methods provided by both Sendcloud and FedEx. Before, users could only see pickup points close to their registered address. Task-3645144

*: delivery, sale, sale_management, sale_stock, stock_delivery, website_sale This commit introduces several changes and improvements to pickup point delivery feature. ** Behavior before this commit: - Users could choose a pickup address on checkout, if a proper delivery method is enabled. - Data of this address is attached to the sales order in a json field and the delivery address is set to the customer's address as in normal delivery. - On confirming the sale order, a new address for the pickup point is created as a child to the customer's contact. Its data is retrieved from the above mentioned json field. - On confirming the delivery order, the address data to be passed to the shipping provider to process the shipment is retrieved from the related sales order (i.e. pickup delivery orders _must_ have a related sales order). - It was not possible to change the pickup address from the backend after the user confirms the order. ** Behavior after this commit: - The json field is removed from the sales order. - When the user confirms an order with a pickup delivery, a new address is created with the point data and is attached to the customer as a child address (if there's no existing address with the same data). - A new json field is added to `res.partner` to capture the info needed by the shipping provider later when processing the shipment. - Pickup address can be changed from sales/delivery order through a special wizard that can be opened only when the delivery is pickup. - Pickup addresses can be searched by postal code from this wizard to enable getting all points from the provider, not necessarily ones close to the customer. Task-3645144

mohammadhossam changed the title ~~[IMP] website_sale: enable searching by zip code in pickup deliveries~~ [IMP] website_sale: pickup point delivery cleanup Apr 2, 2024

C3POdoo added the RD research & development, internal work label Apr 2, 2024

mohammadhossam force-pushed the master-pickup-point-cleanup-abdu branch 2 times, most recently from 30378d2 to 59373ab Compare April 8, 2024 14:47

mohammadhossam changed the title ~~[IMP] website_sale: pickup point delivery cleanup~~ [IMP] *: pickup point delivery cleanup Apr 8, 2024

mohammadhossam force-pushed the master-pickup-point-cleanup-abdu branch from 59373ab to 9b2fd96 Compare April 8, 2024 14:58

mohammadhossam changed the title ~~[IMP] *: pickup point delivery cleanup~~ [IMP/REF] *: pickup point delivery cleanup Apr 8, 2024

mohammadhossam force-pushed the master-pickup-point-cleanup-abdu branch 4 times, most recently from cf30690 to a04f23d Compare April 15, 2024 13:14

mohammadhossam force-pushed the master-pickup-point-cleanup-abdu branch 3 times, most recently from a4cbff6 to 8d10ed6 Compare April 24, 2024 14:22

amoyaux reviewed Apr 26, 2024

View reviewed changes

addons/delivery/models/sale_order.py Outdated Show resolved Hide resolved

amoyaux requested changes Apr 26, 2024

View reviewed changes

mohammadhossam force-pushed the master-pickup-point-cleanup-abdu branch from 8d10ed6 to ab29202 Compare April 26, 2024 14:51

mohammadhossam force-pushed the master-pickup-point-cleanup-abdu branch 5 times, most recently from f114cef to 05e9012 Compare April 26, 2024 15:20

mohammadhossam force-pushed the master-pickup-point-cleanup-abdu branch from 05e9012 to d967636 Compare April 26, 2024 15:50

mohammadhossam requested a review from amoyaux April 29, 2024 08:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[IMP/REF] *: pickup point delivery cleanup #160187

[IMP/REF] *: pickup point delivery cleanup #160187

mohammadhossam commented Apr 2, 2024 •

edited

robodoo commented Apr 2, 2024

amoyaux Apr 26, 2024

amoyaux left a comment

amoyaux Apr 26, 2024

mohammadhossam Apr 26, 2024

amoyaux Apr 26, 2024

amoyaux Apr 26, 2024

mohammadhossam Apr 26, 2024 •

edited

amoyaux Apr 26, 2024

mohammadhossam Apr 26, 2024

amoyaux Apr 26, 2024

amoyaux Apr 26, 2024


		import { Component } from "@odoo/owl";

		export class FedexLocations extends Component {


		import { Component } from "@odoo/owl";

		export class SendcloudLocations extends Component {

[IMP/REF] *: pickup point delivery cleanup #160187

Are you sure you want to change the base?

[IMP/REF] *: pickup point delivery cleanup #160187

Conversation

mohammadhossam commented Apr 2, 2024 • edited

Behavior before this PR:

Behavior after this PR:

robodoo commented Apr 2, 2024

Choose a reason for hiding this comment

amoyaux left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mohammadhossam Apr 26, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mohammadhossam commented Apr 2, 2024 •

edited

mohammadhossam Apr 26, 2024 •

edited