[IMP] membership_[extension,type,custom_date]: Add restraint on date_…

…from and date_to on membership line

This was a bit tricky to get right, architecturally, but I think I
arrived at the correct design decision. On the whole, the new
restriction makes sense. Following from the fact that dates are
mandatory on 'fixed dates' (read: default) membership products, and
following from the fact that dates are always set on the membership
lines of 'variable period' membership products, membership lines should
_always_ have values for date_from and date_to. The fact that these
fields are not already required is a strange design decision.

The constraint in membership_extension _effectively_ sets required=True
on these fields.

However, there is a use-case for memberships that do not have an end
date (using the 'custom' membership product). For those scenarios, we
add a toggle on the product (exclusive to 'custom' products) on whether
to apply the constraint.

Signed-off-by: Carmen Bianca BAKKER <[email protected]>

membership_custom_date/__manifest__.py

@@ -20,6 +20,7 @@
     ],
     "excludes": [],
     "data": [
+        "views/product_template_views.xml",
         "wizard/membership_invoice_views.xml",
     ],
     "demo": [],

membership_custom_date/models/__init__.py

@@ -3,3 +3,4 @@
 # SPDX-License-Identifier: AGPL-3.0-or-later
 
 from . import product_template
+from . import membership_membership_line

membership_custom_date/models/membership_membership_line.py

@@ -0,0 +1,24 @@
+# SPDX-FileCopyrightText: 2023 Coop IT Easy SC
+#
+# SPDX-License-Identifier: AGPL-3.0-or-later
+
+from odoo import api, fields, models
+
+
+class MembershipLine(models.Model):
+    _inherit = "membership.membership_line"
+
+    dates_mandatory = fields.Boolean(related="membership_id.dates_mandatory")
+
+    @api.constrains(
+        "membership_id",
+        "membership_type",
+        "dates_mandatory",
+        "date_from",
+        "date_to",
+    )
+    def _check_mandatory_dates(self):
+        lines = self.filtered(
+            lambda line: line.membership_type != "custom" or line.dates_mandatory
+        )
+        return super(MembershipLine, lines)._check_mandatory_dates()

membership_custom_date/models/product_template.py

@@ -14,10 +14,18 @@ class ProductTemplate(models.Model):
         ],
         ondelete={"custom": "set default"},
     )
+    dates_mandatory = fields.Boolean(
+        string="Mandatory dates",
+        default=True,
+        help="Membership lines that use this product must have a start and end date.",
+    )
 
     @api.model
     def _correct_vals_membership_type(self, vals):
         vals = super()._correct_vals_membership_type(vals)
         if vals.get("membership_type") == "custom":
             vals["membership_date_from"] = vals["membership_date_to"] = False
+        # TODO: should we set dates_mandatory back to True if switching away
+        # from custom? At the moment this variable is not used _at all_ for
+        # non-custom memberships.
         return vals

membership_custom_date/tests/__init__.py

@@ -0,0 +1,5 @@
+# SPDX-FileCopyrightText: 2023 Coop IT Easy SC
+#
+# SPDX-License-Identifier: AGPL-3.0-or-later
+
+from . import test_membership

membership_custom_date/tests/test_membership.py

