Better validation for phone number in billing info

This commit is contained in:
thomas girod 2024-09-27 22:35:03 +02:00
parent d29a5cdb44
commit 1b1284d3d0
8 changed files with 89 additions and 7 deletions

View File

@ -2,6 +2,7 @@ from ajax_select import make_ajax_field
from ajax_select.fields import AutoCompleteSelectField, AutoCompleteSelectMultipleField from ajax_select.fields import AutoCompleteSelectField, AutoCompleteSelectMultipleField
from django import forms from django import forms
from django.utils.translation import gettext_lazy as _ from django.utils.translation import gettext_lazy as _
from phonenumber_field.widgets import RegionalPhoneNumberWidget
from core.views.forms import NFCTextInput, SelectDate, SelectDateTime from core.views.forms import NFCTextInput, SelectDate, SelectDateTime
from counter.models import ( from counter.models import (
@ -28,6 +29,9 @@ class BillingInfoForm(forms.ModelForm):
"country", "country",
"phone_number", "phone_number",
] ]
widgets = {
"phone_number": RegionalPhoneNumberWidget,
}
class StudentCardForm(forms.ModelForm): class StudentCardForm(forms.ModelForm):

View File

@ -428,6 +428,24 @@ class TestBillingInfo:
assert infos.phone_number.as_national == "06123 45678" assert infos.phone_number.as_national == "06123 45678"
assert infos.phone_number.country_code == 49 assert infos.phone_number.country_code == 49
@pytest.mark.parametrize(
"phone_number", ["061234567a", "06 12 34 56", "061234567879", "azertyuiop"]
)
def test_invalid_phone_number(
self, client: Client, payload: dict, phone_number: str
):
"""Test that invalid phone numbers are rejected."""
user = subscriber_user.make()
client.force_login(user)
payload["phone_number"] = phone_number
response = client.put(
reverse("api:put_billing_info", args=[user.id]),
json.dumps(payload),
content_type="application/json",
)
assert response.status_code == 422
assert not BillingInfo.objects.filter(customer__user=user).exists()
class TestBarmanConnection(TestCase): class TestBarmanConnection(TestCase):
@classmethod @classmethod

View File

@ -1,6 +1,11 @@
from typing import Annotated
from ninja import ModelSchema, Schema from ninja import ModelSchema, Schema
from pydantic import Field, NonNegativeInt, PositiveInt, TypeAdapter from pydantic import Field, NonNegativeInt, PositiveInt, TypeAdapter
# from phonenumber_field.phonenumber import PhoneNumber
from pydantic_extra_types.phone_numbers import PhoneNumber, PhoneNumberValidator
from counter.models import BillingInfo from counter.models import BillingInfo
@ -35,4 +40,4 @@ class BillingInfoSchema(ModelSchema):
# for reasons described in the model, BillingInfo.phone_number # for reasons described in the model, BillingInfo.phone_number
# in nullable, but null values shouldn't be actually allowed, # in nullable, but null values shouldn't be actually allowed,
# so we force the field to be required # so we force the field to be required
phone_number: str phone_number: Annotated[PhoneNumber, PhoneNumberValidator(default_region="FR")]

View File

@ -42,7 +42,16 @@ document.addEventListener("alpine:init", () => {
this.req_state = res.ok this.req_state = res.ok
? BillingInfoReqState.SUCCESS ? BillingInfoReqState.SUCCESS
: BillingInfoReqState.FAILURE; : BillingInfoReqState.FAILURE;
if (res.ok) { if (res.status === 422) {
const errors = (await res.json())["detail"].map((err) => err["loc"]).flat();
Array.from(form.querySelectorAll("input"))
.filter((elem) => errors.includes(elem.name))
.forEach((elem) => {
elem.setCustomValidity(gettext("Incorrect value"));
elem.reportValidity();
elem.oninput = () => elem.setCustomValidity("");
});
} else if (res.ok) {
Alpine.store("billing_inputs").fill(); Alpine.store("billing_inputs").fill();
} }
}, },

View File

@ -144,7 +144,7 @@ class EbouticCommand(LoginRequiredMixin, TemplateView):
elif default_billing_info.phone_number is None: elif default_billing_info.phone_number is None:
kwargs["billing_infos_state"] = BillingInfoState.MISSING_PHONE_NUMBER kwargs["billing_infos_state"] = BillingInfoState.MISSING_PHONE_NUMBER
else: else:
kwargs["billing_infos_state"] = BillingInfoState.EMPTY kwargs["billing_infos_state"] = BillingInfoState.VALID
if kwargs["billing_infos_state"] == BillingInfoState.VALID: if kwargs["billing_infos_state"] == BillingInfoState.VALID:
# the user has already filled all of its billing_infos, thus we can # the user has already filled all of its billing_infos, thus we can
# get it without expecting an error # get it without expecting an error

View File

