Skip to content

Commit

Permalink
[IMP] membership_extension & _variable_period: Make sure necessary fi…
Browse files Browse the repository at this point in the history
…elds are always truthy

This is an additional sanity check to make sure that fields that are
expected to be available are actually available.

The situation here is a little tricky. Especially as pertains date_from
and date_to; these fields are `required=True` nowhere. Not on the
product, not on the membership invoice, and not on the membership line.
However, they _are_ set as required in the product and membership line
views. If these values are ever empty, it's a sure sign that something
went wrong somewhere, because the user should not be able to reach such
a state via the UI.

One sure way to reach that state was via the demo data, however, which
had no start and end dates. This is now fixed.

The situation for the variable period fields is similar. These fields
should never be empty when the membership type is variable, and it's not
typically possible to empty these values via the UI. For convenience's
sake, I simply set them to required an sich, with a default value.

Signed-off-by: Carmen Bianca BAKKER <[email protected]>
  • Loading branch information
carmenbianca committed Oct 6, 2023
1 parent fe1541c commit 315bdba
Show file tree
Hide file tree
Showing 7 changed files with 114 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,13 @@ class TestMembershipDelegateSetup(TestContractBase):
def setUpClass(cls):
super(TestMembershipDelegateSetup, cls).setUpClass()
cls.partner2 = cls.env["res.partner"].create({"name": "Mrs. Odoo"})
cls.product_1.membership = True
cls.product_1.write(
{
"membership_date_from": "2023-01-01",
"membership_date_to": "2023-01-02",
"membership": True,
}
)
cls.contract.delegated_member_id = cls.partner2

def test_01_generate_and_delegate(self):
Expand Down
15 changes: 15 additions & 0 deletions membership_extension/demo/product_template_demo.xml
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,11 @@
<field name="name">Membership Gold</field>
<field name="membership" eval="True" />
<field name="type">service</field>
<field name="membership_date_from" eval="DateTime.today()" />
<field
name="membership_date_to"
eval="DateTime.today() + relativedelta(years=1)"
/>
</record>
<record id="membership_1_product_template" model="product.template">
<field
Expand All @@ -19,6 +24,11 @@
<field name="name">Membership Silver</field>
<field name="membership" eval="True" />
<field name="type">service</field>
<field name="membership_date_from" eval="DateTime.today()" />
<field
name="membership_date_to"
eval="DateTime.today() + relativedelta(years=1)"
/>
</record>
<record id="membership_2_product_template" model="product.template">
<field
Expand All @@ -28,5 +38,10 @@
<field name="name">Membership Bronze</field>
<field name="membership" eval="True" />
<field name="type">service</field>
<field name="membership_date_from" eval="DateTime.today()" />
<field
name="membership_date_to"
eval="DateTime.today() + relativedelta(years=1)"
/>
</record>
</odoo>
13 changes: 12 additions & 1 deletion membership_extension/models/product_template.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@

from datetime import timedelta

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


class ProductTemplate(models.Model):
Expand Down Expand Up @@ -32,3 +33,13 @@ def _compute_membership_category_id(self):
if record.company_id and record.membership_category_id.company_id:
if record.membership_category_id.company_id != record.company_id:
record.membership_category_id = False

@api.constrains("membership_date_from", "membership_date_to", "membership")
def _check_membership_dates(self):
if self.filtered(
lambda record: record.membership
and (not record.membership_date_from or not record.membership_date_to)
):
raise ValidationError(
_("A membership product must have a start date and an end date.")
)
38 changes: 38 additions & 0 deletions membership_extension/tests/test_membership.py
Original file line number Diff line number Diff line change
Expand Up @@ -457,3 +457,41 @@ def test_category_multicompany(self):
self.assertTrue(template.membership_category_id)
template.company_id = company_b
self.assertFalse(template.membership_category_id)

def test_no_dates(self):
with self.assertRaises(ValidationError):
self.env["product.template"].create(
{
"name": "Test Membership",
"membership": True,
"type": "service",
}
)
with self.assertRaises(ValidationError):
self.env["product.template"].create(
{
"name": "Test Membership",
"membership": True,
"type": "service",
"membership_date_from": "1970-01-01",
}
)
with self.assertRaises(ValidationError):
self.env["product.template"].create(
{
"name": "Test Membership",
"membership": True,
"type": "service",
"membership_date_to": "1970-01-01",
}
)
# No error
self.env["product.template"].create(
{
"name": "Test Membership",
"membership": True,
"type": "service",
"membership_date_from": "1970-01-01",
"membership_date_to": "1970-01-02",
}
)
20 changes: 20 additions & 0 deletions membership_variable_period/migrations/16.0.1.1.0/pre-migration.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
# SPDX-FileCopyrightText: 2023 Coop IT Easy SC
#
# SPDX-License-Identifier: AGPL-3.0-or-later


def migrate(cr, version):
cr.execute(
"""
UPDATE product_template
SET membership_interval_qty = 1
WHERE membership_interval_qty IS null;
"""
)
cr.execute(
"""
UPDATE product_template
SET membership_interval_unit = 'years'
WHERE membership_interval_unit IS null;
"""
)
23 changes: 22 additions & 1 deletion membership_variable_period/models/product_template.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,14 @@ def _get_next_date(self, date, qty=1):
default="fixed",
required=True,
)
membership_interval_qty = fields.Integer(string="Interval quantity", default=1)
membership_interval_qty = fields.Integer(
string="Interval quantity",
default=1,
# Technically only required if membership_type is variable, but because
# there's a default value, this should be fine, and should prevent bad
# scenarios where a value is expected but none is available.
required=True,
)
membership_interval_unit = fields.Selection(
string="Interval Unit",
selection=[
Expand All @@ -52,6 +59,8 @@ def _get_next_date(self, date, qty=1):
("years", "years"),
],
default="years",
# Same comment as on membership_interval_qty in re being required.
required=True,
)

def _correct_vals_membership_type(self, vals, membership_type):
Expand Down Expand Up @@ -79,3 +88,15 @@ def write(self, vals):
)
super(ProductTemplate, rec).write(vals2)
return True

@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"),
)._check_membership_dates()
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@ def setUp(self):
{
"name": "Membership product with variable period",
"membership": True,
"membership_date_from": "2015-01-01",
"membership_date_to": "2015-12-31",
"membership_type": "variable",
"membership_interval_qty": 1,
"membership_interval_unit": "weeks",
Expand Down

0 comments on commit 315bdba

Please sign in to comment.