@@ -0,0 +1,114 @@
+# SPDX-FileCopyrightText: 2023 Coop IT Easy SC
+#
+# SPDX-License-Identifier: AGPL-3.0-or-later
+
+
+from odoo import fields
+from odoo.exceptions import ValidationError
+from odoo.tests.common import Form, TransactionCase
+
+
+class TestMembership(TransactionCase):
+    @classmethod
+    def setUpClass(cls):
+        super().setUpClass()
+        cls.product = cls.env["product.product"].create(
+            {
+                "type": "service",
+                "name": "Custom Membership",
+                "membership": True,
+                "membership_type": "custom",
+                "list_price": 100,
+                "dates_mandatory": True,
+            }
+        )
+        cls.partner = cls.env["res.partner"].create({"name": "Test Partner"})
+
+    def test_mandatory_dates(self):
+        with self.assertRaises(ValidationError):
+            self.env["membership.membership_line"].create(
+                {
+                    "membership_id": self.product.id,
+                    "member_price": 100.00,
+                    "date": fields.Date.today(),
+                    # No dates
+                    "partner": self.partner.id,
+                    "state": "waiting",
+                }
+            )
+        self.product.dates_mandatory = False
+        self.env["membership.membership_line"].create(
+            {
+                "membership_id": self.product.id,
+                "member_price": 100.00,
+                "date": fields.Date.today(),
+                # No dates
+                "partner": self.partner.id,
+                "state": "waiting",
+            }
+        )
+
+    def test_check_membership_dates_required(self):
+        invoice_form = Form(
+            self.env["membership.invoice"].with_context(
+                default_partner_id=self.partner.id
+            )
+        )
+        invoice_form.product_id = self.product
+
+        message = "is a required field"
+
+        invoice_form.date_from = False
+        invoice_form.date_to = False
+        with self.assertRaisesRegex(AssertionError, message):
+            invoice_form.save()
+
+        invoice_form.date_from = "2023-01-01"
+        invoice_form.date_to = False
+        with self.assertRaisesRegex(AssertionError, message):
+            invoice_form.save()
+
+        invoice_form.date_from = False
+        invoice_form.date_to = "2023-12-31"
+        with self.assertRaisesRegex(AssertionError, message):
+            invoice_form.save()
+
+        invoice_form.date_from = "2023-01-01"
+        invoice_form.date_to = "2023-12-31"
+        invoice_form.save()
+
+    def test_check_membership_invoice_check_dates(self):
+        wizard = self.env["membership.invoice"].create(
+            {
+                "product_id": self.product.id,
+                "member_price": 100,
+            }
+        )
+        wizard = wizard.with_context(
+            default_partner_id=self.partner.id,
+            active_ids=self.partner.id,
+        )
+
+        self.assertEqual(wizard.product_id, self.product)
+
+        message = "start and end dates are mandatory"
+
+        # already the case, but to be explicit here
+        wizard.date_from = False
+        wizard.date_to = False
+        with self.assertRaisesRegex(ValidationError, message):
+            wizard.membership_invoice()
+
+        wizard.date_from = "2023-01-01"
+        wizard.date_to = False
+        with self.assertRaisesRegex(ValidationError, message):
+            wizard.membership_invoice()
+
+        wizard.date_from = False
+        wizard.date_to = "2023-12-31"
+        with self.assertRaisesRegex(ValidationError, message):
+            wizard.membership_invoice()
+
+        wizard.date_from = "2023-01-01"
+        wizard.date_to = "2023-12-31"
+        wizard.membership_invoice()

membership_custom_date/views/product_template_views.xml

@@ -0,0 +1,20 @@
+<?xml version="1.0" encoding="utf-8" ?>
+<!--
+SPDX-FileCopyrightText: 2023 Coop IT Easy SC
+SPDX-License-Identifier: AGPL-3.0-or-later
+-->
+<odoo>
+    <record id="membership_products_form_period" model="ir.ui.view">
+        <field name="name">Membership Products (custom type)</field>
+        <field name="model">product.template</field>
+        <field name="inherit_id" ref="membership_type.membership_products_form" />
+        <field name="arch" type="xml">
+            <field name="membership_type" position="after">
+                <field
+                    name="dates_mandatory"
+                    attrs="{'invisible': [('membership_type', '!=', 'custom')]}"
+                />
+            </field>
+        </field>
+    </record>
+</odoo>

membership_custom_date/wizard/membership_invoice.py

@@ -17,7 +17,9 @@ class MembershipInvoice(models.TransientModel):
     )
 
     def membership_invoice(self):
-        res = super().membership_invoice()
+        res = super(
+            MembershipInvoice, self.with_context(skip_mandatory_dates=True)
+        ).membership_invoice()
 
         account_moves = self.env["account.move"].search(res["domain"])
         membership_lines = account_moves.mapped(

membership_custom_date/wizard/membership_invoice_views.xml

@@ -9,11 +9,11 @@
                 <field name="product_id_type" invisible="1" />
                 <field
                     name="date_from"
-                    attrs="{'invisible': [('product_id_type', '!=', 'custom')]}"
+                    attrs="{'invisible': [('product_id_type', '!=', 'custom')], 'required': [('product_id_type', '=', 'custom')]}"
                 />
                 <field
                     name="date_to"
-                    attrs="{'invisible': [('product_id_type', '!=', 'custom')]}"
+                    attrs="{'invisible': [('product_id_type', '!=', 'custom')], 'required': [('product_id_type', '=', 'custom')]}"
                 />
             </xpath>
         </field>

membership_extension/models/membership_line.py

@@ -1,12 +1,13 @@
 # Copyright 2016 Antonio Espinosa <[email protected]>
 # Copyright 2017 David Vidal <[email protected]>
 # Copyright 2019 Onestein - Andrea Stirpe
+# Copyright 2023 Coop IT Easy SC
 # License AGPL-3.0 or later (https://www.gnu.org/licenses/agpl).
 
 from datetime import timedelta
 
 from odoo import _, api, fields, models
-from odoo.exceptions import UserError
+from odoo.exceptions import UserError, ValidationError
 
 
 class MembershipLine(models.Model):
@@ -35,6 +36,20 @@ class MembershipLine(models.Model):
         ),
     ]
 