@ -7,7 +7,7 @@
msgid "" msgid ""
msgstr "" msgstr ""
"Report-Msgid-Bugs-To: \n" "Report-Msgid-Bugs-To: \n"
"POT-Creation-Date: 2024-09-03 15:22+0200\n" "POT-Creation-Date: 2024-09-27 22:32+0200\n"
"PO-Revision-Date: 2024-09-17 11:54+0200\n" "PO-Revision-Date: 2024-09-17 11:54+0200\n"
"Last-Translator: Sli <antoine@bartuccio.fr>\n" "Last-Translator: Sli <antoine@bartuccio.fr>\n"
"Language-Team: AE info <ae.info@utbm.fr>\n" "Language-Team: AE info <ae.info@utbm.fr>\n"
@ -16,9 +16,24 @@ msgstr ""
"Content-Type: text/plain; charset=UTF-8\n" "Content-Type: text/plain; charset=UTF-8\n"
"Content-Transfer-Encoding: 8bit\n" "Content-Transfer-Encoding: 8bit\n"
"Plural-Forms: nplurals=2; plural=(n > 1);\n" "Plural-Forms: nplurals=2; plural=(n > 1);\n"
#: core/static/user/js/family_graph.js:230
#: core/static/user/js/family_graph.js:233
msgid "family_tree.%(extension)s" msgid "family_tree.%(extension)s"
msgstr "arbre_genealogique.%(extension)s" msgstr "arbre_genealogique.%(extension)s"
#: sas/static/sas/js/picture.js:52
#: core/static/user/js/user_edit.js:93
#, javascript-format
msgid "captured.%s"
msgstr "capture.%s"
#: eboutic/static/eboutic/js/makecommand.js:50
msgid "Incorrect value"
msgstr "Valeur incorrecte"
#: sas/static/sas/js/viewer.js:196
msgid "Couldn't moderate picture"
msgstr "Echec de la suppression de la photo"
#: sas/static/sas/js/viewer.js:209
msgid "Couldn't delete picture" msgid "Couldn't delete picture"
msgstr "Echec de la suppression de la photo" msgstr "Echec de la suppression de la photo"

29
poetry.lock generated
View File

@ -1813,6 +1813,33 @@ files = [
[package.dependencies] [package.dependencies]
typing-extensions = ">=4.6.0,<4.7.0 || >4.7.0" typing-extensions = ">=4.6.0,<4.7.0 || >4.7.0"
[[package]]
name = "pydantic-extra-types"
version = "2.9.0"
description = "Extra Pydantic types."
optional = false
python-versions = ">=3.8"
files = []
develop = false
[package.dependencies]
pydantic = ">=2.5.2"
typing-extensions = "*"
[package.extras]
all = ["pendulum (>=3.0.0,<4.0.0)", "phonenumbers (>=8,<9)", "pycountry (>=23)", "python-ulid (>=1,<2)", "python-ulid (>=1,<3)", "pytz (>=2024.1)", "semver (>=3.0.2)", "semver (>=3.0.2,<3.1.0)", "tzdata (>=2024.1)"]
pendulum = ["pendulum (>=3.0.0,<4.0.0)"]
phonenumbers = ["phonenumbers (>=8,<9)"]
pycountry = ["pycountry (>=23)"]
python-ulid = ["python-ulid (>=1,<2)", "python-ulid (>=1,<3)"]
semver = ["semver (>=3.0.2)"]
[package.source]
type = "git"
url = "https://github.com/pydantic/pydantic-extra-types.git"
reference = "HEAD"
resolved_reference = "58db4b096d7c90566d3d48d51b4665c01a591df6"
[[package]] [[package]]
name = "pygments" name = "pygments"
version = "2.18.0" version = "2.18.0"
@ -2626,4 +2653,4 @@ filelock = ">=3.4"
[metadata] [metadata]
lock-version = "2.0" lock-version = "2.0"
python-versions = "^3.12" python-versions = "^3.12"
content-hash = "b6202203d272cecdb607ea8ebc1ba12dd8369e4f387f65692c4a9681915e6f48" content-hash = "c9c49497cc576b24c96ea914b74ef5c3a0c2981c488a599752f05aabb575f8d8"

View File

@ -45,6 +45,10 @@ dict2xml = "^1.7.3"
Sphinx = "^5" # Needed for building xapian Sphinx = "^5" # Needed for building xapian
tomli = "^2.0.1" tomli = "^2.0.1"
django-honeypot = "^1.2.1" django-honeypot = "^1.2.1"
# When I introduced pydantic-extra-types, I needed *right now*
# the PhoneNumberValidator class which was on the master branch but not released yet.
# Once it's released, switch this to a regular version.
pydantic-extra-types = { git = "https://github.com/pydantic/pydantic-extra-types.git", rev = "58db4b0" }
[tool.poetry.group.prod.dependencies] [tool.poetry.group.prod.dependencies]
# deps used in prod, but unnecessary for development # deps used in prod, but unnecessary for development