[13.0] Add module sale_partner_delivery_window - alpha

[grindtildeath]

⚠️ ⚠️ ⚠️ ⚠️ ⚠️
Requires:

• [13.0][IMP] sale: Refactor calculation of expected_date on sale.order.line odoo/odoo#48322

Depends on:

• [13.0][ADD] base_time_window server-tools#1798
• [13.0] Add module partner_tz partner-contact#876

This module allows to define scheduling preference for delivery orders on
customers, in order to select possible delivery windows to postpone deliveries to.

As the delivery schedule is a preference, the schedule date of delivery orders
can always be changed to a datetime that is not preferred by the customer either
by setting a commitment date on sale order or by changing the date on the
picking.

On partners form view, under the "Sales & Purchases" tab, one can define a
"Delivery schedule preference" for each partner.

Possible configurations are:

• Any time: Do not postpone deliveries
• Fixed time windows: Postpone deliveries to the next preferred time window

After selecting "Fixed time windows", one can define the preferred delivery
windows in the embedded tree view below.

[simahawk]

use the serialized fields ;)

[bealdav]

Hi @grindtildeath thanks for this nice feature.
On our side we consider to contribute on such feature on purchase side (v14).
Don't you think it could be possible to store weekdays in a module like partner_weekday ?
Partners can use this checkbox according they are customer ou supplier

[grindtildeath]

@bealdav I am (or we are at camptocamp) always in favour of pooling features into a same module. But I guess if we use it here will depend on the implementation choices you take.

[jgrandguillaume]

Hi @bealdav

I like the idea, but how to handle a partner that is both a supplier and a customer here ? May be we do not support this in v1 ?

Regards,

Joël

[jgrandguillaume]

@bealdav We do this :)

[bealdav]

Really good question I thought too.
This event should be very rare but can occurs

Partners that could have needs this feature can be goods suppliers, carriers, customers, dropoff sites.
But to be in 2 cases as current main business flow could not occured.

If it could, it seems this kind of model is more appropriate

https://github.com/OCA/delivery-carrier/blob/12.0/partner_delivery_schedule/models/partner_delivery_schedule.py
https://github.com/OCA/delivery-carrier/blob/12.0/partner_delivery_schedule/models/res_partner.py

But not generic enough because limited to delivery

I don't know what is the best ?

[lmignon]

Hi,

For one of our customer, I implemented the same kind of requirements. My UC is to be able to specify some delivery windows on the partner. Here it's the models I implemented . I cant propose all these devs fully tested into a dedicated addon (v10)...

# -*- coding: utf-8 -*-
# Copyright 2020 ACSONE SA/NV
# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl).

from odoo import _, api, fields, models, tools


class DeliveryWeekDay(models.Model):

    _name = "delivery.week.day"
    _description = "Delivery Week Day"

    name = fields.Selection(
        selection=[
            ("0", "Monday"),
            ("1", "Tuesday"),
            ("2", "Wednesday"),
            ("3", "Thursday"),
            ("4", "Friday"),
            ("5", "Saturday"),
            ("6", "Sunday"),
        ],
        required=True,
    )
    _sql_constraints = [
        ("name_uniq", "UNIQUE(name)", _("Name must be unique"))
    ]

    @api.depends("name")
    def _compute_display_name(self):
        """
        WORKAROUND since Odoo doesn't handle properly records where name is
        a selection
        """
        translated_values = dict(
            self._fields["name"]._description_selection(self.env)
        )
        for record in self:
            record.display_name = translated_values[record.name]

    @api.multi
    def name_get(self):
        """
        WORKAROUND since Odoo doesn't handle properly records where name is
        a selection
        """
        return [(r.id, r.display_name) for r in self]

    @api.model
    @tools.ormcache("name")
    def _get_id_by_name(self, name):
        return self.search([("name", "=", name)], limit=1).id

    @api.model
    def create(self, vals):
        result = super(DeliveryWeekDay, self).create(vals)
        self._get_id_by_name.clear_cache(self)
        return result

    @api.multi
    def write(self, vals):
        result = super(DeliveryWeekDay, self).write(vals)
        self._get_id_by_name.clear_cache(self)
        return result

    @api.multi
    def unlink(self):
        result = super(DeliveryWeekDay, self).unlink()
        self._get_id_by_name.clear_cache(self)
        return result

# -*- coding: utf-8 -*-
# Copyright 2020 ACSONE SA/NV
# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl).

