[ADD] phone validations #844
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1 +1,3 @@ | ||
| from . import res_partner | ||
| from . import phone_number | ||
| from . import phone_number_validation |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,48 @@ | ||
| import logging | ||
| import re | ||
|
|
||
| from odoo import _, api, models | ||
| from odoo.exceptions import ValidationError | ||
|
|
||
| _logger = logging.getLogger(__name__) | ||
|
|
||
|
|
||
| class G2PPhoneNumber(models.Model): | ||
| _inherit = "g2p.phone.number" | ||
|
|
||
| def write(self, vals): | ||
| res = super().write(vals) | ||
| if "phone_no" in vals or "country_id" in vals: | ||
| self._onchange_phone_validation() | ||
| return res | ||
|
|
||
| @api.model_create_multi | ||
| def create(self, vals): | ||
| record = super().create(vals) | ||
| record._onchange_phone_validation() | ||
| return record | ||
|
|
||
| @api.onchange("phone_no", "country_id") | ||
| def _onchange_phone_validation(self): | ||
| phone_validation = self.env["spp.phone.validation"].search([("state", "=", "active")]) | ||
| if not self.phone_no: | ||
| return | ||
|
|
||
| phone_no = self.phone_no | ||
| if phone_validation: | ||
| validated_success_count = 0 | ||
| error_msg = [] | ||
| for validation in phone_validation: | ||
| if validation.with_prefix: | ||
| pattern = r"^\+?" + re.escape(validation.prefix) + r"\d{" + str(validation.number_of_digits) + r"}$" | ||
| else: | ||
| pattern = r"^\d{" + str(validation.number_of_digits) + r"}$" | ||
| if re.match(pattern, phone_no): | ||
| validated_success_count += 1 | ||
| else: | ||
| error_msg.append(validation.name) | ||
|
|
||
| if validated_success_count == 0: | ||
| message = "Phone number must match one of the following formats: " + ", ".join(error_msg) | ||
| raise ValidationError(_(message)) | ||
| return | ||
|
Comment on lines
+26
to
+48
There was a problem hiding this comment. This method is called from Additionally, the validation logic can be made more efficient. Instead of iterating through all validations and counting successes, you can stop at the first successful match. If no match is found after checking all validations, then you can raise the error. def _onchange_phone_validation(self):
phone_validation = self.env["spp.phone.validation"].search([("state", "=", "active")])
if not phone_validation:
return
for record in self:
if not record.phone_no:
continue
phone_no = record.phone_no
is_valid = False
for validation in phone_validation:
if validation.with_prefix:
pattern = r"^\+?" + re.escape(validation.prefix) + r"\d{" + str(validation.number_of_digits) + r"}$"
else:
pattern = r"^\d{" + str(validation.number_of_digits) + r"}$"
if re.match(pattern, phone_no):
is_valid = True
break
if not is_valid:
error_msg = [v.name for v in phone_validation]
message = "Phone number must match one of the following formats: " + ", ".join(error_msg)
raise ValidationError(_(message))
return |
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| from odoo import api, fields, models | ||
|
|
||
|
|
||
| class SPPPhoneValidation(models.Model): | ||
| _name = "spp.phone.validation" | ||
| _description = "SPP Phone Validation" | ||
|
|
||
| name = fields.Char(string="Sample Format", compute="_compute_name") | ||
| number_of_digits = fields.Integer(string="Number of Digits", required=True) | ||
| with_prefix = fields.Boolean(string="With Prefix") | ||
| prefix = fields.Char(string="Prefix") | ||
| state = fields.Selection( | ||
| [("active", "Active"), ("inactive", "Inactive")], | ||
| string="State", | ||
| default="active", | ||
| ) | ||
|
|
||
| @api.depends("number_of_digits", "with_prefix", "prefix") | ||
| def _compute_name(self): | ||
| for record in self: | ||
| if record.with_prefix and record.prefix: | ||
| record.name = f"{record.prefix}{'X'*record.number_of_digits}" | ||
| else: | ||
| record.name = "X" * record.number_of_digits | ||
|
|
||
| def activate_phone_validation(self): | ||
| for record in self: | ||
| record.state = "active" | ||
|
|
||
| def deactivate_phone_validation(self): | ||
| for record in self: | ||
| record.state = "inactive" |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1 +1,2 @@ | ||
| from . import test_res_partner | ||
| from . import test_phone_number_validation |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,69 @@ | ||
| from odoo.exceptions import ValidationError | ||
| from odoo.tests import TransactionCase | ||
|
|
||
|
|
||
| class TestPhoneValidation(TransactionCase): | ||
|
There was a problem hiding this comment. The current test suite is a good start, but it could be improved to cover more cases:
Example for point 1: # Test create
with self.assertRaises(ValidationError):
self.phone_model.create({
"partner_id": self.registrant.id,
"phone_no": "12345",
"country_id": self.env.ref("base.ph").id,
})
# Test write
phone = self.phone_model.create(...) # with valid data
with self.assertRaises(ValidationError):
phone.write({"phone_no": "12345"}) |
||
| @classmethod | ||
| def setUpClass(cls): | ||
| super().setUpClass() | ||
| cls.partner_model = cls.env["res.partner"] | ||
| cls.phone_validation_model = cls.env["spp.phone.validation"] | ||
| cls.phone_model = cls.env["g2p.phone.number"] | ||
| cls.registrant = cls.partner_model.create( | ||
| { | ||
| "name": "Test Registrant", | ||
| "is_registrant": True, | ||
| } | ||
| ) | ||
| cls.phone_validation_1 = cls.phone_validation_model.create( | ||
| { | ||
| "number_of_digits": 10, | ||
| "with_prefix": True, | ||
| "prefix": "+63", | ||
| "state": "active", | ||
| } | ||
| ) | ||
| cls.phone_validation_2 = cls.phone_validation_model.create( | ||
| { | ||
| "number_of_digits": 11, | ||
| "with_prefix": False, | ||
| "state": "active", | ||
| } | ||
| ) | ||
|
|
||
| def test_01_create_phone_with_invalid_number(self): | ||
| phone = self.phone_model.create( | ||
| { | ||
| "partner_id": self.registrant.id, | ||
| "phone_no": "+639123456789", | ||
| "country_id": self.env.ref("base.ph").id, | ||
| } | ||
| ) | ||
|
|
||
| with self.assertRaises(ValidationError) as cm: | ||
| phone.phone_no = "12345" | ||
| phone._onchange_phone_validation() | ||
|
|
||
| self.assertIn("Phone number must match one of the following formats", str(cm.exception)) | ||
|
|
||
| def test_02_create_phone_with_valid_number_with_prefix(self): | ||
| phone = self.phone_model.create( | ||
| { | ||
| "partner_id": self.registrant.id, | ||
| "phone_no": "+639123456789", | ||
| "country_id": self.env.ref("base.ph").id, | ||
| } | ||
| ) | ||
| phone._onchange_phone_validation() | ||
| self.assertEqual(phone.phone_no, "+639123456789") | ||
|
|
||
| def test_03_create_phone_with_valid_number_without_prefix(self): | ||
| phone = self.phone_model.create( | ||
| { | ||
| "partner_id": self.registrant.id, | ||
| "phone_no": "09123456789", | ||
| "country_id": self.env.ref("base.ph").id, | ||
| } | ||
| ) | ||
| phone._onchange_phone_validation() | ||
| self.assertEqual(phone.phone_no, "09123456789") | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,74 @@ | ||||||
| <?xml version="1.0" encoding="UTF-8" ?> | ||||||
| <!-- | ||||||
| Part of OpenSPP Registry. See LICENSE file for full copyright and licensing details. | ||||||
| --> | ||||||
| <odoo> | ||||||
| <record id="view_phone_validation_tree" model="ir.ui.view"> | ||||||
| <field name="name">view_phone_validation_tree</field> | ||||||
| <field name="model">spp.phone.validation</field> | ||||||
| <field name="priority">1</field> | ||||||
| <field name="arch" type="xml"> | ||||||
| <tree create="true" edit="true" delete="false" editable="top"> | ||||||
| <field name="number_of_digits" /> | ||||||
| <field name="with_prefix" /> | ||||||
| <field name="prefix" /> | ||||||
| <field name="name" /> | ||||||
|
There was a problem hiding this comment. The
Suggested change
|
||||||
| <field | ||||||
| name="state" | ||||||
| decoration-danger="state == 'inactive'" | ||||||
| decoration-success="state == 'active'" | ||||||
| readonly="1" | ||||||
| /> | ||||||
| <button | ||||||
| name="activate_phone_validation" | ||||||
| title="Activate" | ||||||
| type="object" | ||||||
| icon="fa-check" | ||||||
| class="btn-success" | ||||||
| invisible="state == 'active'" | ||||||
| /> | ||||||
| <button | ||||||
| name="deactivate_phone_validation" | ||||||
| title="Deactivate" | ||||||
| type="object" | ||||||
| icon="fa-times" | ||||||
| class="btn-danger" | ||||||
| invisible="state == 'inactive'" | ||||||
| /> | ||||||
| </tree> | ||||||
| </field> | ||||||
| </record> | ||||||
|
|
||||||
| <record id="action_phone_validation" model="ir.actions.act_window"> | ||||||
| <field name="name">Phone Validation</field> | ||||||
| <field name="type">ir.actions.act_window</field> | ||||||
| <field name="res_model">spp.phone.validation</field> | ||||||
| <field name="view_mode">tree</field> | ||||||
| <field name="context">{}</field> | ||||||
| <field name="domain">[]</field> | ||||||
| <field name="help" type="html"> | ||||||
| <p class="o_view_nocontent_smiling_face"> | ||||||
| Add a Phone Validation! | ||||||
| </p><p> | ||||||
| Click the create button to enter a phone validation. | ||||||
| </p> | ||||||
| </field> | ||||||
| </record> | ||||||
|
|
||||||
| <record id="action_phone_validation_tree_view" model="ir.actions.act_window.view"> | ||||||
| <field name="sequence" eval="1" /> | ||||||
| <field name="view_mode">tree</field> | ||||||
| <field name="view_id" ref="view_phone_validation_tree" /> | ||||||
| <field name="act_window_id" ref="action_phone_validation" /> | ||||||
| </record> | ||||||
|
|
||||||
| <menuitem | ||||||
| id="menu_phone_validation" | ||||||
| name="Phone Validation" | ||||||
| action="action_phone_validation" | ||||||
| parent="g2p_registry_base.g2p_configuration_menu_root" | ||||||
| sequence="10" | ||||||
| groups="g2p_registry_base.group_g2p_admin,g2p_registry_base.group_g2p_registrar" | ||||||
| /> | ||||||
|
|
||||||
| </odoo> | ||||||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
createmethod is decorated with@api.model_create_multibut is defined as an instance method (def create(self, vals):). According to Odoo conventions, it should be a class method. This is confusing and can lead to unexpected behavior. Please refactor it to use the standard signaturedef create(cls, vals_list):and conventional variable names.