+    @api.constrains("date_from", "date_to")
+    def _check_mandatory_dates(self):
+        # This is a rather dirty mechanism to skip this check. It can be used
+        # when creating a membership line and later writing date_from and
+        # date_to to it in the same workflow. It is not used in this module an
+        # sich.
+        if self.env.context.get("skip_mandatory_dates"):
+            return
+        for line in self:
+            if not line.date_from or not line.date_to:
+                raise ValidationError(
+                    _("Error: the start and end dates are mandatory.")
+                )
+
     @api.depends("membership_id")
     def _compute_member_price(self):
         for partner in self:

membership_extension/tests/test_membership.py

@@ -249,6 +249,8 @@ def test_membership_line_onchange(self):
                 "membership_id": self.gold_product.id,
                 "member_price": 100.00,
                 "date": fields.Date.today(),
+                "date_from": fields.Date.today(),
+                "date_to": fields.Date.today(),
                 "partner": self.partner.id,
                 "state": "invoiced",
             }
@@ -495,3 +497,27 @@ def test_no_dates(self):
                 "membership_date_to": "1970-01-02",
             }
         )
+
+    def test_membership_line_no_dates(self):
+        with self.assertRaises(ValidationError):
+            self.env["membership.membership_line"].create(
+                {
+                    "membership_id": self.gold_product.id,
+                    "member_price": 100.00,
+                    "date": fields.Date.today(),
+                    "date_from": fields.Date.today(),
+                    "partner": self.partner.id,
+                    "state": "waiting",
+                }
+            )
+        with self.assertRaises(ValidationError):
+            self.env["membership.membership_line"].create(
+                {
+                    "membership_id": self.gold_product.id,
+                    "member_price": 100.00,
+                    "date": fields.Date.today(),
+                    "date_to": fields.Date.today(),
+                    "partner": self.partner.id,
+                    "state": "waiting",
+                }
+            )

membership_type/models/__init__.py

@@ -3,3 +3,4 @@
 # SPDX-License-Identifier: AGPL-3.0-or-later
 
 from . import product_template
+from . import membership_membership_line

membership_type/models/membership_membership_line.py

@@ -0,0 +1,11 @@
+# SPDX-FileCopyrightText: 2023 Coop IT Easy SC
+#
+# SPDX-License-Identifier: AGPL-3.0-or-later
+
+from odoo import fields, models
+
+
+class MembershipLine(models.Model):
+    _inherit = "membership.membership_line"
+
+    membership_type = fields.Selection(related="membership_id.membership_type")

membership_variable_period/models/account_move_line.py

@@ -72,7 +72,9 @@ def write(self, vals):
 
     @api.model_create_multi
     def create(self, vals_list):
-        lines = super().create(vals_list)
+        lines = super(
+            AccountMoveLine, self.with_context(skip_mandatory_dates=True)
+        ).create(vals_list)
         membership_types = self._get_variable_period_product_membership_types()
         for line in lines.filtered(
             lambda l: l.move_id.move_type == "out_invoice"

membership_variable_period/tests/test_membership_variable_period.py

@@ -224,3 +224,18 @@ def test_create_invoice_line_with_no_product(self):
 
         self.assertFalse(invoice.invoice_line_ids[0].product_id)
         self.assertFalse(invoice.invoice_line_ids[0].membership_lines)
+
+    def test_invoice_mandatory_dates_check(self):
+        wizard = self.env["membership.invoice"].create(
+            {
+                "product_id": self.product.id,
+                "member_price": 100,
+            }
+        )
+        wizard = wizard.with_context(
+            default_partner_id=self.partner.id,
+            active_ids=self.partner.id,
+        )
+
+        # Expect no error
+        wizard.membership_invoice()