import math

from psycopg2.extensions import AsIs

from odoo import _, api, fields, models
from odoo.exceptions import ValidationError


class DeliveryWindow(models.Model):

    _name = "delivery.window"
    _description = "Delivery Window"
    _order = "partner_id, start"

    start = fields.Float("From", required=True)
    end = fields.Float("To", required=True)
    week_day_ids = fields.Many2many(
        comodel_name="alc.delivery.week.day", required=True
    )
    partner_id = fields.Many2one(
        "res.partner", required=True, index=True, ondelete='cascade'
    )

    @api.constrains("start", "end", "week_day_ids")
    def check_window_no_onverlaps(self):
        week_days_field = self._fields["week_day_ids"]
        for record in self:
            if record.start > record.end:
                raise ValidationError(
                    _("%s must be > %s")
                    % (
                        self.float_to_time_repr(record.end),
                        self.float_to_time_repr(record.start),
                    )
                )
            # here we use a plain SQL query to benefit of the numrange
            # function available in PostgresSQL
            # (http://www.postgresql.org/docs/current/static/rangetypes.html)
            SQL = """
                SELECT
                    id
                FROM
                    %(table)s w
                    join %(relation)s as d
                    on d.%(relation_window_fkey)s = w.id
                WHERE
                    NUMRANGE(w.start::numeric, w.end::numeric) &&
                        NUMRANGE(%(start)s::numeric, %(end)s::numeric)
                    AND w.id != %(window_id)s
                    AND d.%(relation_week_day_fkey)s in %(week_day_ids)s
                    AND w.partner_id = %(partner_id)s"""
            self.env.cr.execute(
                SQL,
                dict(
                    table=AsIs(self._table),
                    relation=AsIs(week_days_field.relation),
                    relation_window_fkey=AsIs(week_days_field.column1),
                    relation_week_day_fkey=AsIs(week_days_field.column2),
                    start=record.start,
                    end=record.end,
                    window_id=record.id,
                    week_day_ids=tuple(record.week_day_ids.ids),
                    partner_id=record.partner_id.id,
                ),
            )
            res = self.env.cr.fetchall()
            if res:
                other = self.browse(res[0][0])
                raise ValidationError(
                    _("%s overlaps %s")
                    % (record.display_name, other.display_name)
                )

    @api.depends("start", "end", "week_day_ids")
    def _compute_display_name(self):
        for record in self:
            "{days}: From {start} to {end}".format(
                days=", ".join(record.week_day_ids.mapped("display_name")),
                start=self.float_to_time_repr(record.start),
                end=self.float_to_time_repr(record.end),
            )

    @api.model
    def float_to_time_repr(self, value):
        pattern = "%02d:%02d"
        hour = math.floor(value)
        min = round((value % 1) * 60)
        if min == 60:
            min = 0
            hour += 1

        return pattern % (hour, min)

# -*- coding: utf-8 -*-
# Copyright 2020 ACSONE SA/NV
# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl).

from collections import defaultdict

from odoo import fields, models


class ResPartner(models.Model):

    _inherit = "res.partner"

    delivery_window_ids = fields.One2many(
        comodel_name="delivery.window",
        inverse_name="partner_id",
        string="Delivery windows",
        help="If specified, delivery is only possible into the specified "
        "time windows. (Leaves empty if no restriction)",
    )

    def get_delivery_windows(self, day_name):
        """
        Return the list of delivery windows by partner id for the given day

        :param day: The day name (see delivery.week.day, ex: 0,1,2,...)
        :return: dict partner_id:[delivery_window, ]
        """
        week_day_id = self.env["delivery.week.day"]._get_id_by_name(
            day_name
        )
        res = defaultdict(list)
        windows = self.env["delivery.window"].search(
            [
                ("partner_id", "in", self.ids),
                ("week_day_ids", "in", week_day_id),
            ]
        )
        for window in windows:
            res[window.partner_id.id].append(window)
        return res

We could generalize these models and define a more generic concept: 'partner.time.window' and on this 'partner.time.window' add a field "usage" to specify if a time window is for delivery, .....

If we agree on this, I can provide this new addon.

[grindtildeath]

@lmignon Thanks for your input.
We discussed this morning with @jgrandguillaume, and I will rework the feature tomorrow in order to use a more generic approach and to allow time windows during a day as well.
From what we discussed it would look very close to what you did, but we'd favour using a Selection field for days in the model you called delivery.window instead of using another relation. Or is there a particular reason that made you choose a dedicated model?
Also we can definitely add a usage field if we do a kind of "base" module for this.

