From 92e9f50cbf8219ec8a127c04c3236dfe7b82e06b Mon Sep 17 00:00:00 2001 From: Carmen Bianca BAKKER Date: Thu, 23 Nov 2023 17:13:16 +0100 Subject: [PATCH] [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 --- membership_custom_date/__manifest__.py | 1 + membership_custom_date/models/__init__.py | 1 + .../models/membership_membership_line.py | 24 ++++ .../models/product_template.py | 8 ++ membership_custom_date/tests/__init__.py | 5 + .../tests/test_membership.py | 120 ++++++++++++++++++ .../views/product_template_views.xml | 20 +++ .../wizard/membership_invoice.py | 5 +- .../wizard/membership_invoice_views.xml | 5 +- .../models/membership_line.py | 17 ++- membership_extension/tests/test_membership.py | 26 ++++ membership_type/models/__init__.py | 1 + .../models/membership_membership_line.py | 11 ++ membership_type/models/product_template.py | 11 +- .../models/account_move_line.py | 4 +- .../tests/test_membership_variable_period.py | 15 +++ 16 files changed, 268 insertions(+), 6 deletions(-) create mode 100644 membership_custom_date/models/membership_membership_line.py create mode 100644 membership_custom_date/tests/__init__.py create mode 100644 membership_custom_date/tests/test_membership.py create mode 100644 membership_custom_date/views/product_template_views.xml create mode 100644 membership_type/models/membership_membership_line.py diff --git a/membership_custom_date/__manifest__.py b/membership_custom_date/__manifest__.py index 31e832541..34d297f0b 100644 --- a/membership_custom_date/__manifest__.py +++ b/membership_custom_date/__manifest__.py @@ -20,6 +20,7 @@ ], "excludes": [], "data": [ + "views/product_template_views.xml", "wizard/membership_invoice_views.xml", ], "demo": [], diff --git a/membership_custom_date/models/__init__.py b/membership_custom_date/models/__init__.py index 8afbbed9b..c45790b74 100644 --- a/membership_custom_date/models/__init__.py +++ b/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 diff --git a/membership_custom_date/models/membership_membership_line.py b/membership_custom_date/models/membership_membership_line.py new file mode 100644 index 000000000..d73301c05 --- /dev/null +++ b/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() diff --git a/membership_custom_date/models/product_template.py b/membership_custom_date/models/product_template.py index 41edd977f..20546686f 100644 --- a/membership_custom_date/models/product_template.py +++ b/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 diff --git a/membership_custom_date/tests/__init__.py b/membership_custom_date/tests/__init__.py new file mode 100644 index 000000000..9618dc51d --- /dev/null +++ b/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 diff --git a/membership_custom_date/tests/test_membership.py b/membership_custom_date/tests/test_membership.py new file mode 100644 index 000000000..3b80f6273 --- /dev/null +++ b/membership_custom_date/tests/test_membership.py @@ -0,0 +1,120 @@ +# 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() + + self.product.dates_mandatory = False + invoice_form.product_id = self.product + invoice_form.date_from = False + invoice_form.date_to = False + 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() diff --git a/membership_custom_date/views/product_template_views.xml b/membership_custom_date/views/product_template_views.xml new file mode 100644 index 000000000..169e9905a --- /dev/null +++ b/membership_custom_date/views/product_template_views.xml @@ -0,0 +1,20 @@ + + + + + Membership Products (custom type) + product.template + + + + + + + + diff --git a/membership_custom_date/wizard/membership_invoice.py b/membership_custom_date/wizard/membership_invoice.py index e49ec1fc9..6aaf1acf4 100644 --- a/membership_custom_date/wizard/membership_invoice.py +++ b/membership_custom_date/wizard/membership_invoice.py @@ -15,9 +15,12 @@ class MembershipInvoice(models.TransientModel): string="Membership Type", related="product_id.membership_type", ) + dates_mandatory = fields.Boolean(related="product_id.dates_mandatory") 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( diff --git a/membership_custom_date/wizard/membership_invoice_views.xml b/membership_custom_date/wizard/membership_invoice_views.xml index 7cfae3c74..40e424b3e 100644 --- a/membership_custom_date/wizard/membership_invoice_views.xml +++ b/membership_custom_date/wizard/membership_invoice_views.xml @@ -7,13 +7,14 @@ + diff --git a/membership_extension/models/membership_line.py b/membership_extension/models/membership_line.py index e811e6cd4..edfa60517 100644 --- a/membership_extension/models/membership_line.py +++ b/membership_extension/models/membership_line.py @@ -1,12 +1,13 @@ # Copyright 2016 Antonio Espinosa # Copyright 2017 David Vidal # 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: diff --git a/membership_extension/tests/test_membership.py b/membership_extension/tests/test_membership.py index b25787e99..cce103bce 100644 --- a/membership_extension/tests/test_membership.py +++ b/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", + } + ) diff --git a/membership_type/models/__init__.py b/membership_type/models/__init__.py index 00802a0ce..83f862078 100644 --- a/membership_type/models/__init__.py +++ b/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 diff --git a/membership_type/models/membership_membership_line.py b/membership_type/models/membership_membership_line.py new file mode 100644 index 000000000..e31bdcd68 --- /dev/null +++ b/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") diff --git a/membership_type/models/product_template.py b/membership_type/models/product_template.py index 410876bad..9ca43e6aa 100644 --- a/membership_type/models/product_template.py +++ b/membership_type/models/product_template.py @@ -17,14 +17,23 @@ class ProductTemplate(models.Model): required=True, ) + @api.model + def _is_check_dates_for_type(self, membership_type): + # Override this to pass more membership types to _check_membership_dates + return membership_type == "fixed" + @api.constrains( + "membership_date_from", + "membership_date_to", "membership", "membership_type", ) def _check_membership_dates(self): return super( ProductTemplate, - self.filtered(lambda record: record.membership_type == "fixed"), + self.filtered( + lambda record: self._is_check_dates_for_type(record.membership_type) + ), )._check_membership_dates() @api.model diff --git a/membership_variable_period/models/account_move_line.py b/membership_variable_period/models/account_move_line.py index eb69d4622..6497ea666 100644 --- a/membership_variable_period/models/account_move_line.py +++ b/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" diff --git a/membership_variable_period/tests/test_membership_variable_period.py b/membership_variable_period/tests/test_membership_variable_period.py index b73a8e980..135b19e0b 100644 --- a/membership_variable_period/tests/test_membership_variable_period.py +++ b/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()