[lmignon]

From what we discussed it would look very close to what you did, but we'd favour using a Selection field for days in the model you called delivery.window instead of using another relation. Or is there a particular reason that made you choose a dedicated model?

There are two main motivations behind this design:

• usability/maintainability: When you specify a time window it is easier to specify the days to which this applies than to have to repeat the same information for each day. (If the schedule changes, only one line is changed).
• flexibility: It's easy with this approach to remove a day from the list of available days or to add an other unity.

I can provide all the unittest for this implementation...

[bealdav]

I'm fully agree with all you're talking.

On the vocabulary side, I would switch a more generic word : availability.window
because you may want reference that your partner is available to step in your company these weekdays only (field services or any) not necessarily to delivery something.
What do you think ?

[lmignon]

I'm fully agree with all you're talking.

On the vocabulary side, I would switch a more generic word : availability.window
because you may want reference that your partner is available to step in your company these weekdays only (field services or any) not necessarily to delivery something.
What do you think ?

IMO availability is too restrictive. Imagine that I want to use theses models to express unavailability 😏

[bealdav]

@lmignon I'm sure u understood what I mean, then I let you define the adhoc name. Good luck
But for any help it can be a rescue https://www.lerobert.com ;-)

[jgrandguillaume]

let's call it weekday.time.windows ;)

cc @lmignon @grindtildeath

[grindtildeath]

@lmignon These are two valid points, so let's use your implementation 👍

I'll submit a PR for this base module tomorrow by cherry-picking your class files here, and we can still debate about the appropriate wording afterwards 😅

[lmignon]

@grindtildeath Thank you for making this code a generic module! Here it's the unittest... (this source code is into a private repos..)

@jgrandguillaume weekday.time.window or time.window are both fine for me.

# -*- coding: utf-8 -*-
# Copyright 2020 ACSONE SA/NV
# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl).

from odoo.exceptions import ValidationError
from odoo.tests import SavepointCase


class TestDeliveryWindow(SavepointCase):
    @classmethod
    def setUpClass(cls):
        super(TestDeliveryWindow, cls).setUpClass()
        cls.partner_1 = cls.env['res.partner'].create({'name': 'partner 1'})
        cls.partner_2 = cls.env['res.partner'].create({'name': 'patner 2'})
        cls.DeliveryWindow = cls.env["delivery.window"]
        cls.monday = cls.env.ref(
            "partner_delivery_window.delivery_weed_day_monday"
        )
        cls.sunday = cls.env.ref(
            "partner_delivery_window.delivery_weed_day_sunday"
        )

    def test_00(self):
        """
        Data:
            A partner without delivery window
        Test Case:
            Add a delivery window
        Expected result:
            A delivery window is created for the partner
        """

        self.assertFalse(self.partner_1.delivery_window_ids)
        self.DeliveryWindow.create(
            {
                "partner_id": self.partner_1.id,
                "start": 10.0,
                "end": 12.0,
                "week_day_ids": [(4, self.monday.id)],
            }
        )
        self.assertTrue(self.partner_1.delivery_window_ids)
        delivery_window = self.partner_1.delivery_window_ids
        self.assertEqual(delivery_window.start, 10.0)
        self.assertEqual(delivery_window.end, 12.0)
        self.assertEqual(delivery_window.week_day_ids, self.monday)

    def test_01(self):
        """
        Data:
            A partner without delivery window
        Test Case:
            1 Add a delivery window
            2 unlink the partner
        Expected result:
            1 A delivery window is created for the partner
            2 The delivery window is removed
        """
        partner_id = self.partner_1.id
        self.assertFalse(self.partner_1.delivery_window_ids)
        self.DeliveryWindow.create(
            {
                "partner_id": self.partner_1.id,
                "start": 10.0,
                "end": 12.0,
                "week_day_ids": [(4, self.monday.id)],
            }
        )
        self.assertTrue(self.partner_1.delivery_window_ids)
        delivery_window = self.DeliveryWindow.search(
            [("partner_id", "=", partner_id)]
        )
        self.assertTrue(delivery_window)
        self.partner_1.unlink()
        self.assertFalse(delivery_window.exists())

    def test_02(self):
        """
        Data:
            A partner without delivery window
        Test Case:
            1 Add a delivery window
            2 Add a second delivery window that overlaps the first one (same day)
        Expected result:
            1 A delivery window is created for the partner
            2 ValidationError is raised
        """
        self.DeliveryWindow.create(
            {
                "partner_id": self.partner_1.id,
                "start": 10.0,
                "end": 12.0,
                "week_day_ids": [(4, self.monday.id)],
            }
        )
        with self.assertRaises(ValidationError):
            self.DeliveryWindow.create(
                {
                    "partner_id": self.partner_1.id,
                    "start": 11.0,
                    "end": 13.0,
                    "week_day_ids": [(4, self.monday.id), (4, self.sunday.id)],
                }
            )

    def test_03(self):
        """
        Data:
            A partner without delivery window
        Test Case:
            1 Add a delivery window
            2 Add a second delivery window that overlaps the first one (another day)
        Expected result:
            1 A delivery window is created for the partner
            2 A second  delivery window is created for the partner
        """
        self.assertFalse(self.partner_1.delivery_window_ids)
        self.DeliveryWindow.create(
            {
                "partner_id": self.partner_1.id,
                "start": 10.0,
                "end": 12.0,
                "week_day_ids": [(4, self.monday.id)],
            }
        )
        self.assertTrue(self.partner_1.delivery_window_ids)
        self.DeliveryWindow.create(
            {
                "partner_id": self.partner_1.id,
                "start": 11.0,
                "end": 13.0,
                "week_day_ids": [(4, self.sunday.id)],
            }
        )
        self.assertEquals(len(self.partner_1.delivery_window_ids), 2)

    def test_04(self):
        """
        Data:
            Partner 1 without delivery window
            Partner 2 without delivery window
        Test Case:
            1 Add a delivery window to partner 1
            2 Add the same delivery window to partner 2
        Expected result:
            1 A delivery window is created for the partner 1
            1 A delivery window is created for the partner 2
        """
        self.assertFalse(self.partner_1.delivery_window_ids)
        self.DeliveryWindow.create(
            {
                "partner_id": self.partner_1.id,
                "start": 10.0,
                "end": 12.0,
                "week_day_ids": [(4, self.monday.id)],
            }
        )
        self.assertTrue(self.partner_1.delivery_window_ids)
        self.assertFalse(self.partner_2.delivery_window_ids)
        self.DeliveryWindow.create(
            {
                "partner_id": self.partner_2.id,
                "start": 10.0,
                "end": 12.0,
                "week_day_ids": [(4, self.monday.id)],
            }
        )
        self.assertTrue(self.partner_2.delivery_window_ids)

    def test_05(self):
        """""
        Data:
            Partner 1 without delivery window
        Test Case:
            Add a delivery window to partner 1 with end > start
        Expected result:
            ValidationError is raised
        """
        with self.assertRaises(ValidationError):
            self.DeliveryWindow.create(
                {
                    "partner_id": self.partner_1.id,
                    "start": 14.0,
                    "end": 12.0,
                    "week_day_ids": [(4, self.monday.id)],
                }
            )

the data file

<?xml version="1.0" encoding="utf-8"?>
<!-- Copyright 2020 ACSONE SA/NV
     License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl). -->

<odoo noupdate="1">

    <record model="delivery.week.day" id="delivery_weed_day_monday">
        <field name="name">0</field>
    </record>

    <record model="delivery.week.day" id="delivery_weed_day_tuesday">
        <field name="name">1</field>
    </record>

    <record model="delivery.week.day" id="delivery_weed_day_wednesday">
        <field name="name">2</field>
    </record>

    <record model="delivery.week.day" id="delivery_weed_day_thursday">
        <field name="name">3</field>
    </record>

    <record model="delivery.week.day" id="delivery_weed_day_friday">
        <field name="name">4</field>
    </record>

    <record model="delivery.week.day" id="delivery_weed_day_saturday">
        <field name="name">5</field>
    </record>

    <record model="delivery.week.day" id="delivery_weed_day_sunday">
        <field name="name">6</field>
    </record>
</odoo>

and the Alcs

<?xml version="1.0" encoding="utf-8"?>
<!-- Copyright 2020 ACSONE SA/NV
     License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl). -->

<odoo>
    <record model="ir.model.access" id="delivery_week_day_access_read">
        <field name="name">delivery.window.day access read</field>
        <field name="model_id" ref="model_delivery_week_day"/>
        <field name="group_id" ref="base.group_user"/>
        <field name="perm_read" eval="1"/>
        <field name="perm_create" eval="0"/>
        <field name="perm_write" eval="0"/>
        <field name="perm_unlink" eval="0"/>
    </record>

    <record model="ir.model.access" id="delivery_window_access_read">
        <field name="name">delivery.window access read</field>
        <field name="model_id" ref="model_delivery_window"/>
        <field name="group_id" ref="base.group_user"/>
        <field name="perm_read" eval="1"/>
        <field name="perm_create" eval="0"/>
        <field name="perm_write" eval="0"/>
        <field name="perm_unlink" eval="0"/>
    </record>

    <record model="ir.model.access" id="delivery_window_access_manage">
        <field name="name">delivery.window access read</field>
        <field name="model_id" ref="model_delivery_window"/>
        <field name="group_id" ref="sales_team.group_sale_manager"/>
        <field name="perm_read" eval="1"/>
        <field name="perm_create" eval="1"/>
        <field name="perm_write" eval="1"/>
        <field name="perm_unlink" eval="1"/>
    </record>
</odoo>

[grindtildeath]

I'm not finished yet, but here is the WIP:

• Base module for time windows providing a mixin for any usage one might need of it: [13.0][ADD] base_time_window server-tools#1798
• New module stock_partner_delivery_window implementing model partner.delivery.time.window to avoid hard dependency on sale: [13.0][ADD] stock_partner_delivery_window stock-logistics-workflow#642
• Renamed this module to sale_partner_delivery_window to add dependency on sale and extra computation.

I'll polish a bit the PRs tomorrow, but feel free to comment.

Again thanks @lmignon for the excerpts 😉

[lmignon]

@grindtildeath Thank you for your work

[jbaudoux]

The date_planned is the date of the shipping, not the customer expected date.
delivery_date = date_planned + timedelta(days=self.order_id.company_id.security_lead)
That later is the date you need to check with delivery window.
Then you have to subtract back the security lead from next_preferred_date for the comparison with date_planned

[jbaudoux]

Starting from here you should convert from_date that is in UTC to partner TZ

[grindtildeath]

I guess this must be fixed with OCA/stock-logistics-workflow#642 (comment)

[jbaudoux]

limit=1

[jbaudoux]

Suggested change

	from_date, from_date + timedelta(days=8)
	from_date, from_date + timedelta(days=7)

date_range already add a day. So current weekday is already present at the beginning and at the end

[jbaudoux]

Why computing for a predefined range. Why not try each day after each other until you match a valid one? This could be more convenient to hook with another module that would define leave periods (holidays).

[grindtildeath]

IMO with the actual implementation, there's not much sense to compute opening times for more than a week, but I moved the number of days to function arguments so that it could be overrided as needed.
However I would strongly advise against managing holidays the same way we managed times, and we should trend to use resource.calendar, what will need a consequent refactoring.

[jbaudoux]

Ensure conversion back to UTC

[grindtildeath]

This should also be fixed by OCA/stock-logistics-workflow#642 (comment)

[jbaudoux]

Wrongly clicked "Approve" while meaning "Request changes"

[grindtildeath]

@jbaudoux As I couldn't fix the tests yet, I'll push the fixing commit tomorrow to avoid leaving this PR in a worse state 😅

Serialized field has been discarded, too complex at this stage

[sebalix]

Tested locally: if the partner is still not defined on the SO while we enter SO lines, the following error is raised:

[...]
  File "/odoo/src/addons/sale_stock/models/sale_order.py", line 53, in _compute_expected_date
    super(SaleOrder, self)._compute_expected_date()
  File "/odoo/src/addons/sale/models/sale.py", line 294, in _compute_expected_date
    dt = line._expected_date()
  File "/odoo/external-src/sale-workflow/sale_partner_delivery_window/models/sale_order.py", line 73, in _expected_date
    return partner.next_delivery_window_start_datetime(from_date=expected_date)
  File "/odoo/external-src/sale-workflow/sale_partner_delivery_window/models/res_partner.py", line 23, in next_delivery_window_start_datetime
    self.ensure_one()
  File "/odoo/src/odoo/models.py", line 5003, in ensure_one
    raise ValueError("Expected singleton: %s" % self)
ValueError: Expected singleton: res.partner()

[jgrandguillaume]

LGTM

[simahawk]

replaced by #1377