From afa640701efbfc5199b72fbe2332e9de9893d62b Mon Sep 17 00:00:00 2001
From: Nigel Gott <nigel@baserow.io>
Date: Tue, 23 Mar 2021 13:29:06 +0000
Subject: [PATCH] Use a simpler regex to validate phone number field, add test
covering validation and database conversion for phone number field, enable
standard filters and sorts.
---
.../src/baserow/contrib/database/config.py | 3 +-
.../src/baserow/contrib/database/db/schema.py | 33 +-
.../contrib/database/fields/field_types.py | 107 ++++-
.../contrib/database/fields/handler.py | 25 +-
.../baserow/contrib/database/fields/models.py | 4 +
.../contrib/database/fields/registries.py | 32 +-
.../migrations/0029_phonenumberfield.py | 29 ++
.../src/baserow/contrib/database/models.py | 4 +-
.../contrib/database/views/view_filters.py | 11 +-
.../api/fields/test_field_views_types.py | 96 +++-
.../contrib/database/db/test_db_schema.py | 6 +-
.../database/field/test_field_handler.py | 413 +++++++++++++++++-
.../database/field/test_field_types.py | 104 ++++-
.../database/table/test_table_models.py | 15 +-
backend/tests/conftest.py | 20 +
backend/tests/fixtures/field.py | 53 ++-
changelog.md | 1 +
.../001-phone-number-field-validation.md | 107 +++++
web-frontend/modules/core/utils/string.js | 9 +
.../row/RowEditFieldPhoneNumber.vue | 27 ++
.../FunctionalGridViewFieldPhoneNumber.vue | 7 +
.../grid/fields/GridViewFieldPhoneNumber.vue | 43 ++
web-frontend/modules/database/fieldTypes.js | 71 ++-
.../database/mixins/phoneNumberField.js | 26 ++
web-frontend/modules/database/plugin.js | 2 +
web-frontend/modules/database/viewFilters.js | 10 +-
26 files changed, 1192 insertions(+), 66 deletions(-)
create mode 100644 backend/src/baserow/contrib/database/migrations/0029_phonenumberfield.py
create mode 100644 docs/decisions/001-phone-number-field-validation.md
create mode 100644 web-frontend/modules/database/components/row/RowEditFieldPhoneNumber.vue
create mode 100644 web-frontend/modules/database/components/view/grid/fields/FunctionalGridViewFieldPhoneNumber.vue
create mode 100644 web-frontend/modules/database/components/view/grid/fields/GridViewFieldPhoneNumber.vue
create mode 100644 web-frontend/modules/database/mixins/phoneNumberField.js
diff --git a/backend/src/baserow/contrib/database/config.py b/backend/src/baserow/contrib/database/config.py
index 37bb6cc8e..444be6cf7 100644
--- a/backend/src/baserow/contrib/database/config.py
+++ b/backend/src/baserow/contrib/database/config.py
@@ -47,7 +47,7 @@ class DatabaseConfig(AppConfig):
from .fields.field_types import (
TextFieldType, LongTextFieldType, URLFieldType, NumberFieldType,
BooleanFieldType, DateFieldType, LinkRowFieldType, EmailFieldType,
- FileFieldType, SingleSelectFieldType
+ FileFieldType, SingleSelectFieldType, PhoneNumberFieldType
)
field_type_registry.register(TextFieldType())
field_type_registry.register(LongTextFieldType())
@@ -59,6 +59,7 @@ class DatabaseConfig(AppConfig):
field_type_registry.register(LinkRowFieldType())
field_type_registry.register(FileFieldType())
field_type_registry.register(SingleSelectFieldType())
+ field_type_registry.register(PhoneNumberFieldType())
from .fields.field_converters import LinkRowFieldConverter, FileFieldConverter
field_converter_registry.register(LinkRowFieldConverter())
diff --git a/backend/src/baserow/contrib/database/db/schema.py b/backend/src/baserow/contrib/database/db/schema.py
index 440490dff..23cd79c2a 100644
--- a/backend/src/baserow/contrib/database/db/schema.py
+++ b/backend/src/baserow/contrib/database/db/schema.py
@@ -10,9 +10,9 @@ class PostgresqlLenientDatabaseSchemaEditor:
format. If the casting still fails the value will be set to null.
"""
- sql_alter_column_type = "ALTER COLUMN %(column)s TYPE %(type)s " \
- "USING pg_temp.try_cast(%(column)s::text)"
- sql_drop_try_cast = "DROP FUNCTION IF EXISTS pg_temp.try_cast(text, int)"
+ sql_alter_column_type = 'ALTER COLUMN %(column)s TYPE %(type)s ' \
+ 'USING pg_temp.try_cast(%(column)s::text)'
+ sql_drop_try_cast = 'DROP FUNCTION IF EXISTS pg_temp.try_cast(text, int)'
sql_create_try_cast = """
create or replace function pg_temp.try_cast(
p_in text,
@@ -35,16 +35,20 @@ class PostgresqlLenientDatabaseSchemaEditor:
"""
def __init__(self, *args, alter_column_prepare_old_value='',
- alter_column_prepare_new_value=''):
+ alter_column_prepare_new_value='',
+ force_alter_column=False):
self.alter_column_prepare_old_value = alter_column_prepare_old_value
self.alter_column_prepare_new_value = alter_column_prepare_new_value
+ self.force_alter_column = force_alter_column
super().__init__(*args)
def _alter_field(self, model, old_field, new_field, old_type, new_type,
old_db_params, new_db_params, strict=False):
+ if self.force_alter_column:
+ old_type = f'{old_type}_forced'
+
if old_type != new_type:
variables = {}
-
if isinstance(self.alter_column_prepare_old_value, tuple):
alter_column_prepare_old_value, v = self.alter_column_prepare_old_value
variables = {**variables, **v}
@@ -57,12 +61,13 @@ class PostgresqlLenientDatabaseSchemaEditor:
else:
alter_column_prepare_new_value = self.alter_column_prepare_new_value
+ quoted_column_name = self.quote_name(new_field.column)
self.execute(self.sql_drop_try_cast)
self.execute(self.sql_create_try_cast % {
- "column": self.quote_name(new_field.column),
- "type": new_type,
- "alter_column_prepare_old_value": alter_column_prepare_old_value,
- "alter_column_prepare_new_value": alter_column_prepare_new_value
+ 'column': quoted_column_name,
+ 'type': new_type,
+ 'alter_column_prepare_old_value': alter_column_prepare_old_value,
+ 'alter_column_prepare_new_value': alter_column_prepare_new_value
}, variables)
return super()._alter_field(model, old_field, new_field, old_type, new_type,
@@ -71,7 +76,8 @@ class PostgresqlLenientDatabaseSchemaEditor:
@contextlib.contextmanager
def lenient_schema_editor(connection, alter_column_prepare_old_value=None,
- alter_column_prepare_new_value=None):
+ alter_column_prepare_new_value=None,
+ force_alter_column=False):
"""
A contextual function that yields a modified version of the connection's schema
editor. This temporary version is more lenient then the regular editor. Normally
@@ -89,6 +95,9 @@ def lenient_schema_editor(connection, alter_column_prepare_old_value=None,
:param alter_column_prepare_new_value: Optionally a query statement converting the
`p_in` text value to the new type.
:type alter_column_prepare_new_value: None or str
+ :param force_alter_column: When true forces the schema editor to run an alter
+ column statement using the previous two alter_column_prepare parameters.
+ :type force_alter_column: bool
:raises ValueError: When the provided connection is not supported. For now only
`postgresql` is supported.
"""
@@ -109,7 +118,9 @@ def lenient_schema_editor(connection, alter_column_prepare_old_value=None,
connection.SchemaEditorClass = schema_editor_class
- kwargs = {}
+ kwargs = {
+ 'force_alter_column': force_alter_column
+ }
if alter_column_prepare_old_value:
kwargs['alter_column_prepare_old_value'] = alter_column_prepare_old_value
diff --git a/backend/src/baserow/contrib/database/fields/field_types.py b/backend/src/baserow/contrib/database/fields/field_types.py
index 9c7c538bd..0ed1a4788 100644
--- a/backend/src/baserow/contrib/database/fields/field_types.py
+++ b/backend/src/baserow/contrib/database/fields/field_types.py
@@ -6,7 +6,7 @@ from dateutil import parser
from dateutil.parser import ParserError
from django.contrib.postgres.fields import JSONField
from django.core.exceptions import ValidationError
-from django.core.validators import URLValidator, EmailValidator
+from django.core.validators import URLValidator, EmailValidator, RegexValidator
from django.db import models
from django.db.models import Case, When, Q, F, Func, Value, CharField
from django.db.models.expressions import RawSQL
@@ -35,7 +35,7 @@ from .models import (
NUMBER_TYPE_INTEGER, NUMBER_TYPE_DECIMAL, TextField, LongTextField, URLField,
NumberField, BooleanField, DateField,
LinkRowField, EmailField, FileField,
- SingleSelectField, SelectOption
+ SingleSelectField, SelectOption, PhoneNumberField
)
from .registries import FieldType, field_type_registry
@@ -196,25 +196,8 @@ class NumberFieldType(FieldType):
return super().get_alter_column_prepare_new_value(connection, from_field,
to_field)
- def after_update(self, from_field, to_field, from_model, to_model, user, connection,
- altered_column, before):
- """
- The allowing of negative values isn't stored in the database field type. If
- the type hasn't changed, but the allowing of negative values has it means that
- the column data hasn't been converted to positive values yet. We need to do
- this here. All the negatives values are set to 0.
- """
-
- if (
- not altered_column
- and not to_field.number_negative
- and from_field.number_negative
- ):
- to_model.objects.filter(**{
- f'field_{to_field.id}__lt': 0
- }).update(**{
- f'field_{to_field.id}': 0
- })
+ def force_same_type_alter_column(self, from_field, to_field):
+ return not to_field.number_negative and from_field.number_negative
def contains_query(self, *args):
return contains_filter(*args)
@@ -1035,3 +1018,85 @@ class SingleSelectFieldType(FieldType):
annotation={f"select_option_value_{field_name}": query},
q={f'select_option_value_{field_name}__icontains': value}
)
+
+
+class PhoneNumberFieldType(FieldType):
+ """
+ A simple wrapper around a TextField which ensures any entered data is a
+ simple phone number.
+
+ See `docs/decisions/001-phone-number-field-validation.md` for context
+ as to why the phone number validation was implemented using a simple regex.
+ """
+
+ type = 'phone_number'
+ model_class = PhoneNumberField
+
+ MAX_PHONE_NUMBER_LENGTH = 100
+ """
+ According to the E.164 (https://en.wikipedia.org/wiki/E.164) standard for
+ international numbers the max length of an E.164 number without formatting is 15
+ characters. However we allow users to store formatting characters, spaces and
+ expect them to be entering numbers not in the E.164 standard but instead a
+ wide range of local standards which might support longer numbers.
+ This is why we have picked a very generous 100 character length to support heavily
+ formatted local numbers.
+ """
+
+ PHONE_NUMBER_REGEX = rf'^[0-9NnXx,+._*()#=;/ -]{{1,{MAX_PHONE_NUMBER_LENGTH}}}$'
+ """
+ Allow common punctuation used in phone numbers and spaces to allow formatting,
+ but otherwise don't allow text as the phone number should work as a link on mobile
+ devices.
+ Duplicated in the frontend code at, please keep in sync:
+ web-frontend/modules/core/utils/string.js#isSimplePhoneNumber
+ """
+
+ simple_phone_number_validator = RegexValidator(
+ regex=PHONE_NUMBER_REGEX)
+
+ def prepare_value_for_db(self, instance, value):
+ if value == '' or value is None:
+ return ''
+ self.simple_phone_number_validator(value)
+
+ return value
+
+ def get_serializer_field(self, instance, **kwargs):
+ return serializers.CharField(
+ required=False,
+ allow_null=True,
+ allow_blank=True,
+ validators=[self.simple_phone_number_validator],
+ max_length=self.MAX_PHONE_NUMBER_LENGTH,
+ **kwargs
+ )
+
+ def get_model_field(self, instance, **kwargs):
+ return models.CharField(
+ default='',
+ blank=True,
+ null=True,
+ max_length=self.MAX_PHONE_NUMBER_LENGTH,
+ validators=[
+ self.simple_phone_number_validator],
+ **kwargs)
+
+ def random_value(self, instance, fake, cache):
+ return fake.phone_number()
+
+ def get_alter_column_prepare_new_value(self, connection, from_field, to_field):
+ if connection.vendor == 'postgresql':
+ return f'''p_in = (
+ case
+ when p_in::text ~* '{self.PHONE_NUMBER_REGEX}'
+ then p_in::text
+ else ''
+ end
+ );'''
+
+ return super().get_alter_column_prepare_new_value(connection, from_field,
+ to_field)
+
+ def contains_query(self, *args):
+ return contains_filter(*args)
diff --git a/backend/src/baserow/contrib/database/fields/handler.py b/backend/src/baserow/contrib/database/fields/handler.py
index 366e5665d..419afc1df 100644
--- a/backend/src/baserow/contrib/database/fields/handler.py
+++ b/backend/src/baserow/contrib/database/fields/handler.py
@@ -1,23 +1,21 @@
import logging
from copy import deepcopy
+from django.conf import settings
from django.db import connections
from django.db.utils import ProgrammingError, DataError
-from django.conf import settings
-from baserow.core.utils import extract_allowed, set_allowed_attrs
from baserow.contrib.database.db.schema import lenient_schema_editor
from baserow.contrib.database.views.handler import ViewHandler
-
+from baserow.core.utils import extract_allowed, set_allowed_attrs
from .exceptions import (
PrimaryFieldAlreadyExists, CannotDeletePrimaryField, CannotChangeFieldType,
FieldDoesNotExist, IncompatiblePrimaryFieldTypeError
)
-from .registries import field_type_registry, field_converter_registry
from .models import Field, SelectOption
+from .registries import field_type_registry, field_converter_registry
from .signals import field_created, field_updated, field_deleted
-
logger = logging.getLogger(__name__)
@@ -159,7 +157,8 @@ class FieldHandler:
# If the provided field type does not match with the current one we need to
# migrate the field to the new type. Because the type has changed we also need
# to remove all view filters.
- if new_type_name and field_type.type != new_type_name:
+ baserow_field_type_changed = new_type_name and field_type.type != new_type_name
+ if baserow_field_type_changed:
field_type = field_type_registry.get(new_type_name)
if field.primary and not field_type.can_be_primary_field:
@@ -217,6 +216,17 @@ class FieldHandler:
connection
)
else:
+ if baserow_field_type_changed:
+ # If the baserow type has changed we always want to force run any alter
+ # column SQL as otherwise it might not run if the two baserow fields
+ # share the same underlying database column type.
+ force_alter_column = True
+ else:
+ force_alter_column = field_type.force_same_type_alter_column(
+ old_field,
+ field
+ )
+
# If no field converter is found we are going to alter the field using the
# the lenient schema editor.
with lenient_schema_editor(
@@ -225,7 +235,8 @@ class FieldHandler:
connection, old_field, field),
field_type.get_alter_column_prepare_new_value(
connection, old_field, field
- )
+ ),
+ force_alter_column
) as schema_editor:
try:
schema_editor.alter_field(from_model, from_model_field,
diff --git a/backend/src/baserow/contrib/database/fields/models.py b/backend/src/baserow/contrib/database/fields/models.py
index c232281e8..924a65a18 100644
--- a/backend/src/baserow/contrib/database/fields/models.py
+++ b/backend/src/baserow/contrib/database/fields/models.py
@@ -304,3 +304,7 @@ class FileField(Field):
class SingleSelectField(Field):
pass
+
+
+class PhoneNumberField(Field):
+ pass
diff --git a/backend/src/baserow/contrib/database/fields/registries.py b/backend/src/baserow/contrib/database/fields/registries.py
index b2a109756..d524661a7 100644
--- a/backend/src/baserow/contrib/database/fields/registries.py
+++ b/backend/src/baserow/contrib/database/fields/registries.py
@@ -215,6 +215,9 @@ class FieldType(MapAPIExceptionsInstanceMixin, APIUrlsInstanceMixin,
"""
Can return an SQL statement to convert the `p_in` variable to a readable text
format for the new field.
+ This SQL will not be run when converting between two fields of the same
+ baserow type which share the same underlying database column type.
+ If you require this then implement force_same_type_alter_column.
Example: return "p_in = lower(p_in);"
@@ -236,8 +239,11 @@ class FieldType(MapAPIExceptionsInstanceMixin, APIUrlsInstanceMixin,
"""
Can return a SQL statement to convert the `p_in` variable from text to a
desired format for the new field.
+ This SQL will not be run when converting between two fields of the same
+ baserow type which share the same underlying database column type.
+ If you require this then implement force_same_type_alter_column.
- Example when a string is converted to a number, to statement could be:
+ Example: when a string is converted to a number, to statement could be:
`REGEXP_REPLACE(p_in, '[^0-9]', '', 'g')` which would remove all non numeric
characters. The p_in variable is the old value as a string.
@@ -410,6 +416,30 @@ class FieldType(MapAPIExceptionsInstanceMixin, APIUrlsInstanceMixin,
return None
+ def force_same_type_alter_column(self, from_field, to_field):
+ """
+ Defines whether the sql provided by the get_alter_column_prepare_{old,new}_value
+ hooks should be forced to run when converting between two fields of this field
+ type which have the same database column type.
+ You only need to implement this when when you have validation and/or data
+ manipulation running as part of your alter_column_prepare SQL which must be
+ run even when from_field and to_field are the same Baserow field type and sql
+ column type. If your field has the same baserow type but will convert into
+ different sql column types then the alter sql will be run automatically and you
+ do not need to use this override.
+
+ :param from_field: The old field instance. It is not recommended to call the
+ save function as this will undo part of the changes that have been made.
+ This is just for comparing values.
+ :type from_field: Field
+ :param to_field: The updated field instance.
+ :type: to_field: Field
+ :return: Whether the alter column sql should be forced to run.
+ :rtype: bool
+ """
+
+ return False
+
class FieldTypeRegistry(APIUrlsRegistryMixin, CustomFieldsRegistryMixin,
ModelRegistryMixin, Registry):
diff --git a/backend/src/baserow/contrib/database/migrations/0029_phonenumberfield.py b/backend/src/baserow/contrib/database/migrations/0029_phonenumberfield.py
new file mode 100644
index 000000000..aea3d0fc4
--- /dev/null
+++ b/backend/src/baserow/contrib/database/migrations/0029_phonenumberfield.py
@@ -0,0 +1,29 @@
+# Generated by Django 2.2.11 on 2021-03-15 09:28
+
+from django.db import migrations, models
+import django.db.models.deletion
+
+
+class Migration(migrations.Migration):
+ dependencies = [
+ ('database', '0028_fix_negative_date'),
+ ]
+
+ operations = [
+ migrations.CreateModel(
+ name='PhoneNumberField',
+ fields=[
+ ('field_ptr', models.OneToOneField(
+ auto_created=True,
+ on_delete=django.db.models.deletion.CASCADE,
+ parent_link=True, primary_key=True,
+ serialize=False,
+ to='database.Field'
+ )),
+ ],
+ options={
+ 'abstract': False,
+ },
+ bases=('database.field',),
+ ),
+ ]
diff --git a/backend/src/baserow/contrib/database/models.py b/backend/src/baserow/contrib/database/models.py
index a28b5d8a8..98b8d40bb 100644
--- a/backend/src/baserow/contrib/database/models.py
+++ b/backend/src/baserow/contrib/database/models.py
@@ -4,7 +4,7 @@ from .table.models import Table
from .views.models import View, GridView, GridViewFieldOptions, ViewFilter
from .fields.models import (
Field, TextField, NumberField, LongTextField, BooleanField, DateField, LinkRowField,
- URLField, EmailField
+ URLField, EmailField, PhoneNumberField
)
from .tokens.models import Token, TokenPermission
@@ -13,7 +13,7 @@ __all__ = [
'Table',
'View', 'GridView', 'GridViewFieldOptions', 'ViewFilter',
'Field', 'TextField', 'NumberField', 'LongTextField', 'BooleanField', 'DateField',
- 'LinkRowField', 'URLField', 'EmailField',
+ 'LinkRowField', 'URLField', 'EmailField', 'PhoneNumberField',
'Token', 'TokenPermission'
]
diff --git a/backend/src/baserow/contrib/database/views/view_filters.py b/backend/src/baserow/contrib/database/views/view_filters.py
index 49cb8d99d..1758f36e4 100644
--- a/backend/src/baserow/contrib/database/views/view_filters.py
+++ b/backend/src/baserow/contrib/database/views/view_filters.py
@@ -11,7 +11,7 @@ from pytz import timezone
from baserow.contrib.database.fields.field_types import (
TextFieldType, LongTextFieldType, URLFieldType, NumberFieldType, DateFieldType,
LinkRowFieldType, BooleanFieldType, EmailFieldType, FileFieldType,
- SingleSelectFieldType
+ SingleSelectFieldType, PhoneNumberFieldType
)
from .registries import ViewFilterType
from baserow.contrib.database.fields.field_filters import contains_filter, \
@@ -38,7 +38,8 @@ class EqualViewFilterType(ViewFilterType):
URLFieldType.type,
NumberFieldType.type,
BooleanFieldType.type,
- EmailFieldType.type
+ EmailFieldType.type,
+ PhoneNumberFieldType.type
]
def get_filter(self, field_name, value, model_field, field):
@@ -89,7 +90,8 @@ class ContainsViewFilterType(ViewFilterType):
TextFieldType.type,
LongTextFieldType.type,
URLFieldType.type,
- EmailFieldType.type
+ EmailFieldType.type,
+ PhoneNumberFieldType.type
]
def get_filter(self, *args):
@@ -285,7 +287,8 @@ class EmptyViewFilterType(ViewFilterType):
LinkRowFieldType.type,
EmailFieldType.type,
FileFieldType.type,
- SingleSelectFieldType.type
+ SingleSelectFieldType.type,
+ PhoneNumberFieldType.type
]
def get_filter(self, field_name, value, model_field, field):
diff --git a/backend/tests/baserow/contrib/database/api/fields/test_field_views_types.py b/backend/tests/baserow/contrib/database/api/fields/test_field_views_types.py
index 5737d6c47..c8a32ef93 100644
--- a/backend/tests/baserow/contrib/database/api/fields/test_field_views_types.py
+++ b/backend/tests/baserow/contrib/database/api/fields/test_field_views_types.py
@@ -10,7 +10,8 @@ from rest_framework.status import HTTP_200_OK, HTTP_204_NO_CONTENT, HTTP_400_BAD
from django.shortcuts import reverse
from baserow.contrib.database.fields.models import (
- LongTextField, URLField, DateField, EmailField, FileField, NumberField
+ LongTextField, URLField, DateField, EmailField, FileField, NumberField,
+ PhoneNumberField
)
@@ -842,3 +843,96 @@ def test_number_field_type(api_client, data_fixture):
response_json['detail'][f'field_{positive_int_field_id}'][0]['code'] ==
'max_digits'
)
+
+
+@pytest.mark.django_db
+def test_phone_number_field_type(api_client, data_fixture):
+ user, token = data_fixture.create_user_and_token(
+ email='test@test.nl', password='password', first_name='Test1')
+ table = data_fixture.create_database_table(user=user)
+
+ response = api_client.post(
+ reverse('api:database:fields:list', kwargs={'table_id': table.id}),
+ {'name': 'phone', 'type': 'phone_number'},
+ format='json',
+ HTTP_AUTHORIZATION=f'JWT {token}'
+ )
+ response_json = response.json()
+ assert response.status_code == HTTP_200_OK
+ assert response_json['type'] == 'phone_number'
+ assert PhoneNumberField.objects.all().count() == 1
+ field_id = response_json['id']
+
+ response = api_client.patch(
+ reverse('api:database:fields:item', kwargs={'field_id': field_id}),
+ {'name': 'Phone'},
+ format='json',
+ HTTP_AUTHORIZATION=f'JWT {token}'
+ )
+ assert response.status_code == HTTP_200_OK
+
+ expected_phone_number = '+44761198672'
+
+ response = api_client.post(
+ reverse('api:database:rows:list', kwargs={'table_id': table.id}),
+ {
+ f'field_{field_id}': expected_phone_number
+ },
+ format='json',
+ HTTP_AUTHORIZATION=f'JWT {token}'
+ )
+ response_json = response.json()
+ assert response.status_code == HTTP_200_OK
+ assert response_json[f'field_{field_id}'] == expected_phone_number
+
+ model = table.get_model(attribute_names=True)
+ row = model.objects.all().last()
+ assert row.phone == expected_phone_number
+
+ response = api_client.post(
+ reverse('api:database:rows:list', kwargs={'table_id': table.id}),
+ {
+ f'field_{field_id}': ''
+ },
+ format='json',
+ HTTP_AUTHORIZATION=f'JWT {token}'
+ )
+ response_json = response.json()
+ assert response.status_code == HTTP_200_OK
+ assert response_json[f'field_{field_id}'] == ''
+
+ row = model.objects.all().last()
+ assert row.phone == ''
+
+ response = api_client.post(
+ reverse('api:database:rows:list', kwargs={'table_id': table.id}),
+ {
+ f'field_{field_id}': None
+ },
+ format='json',
+ HTTP_AUTHORIZATION=f'JWT {token}'
+ )
+ response_json = response.json()
+ assert response.status_code == HTTP_200_OK
+ assert response_json[f'field_{field_id}'] == ''
+
+ row = model.objects.all().last()
+ assert row.phone == ''
+
+ response = api_client.post(
+ reverse('api:database:rows:list', kwargs={'table_id': table.id}),
+ {},
+ format='json',
+ HTTP_AUTHORIZATION=f'JWT {token}'
+ )
+ response_json = response.json()
+ assert response.status_code == HTTP_200_OK
+ assert response_json[f'field_{field_id}'] == ''
+
+ row = model.objects.all().last()
+ assert row.phone == ''
+
+ email = reverse('api:database:fields:item', kwargs={'field_id': field_id})
+ response = api_client.delete(email, HTTP_AUTHORIZATION=f'JWT {token}')
+ assert response.status_code == HTTP_204_NO_CONTENT
+ assert PhoneNumberField.objects.all().count() == 0
diff --git a/backend/tests/baserow/contrib/database/db/test_db_schema.py b/backend/tests/baserow/contrib/database/db/test_db_schema.py
index 7e8f68d3c..4b5a394d6 100644
--- a/backend/tests/baserow/contrib/database/db/test_db_schema.py
+++ b/backend/tests/baserow/contrib/database/db/test_db_schema.py
@@ -1,5 +1,4 @@
import pytest
-
from django.db import connection
from django.db.backends.base.schema import BaseDatabaseSchemaEditor
from django.db.backends.dummy.base import DatabaseWrapper as DummyDatabaseWrapper
@@ -26,6 +25,7 @@ def test_lenient_schema_editor():
assert isinstance(schema_editor, BaseDatabaseSchemaEditor)
assert schema_editor.alter_column_prepare_old_value == ''
assert schema_editor.alter_column_prepare_new_value == ''
+ assert not schema_editor.force_alter_column
assert connection.SchemaEditorClass != PostgresqlDatabaseSchemaEditor
assert connection.SchemaEditorClass == PostgresqlDatabaseSchemaEditor
@@ -33,7 +33,8 @@ def test_lenient_schema_editor():
with lenient_schema_editor(
connection,
"p_in = REGEXP_REPLACE(p_in, '', 'test', 'g');",
- "p_in = REGEXP_REPLACE(p_in, 'test', '', 'g');"
+ "p_in = REGEXP_REPLACE(p_in, 'test', '', 'g');",
+ True
) as schema_editor:
assert schema_editor.alter_column_prepare_old_value == (
"p_in = REGEXP_REPLACE(p_in, '', 'test', 'g');"
@@ -41,3 +42,4 @@ def test_lenient_schema_editor():
assert schema_editor.alter_column_prepare_new_value == (
"p_in = REGEXP_REPLACE(p_in, 'test', '', 'g');"
)
+ assert schema_editor.force_alter_column
diff --git a/backend/tests/baserow/contrib/database/field/test_field_handler.py b/backend/tests/baserow/contrib/database/field/test_field_handler.py
index 0e3166557..549398686 100644
--- a/backend/tests/baserow/contrib/database/field/test_field_handler.py
+++ b/backend/tests/baserow/contrib/database/field/test_field_handler.py
@@ -1,18 +1,126 @@
-import pytest
+import itertools
+from datetime import date
from decimal import Decimal
from unittest.mock import patch
-from baserow.core.exceptions import UserNotInGroupError
-from baserow.contrib.database.fields.handler import FieldHandler
-from baserow.contrib.database.fields.models import (
- Field, TextField, NumberField, BooleanField, SelectOption
-)
-from baserow.contrib.database.fields.field_types import TextFieldType
-from baserow.contrib.database.fields.registries import field_type_registry
+import pytest
+from django.db import models
+from faker import Faker
+
from baserow.contrib.database.fields.exceptions import (
FieldTypeDoesNotExist, PrimaryFieldAlreadyExists, CannotDeletePrimaryField,
FieldDoesNotExist, IncompatiblePrimaryFieldTypeError, CannotChangeFieldType
)
+from baserow.contrib.database.fields.field_types import TextFieldType, LongTextFieldType
+from baserow.contrib.database.fields.handler import FieldHandler
+from baserow.contrib.database.fields.models import (
+ Field, TextField, NumberField, BooleanField, SelectOption, LongTextField,
+ NUMBER_TYPE_CHOICES
+)
+from baserow.contrib.database.fields.registries import field_type_registry
+from baserow.contrib.database.rows.handler import RowHandler
+from baserow.core.exceptions import UserNotInGroupError
+
+
+def dict_to_pairs(field_type_kwargs):
+ pairs_dict = {}
+ for name, options in field_type_kwargs.items():
+ pairs_dict[name] = []
+ if not isinstance(options, list):
+ options = [options]
+ for option in options:
+ pairs_dict[name].append((name, option))
+ return pairs_dict
+
+
+def construct_all_possible_kwargs(field_type_kwargs):
+ pairs_dict = dict_to_pairs(field_type_kwargs)
+ args = [dict(pairwise_args) for pairwise_args in itertools.product(
+ *pairs_dict.values())]
+
+ return args
+
+
+# You must add --runslow to pytest to run this test, you can do this in intellij by
+# editing the run config for this test and adding --runslow to additional args.
+@pytest.mark.django_db
+@pytest.mark.slow
+def test_can_convert_between_all_fields(data_fixture):
+ """
+ A nuclear option test turned off by default to help verify changes made to
+ field conversions work in every possible conversion scenario. This test checks
+ is possible to convert from every possible field to every other possible field
+ including converting to themselves. It only checks that the conversion does not
+ raise any exceptions.
+ """
+
+ user = data_fixture.create_user()
+ database = data_fixture.create_database_application(user=user)
+ table = data_fixture.create_database_table(database=database, user=user)
+ link_table = data_fixture.create_database_table(database=database, user=user)
+ handler = FieldHandler()
+ row_handler = RowHandler()
+ fake = Faker()
+
+ model = table.get_model()
+ cache = {}
+ # Make a blank row to test empty field conversion also.
+ model.objects.create(**{})
+ second_row_with_values = model.objects.create(**{})
+
+ # Some baserow field types have multiple different 'modes' which result in
+ # different conversion behaviour or entirely different database columns being
+ # created. Here the kwargs which control these modes are enumerated so we can then
+ # generate every possible type of conversion.
+ extra_kwargs_for_type = {
+ 'date': {
+ 'date_include_time': [True, False],
+ },
+ 'number': {
+ 'number_type': [number_type for number_type, _ in NUMBER_TYPE_CHOICES],
+ 'number_negative': [True, False],
+ },
+ 'link_row': {
+ 'link_row_table': link_table
+ }
+ }
+
+ all_possible_kwargs_per_type = {}
+ for field_type_name in field_type_registry.get_types():
+ extra_kwargs = extra_kwargs_for_type.get(field_type_name, {})
+ all_possible_kwargs = construct_all_possible_kwargs(extra_kwargs)
+ all_possible_kwargs_per_type[field_type_name] = all_possible_kwargs
+
+ i = 1
+ for field_type_name, all_possible_kwargs in all_possible_kwargs_per_type.items():
+ for kwargs in all_possible_kwargs:
+ for inner_field_type_name in field_type_registry.get_types():
+ for inner_kwargs in all_possible_kwargs_per_type[inner_field_type_name]:
+ field_type = field_type_registry.get(field_type_name)
+ field_name = f'field_{i}'
+ from_field = handler.create_field(
+ user=user, table=table, type_name=field_type_name,
+ name=field_name,
+ **kwargs
+ )
+ random_value = field_type.random_value(
+ from_field,
+ fake,
+ cache
+ )
+ if isinstance(random_value, date):
+ # Faker produces subtypes of date / datetime which baserow
+ # does not want, instead just convert to str.
+ random_value = str(random_value)
+ row_handler.update_row(user=user, table=table,
+ row_id=second_row_with_values.id,
+ values={
+ f'field_{from_field.id}': random_value
+ })
+ handler.update_field(user=user, field=from_field,
+ new_type_name=inner_field_type_name,
+ **inner_kwargs)
+ i = i + 1
@pytest.mark.django_db
@@ -269,6 +377,295 @@ def test_update_field_failing(data_fixture):
assert TextField.objects.all().count() == 1
+@pytest.mark.django_db
+def test_update_field_when_underlying_sql_type_doesnt_change(data_fixture):
+ class AlwaysLowercaseTextField(TextFieldType):
+ type = 'lowercase_text'
+ model_class = LongTextField
+
+ def get_alter_column_prepare_new_value(self, connection, from_field, to_field):
+ return '''p_in = (lower(p_in));'''
+
+ user = data_fixture.create_user()
+ table = data_fixture.create_database_table(user=user)
+ existing_text_field = data_fixture.create_text_field(table=table, order=1)
+
+ model = table.get_model()
+
+ field_name = f'field_{existing_text_field.id}'
+ row = model.objects.create(**{
+ field_name: 'Test',
+ })
+
+ handler = FieldHandler()
+
+ with patch.dict(
+ field_type_registry.registry,
+ {'lowercase_text': AlwaysLowercaseTextField()}
+ ):
+ handler.update_field(user=user,
+ field=existing_text_field,
+ new_type_name='lowercase_text')
+
+ row.refresh_from_db()
+ assert getattr(row, field_name) == 'test'
+ assert Field.objects.all().count() == 1
+ assert TextField.objects.all().count() == 0
+ assert LongTextField.objects.all().count() == 1
+
+
+@pytest.mark.django_db
+def test_field_which_changes_its_underlying_type_will_have_alter_sql_run(data_fixture):
+ class ReversingTextFieldUsingBothVarCharAndTextSqlTypes(TextFieldType):
+
+ def get_alter_column_prepare_new_value(self, connection, from_field, to_field):
+ return '''p_in = (reverse(p_in));'''
+
+ def get_model_field(self, instance, **kwargs):
+ kwargs['null'] = True
+ kwargs['blank'] = True
+ if instance.text_default == 'use_other_sql_type':
+ return models.TextField(**kwargs)
+ else:
+ return models.CharField(**kwargs)
+
+ user = data_fixture.create_user()
+ table = data_fixture.create_database_table(user=user)
+ existing_text_field = data_fixture.create_text_field(table=table, order=1)
+
+ model = table.get_model()
+
+ field_name = f'field_{existing_text_field.id}'
+ row = model.objects.create(**{
+ field_name: 'Test',
+ })
+
+ handler = FieldHandler()
+
+ with patch.dict(
+ field_type_registry.registry,
+ {'text': ReversingTextFieldUsingBothVarCharAndTextSqlTypes()}
+ ):
+ # Update to the same baserow type, but due to this fields implementation of
+ # get_model_field this will alter the underlying database column from type
+ # of varchar to text, which should make our reversing alter sql run.
+ handler.update_field(user=user,
+ field=existing_text_field,
+ new_type_name='text',
+ text_default='use_other_sql_type')
+
+ row.refresh_from_db()
+ assert getattr(row, field_name) == 'tseT'
+ assert Field.objects.all().count() == 1
+ assert TextField.objects.all().count() == 1
+
+
+@pytest.mark.django_db
+def test_just_changing_a_fields_name_will_not_run_alter_sql(data_fixture):
+ class AlwaysReverseOnUpdateField(TextFieldType):
+ def get_alter_column_prepare_new_value(self, connection, from_field, to_field):
+ return '''p_in = (reverse(p_in));'''
+
+ user = data_fixture.create_user()
+ table = data_fixture.create_database_table(user=user)
+ existing_text_field = data_fixture.create_text_field(table=table, order=1)
+
+ model = table.get_model()
+
+ field_name = f'field_{existing_text_field.id}'
+ row = model.objects.create(**{
+ field_name: 'Test',
+ })
+
+ handler = FieldHandler()
+
+ with patch.dict(
+ field_type_registry.registry,
+ {'text': AlwaysReverseOnUpdateField()}
+ ):
+ handler.update_field(user=user, field=existing_text_field,
+ new_type_name='text', name='new_name')
+
+ row.refresh_from_db()
+ # The field has not been reversed as just the name changed!
+ assert getattr(row, field_name) == 'Test'
+ assert Field.objects.all().count() == 1
+ assert TextField.objects.all().count() == 1
+
+
+@pytest.mark.django_db
+def test_when_field_type_forces_same_type_alter_fields_alter_sql_is_run(data_fixture):
+ class SameTypeAlwaysReverseOnUpdateField(TextFieldType):
+ def get_alter_column_prepare_new_value(self, connection, from_field, to_field):
+ return '''p_in = (reverse(p_in));'''
+
+ def force_same_type_alter_column(self, from_field, to_field):
+ return True
+
+ user = data_fixture.create_user()
+ table = data_fixture.create_database_table(user=user)
+ existing_text_field = data_fixture.create_text_field(table=table, order=1)
+
+ model = table.get_model()
+
+ field_name = f'field_{existing_text_field.id}'
+ row = model.objects.create(**{
+ field_name: 'Test',
+ })
+
+ handler = FieldHandler()
+
+ with patch.dict(
+ field_type_registry.registry,
+ {'text': SameTypeAlwaysReverseOnUpdateField()}
+ ):
+ handler.update_field(user=user, field=existing_text_field,
+ new_type_name='text', name='new_name')
+
+ row.refresh_from_db()
+ # The alter sql has been run due to the force override
+ assert getattr(row, field_name) == 'tseT'
+ assert Field.objects.all().count() == 1
+ assert TextField.objects.all().count() == 1
+
+
+@pytest.mark.django_db
+def test_update_field_with_type_error_on_conversion_should_null_field(data_fixture):
+ class AlwaysThrowsSqlExceptionOnConversionField(TextFieldType):
+ type = 'throws_field'
+ model_class = LongTextField
+
+ def get_alter_column_prepare_new_value(self, connection, from_field, to_field):
+ return '''p_in = (lower(p_in::numeric::text));'''
+
+ user = data_fixture.create_user()
+ table = data_fixture.create_database_table(user=user)
+ existing_text_field = data_fixture.create_text_field(table=table, order=1)
+
+ model = table.get_model()
+
+ field_name = f'field_{existing_text_field.id}'
+ row = model.objects.create(**{
+ field_name: 'Test',
+ })
+
+ handler = FieldHandler()
+
+ with patch.dict(
+ field_type_registry.registry,
+ {'throws_field': AlwaysThrowsSqlExceptionOnConversionField()}
+ ):
+ handler.update_field(user=user,
+ field=existing_text_field,
+ new_type_name='throws_field')
+
+ row.refresh_from_db()
+ assert getattr(row, field_name) is None
+ assert Field.objects.all().count() == 1
+ assert TextField.objects.all().count() == 0
+ assert LongTextField.objects.all().count() == 1
+
+
+@pytest.mark.django_db
+def test_update_field_when_underlying_sql_type_doesnt_change_with_vars(data_fixture):
+ class ReversesWhenConvertsAwayTextField(LongTextFieldType):
+ type = 'reserves_text'
+ model_class = LongTextField
+
+ def get_alter_column_prepare_old_value(self, connection, from_field, to_field):
+ return '''p_in = concat(reverse(p_in), %(some_variable)s);''', {
+ "some_variable": "_POST_FIX"
+ }
+
+ class AlwaysLowercaseTextField(TextFieldType):
+ type = 'lowercase_text'
+ model_class = LongTextField
+
+ def get_alter_column_prepare_new_value(self, connection, from_field, to_field):
+ return '''p_in = concat(%(other_variable)s, lower(p_in));''', {
+ "other_variable": "pre_fix_"
+ }
+
+ user = data_fixture.create_user()
+ table = data_fixture.create_database_table(user=user)
+ existing_field_with_old_value_prep = data_fixture.create_long_text_field(
+ table=table)
+
+ model = table.get_model()
+
+ field_name = f'field_{existing_field_with_old_value_prep.id}'
+ row = model.objects.create(**{
+ field_name: 'Test',
+ })
+
+ handler = FieldHandler()
+
+ with patch.dict(
+ field_type_registry.registry,
+ {
+ 'lowercase_text': AlwaysLowercaseTextField(),
+ 'long_text': ReversesWhenConvertsAwayTextField()
+ }
+ ):
+ handler.update_field(user=user,
+ field=existing_field_with_old_value_prep,
+ new_type_name='lowercase_text')
+
+ row.refresh_from_db()
+ assert getattr(row, field_name) == 'pre_fix_tset_post_fix'
+ assert Field.objects.all().count() == 1
+ assert TextField.objects.all().count() == 0
+ assert LongTextField.objects.all().count() == 1
+
+
+@pytest.mark.django_db
+def test_update_field_when_underlying_sql_type_doesnt_change_old_prep(data_fixture):
+ class ReversesWhenConvertsAwayTextField(LongTextFieldType):
+ type = 'reserves_text'
+ model_class = LongTextField
+
+ def get_alter_column_prepare_old_value(self, connection, from_field, to_field):
+ return '''p_in = (reverse(p_in));'''
+
+ class AlwaysLowercaseTextField(TextFieldType):
+ type = 'lowercase_text'
+ model_class = LongTextField
+
+ def get_alter_column_prepare_new_value(self, connection, from_field, to_field):
+ return '''p_in = (lower(p_in));'''
+
+ user = data_fixture.create_user()
+ table = data_fixture.create_database_table(user=user)
+ existing_field_with_old_value_prep = data_fixture.create_long_text_field(
+ table=table)
+
+ model = table.get_model()
+
+ field_name = f'field_{existing_field_with_old_value_prep.id}'
+ row = model.objects.create(**{
+ field_name: 'Test',
+ })
+
+ handler = FieldHandler()
+
+ with patch.dict(
+ field_type_registry.registry,
+ {
+ 'lowercase_text': AlwaysLowercaseTextField(),
+ 'long_text': ReversesWhenConvertsAwayTextField()
+ }
+ ):
+ handler.update_field(user=user,
+ field=existing_field_with_old_value_prep,
+ new_type_name='lowercase_text')
+
+ row.refresh_from_db()
+ assert getattr(row, field_name) == 'tset'
+ assert Field.objects.all().count() == 1
+ assert TextField.objects.all().count() == 0
+ assert LongTextField.objects.all().count() == 1
+
+
@pytest.mark.django_db
@patch('baserow.contrib.database.fields.signals.field_deleted.send')
def test_delete_field(send_mock, data_fixture):
diff --git a/backend/tests/baserow/contrib/database/field/test_field_types.py b/backend/tests/baserow/contrib/database/field/test_field_types.py
index 1167f9979..89db5e150 100644
--- a/backend/tests/baserow/contrib/database/field/test_field_types.py
+++ b/backend/tests/baserow/contrib/database/field/test_field_types.py
@@ -1,15 +1,18 @@
import pytest
import json
+
+from django.test.utils import override_settings
from faker import Faker
from decimal import Decimal
from django.core.exceptions import ValidationError
+from baserow.contrib.database.fields.field_types import PhoneNumberFieldType
from baserow.core.user_files.exceptions import (
InvalidUserFileNameError, UserFileDoesNotExist
)
from baserow.contrib.database.fields.models import (
- LongTextField, URLField, EmailField, FileField
+ LongTextField, URLField, EmailField, FileField, PhoneNumberField
)
from baserow.contrib.database.fields.handler import FieldHandler
from baserow.contrib.database.rows.handler import RowHandler
@@ -511,3 +514,102 @@ def test_file_field_type(data_fixture):
assert results[0].text is None
assert results[1].text is None
assert results[2].text is None
+
+
+@pytest.mark.django_db
+@override_settings(debug=True)
+def test_phone_number_field_type(data_fixture):
+ user = data_fixture.create_user()
+ table = data_fixture.create_database_table(user=user)
+ data_fixture.create_database_table(user=user, database=table.database)
+
+ field_handler = FieldHandler()
+ row_handler = RowHandler()
+
+ text_field = field_handler.create_field(user=user, table=table,
+ order=1,
+ type_name='text',
+ name='name')
+ phone_number_field = field_handler.create_field(user=user, table=table,
+ type_name='phone_number',
+ name='phonenumber')
+ email_field = field_handler.create_field(user=user, table=table,
+ type_name='email',
+ name='email')
+ number_field = data_fixture.create_number_field(table=table, order=1,
+ number_negative=True, name="number")
+
+ assert len(PhoneNumberField.objects.all()) == 1
+ model = table.get_model(attribute_names=True)
+
+ with pytest.raises(ValidationError):
+ row_handler.create_row(user=user, table=table, values={
+ 'phonenumber': 'invalid phone number'
+ }, model=model)
+
+ with pytest.raises(ValidationError):
+ row_handler.create_row(user=user, table=table, values={
+ 'phonenumber': 'Phone: 2312321 2349432 '
+ }, model=model)
+ with pytest.raises(ValidationError):
+ row_handler.create_row(user=user, table=table, values={
+ 'phonenumber': '1' * (PhoneNumberFieldType.MAX_PHONE_NUMBER_LENGTH+1)
+ }, model=model)
+
+ max_length_phone_number = '1' * PhoneNumberFieldType.MAX_PHONE_NUMBER_LENGTH
+ row_handler.create_row(user=user, table=table, values={
+ 'name': '+45(1424) 322314 324234',
+ 'phonenumber': max_length_phone_number,
+ 'number': 1234534532,
+ 'email': 'a_valid_email_to_be_blanked_after_conversion@email.com'
+ }, model=model)
+ row_handler.create_row(user=user, table=table, values={
+ 'name': 'some text which should be blanked out after conversion',
+ 'phonenumber': '1234567890 NnXx,+._*()#=;/ -',
+ 'number': 0
+ }, model=model)
+ row_handler.create_row(user=user, table=table, values={
+ 'name': max_length_phone_number,
+ 'phonenumber': '',
+ 'number': -10230450,
+ }, model=model)
+ row_handler.create_row(user=user, table=table, values={
+ 'phonenumber': None,
+ 'name': '1' * (PhoneNumberFieldType.MAX_PHONE_NUMBER_LENGTH+1)
+
+ }, model=model)
+ row_handler.create_row(user=user, table=table, values={}, model=model)
+
+ # No actual database type change occurs here as a phone number field is also a text
+ # field. Instead the after_update hook is being used to clear out invalid
+ # phone numbers.
+ field_handler.update_field(user=user, field=text_field,
+ new_type_name='phone_number')
+
+ field_handler.update_field(user=user, field=number_field,
+ new_type_name='phone_number')
+ field_handler.update_field(user=user, field=email_field,
+ new_type_name='phone_number')
+
+ model = table.get_model(attribute_names=True)
+ rows = model.objects.all()
+
+ assert rows[0].name == '+45(1424) 322314 324234'
+ assert rows[0].phonenumber == max_length_phone_number
+ assert rows[0].number == '1234534532'
+ assert rows[0].email == ''
+
+ assert rows[1].name == ''
+ assert rows[1].phonenumber == '1234567890 NnXx,+._*()#=;/ -'
+ assert rows[1].number == '0'
+
+ assert rows[2].name == max_length_phone_number
+ assert rows[2].phonenumber == ''
+ assert rows[2].number == '-10230450'
+
+ assert rows[3].name == ''
+ assert rows[3].phonenumber == ''
+ assert rows[3].number == ''
+
+ field_handler.delete_field(user=user, field=phone_number_field)
+ assert len(PhoneNumberField.objects.all()) == 3
diff --git a/backend/tests/baserow/contrib/database/table/test_table_models.py b/backend/tests/baserow/contrib/database/table/test_table_models.py
index a6beb3c49..818d4879e 100644
--- a/backend/tests/baserow/contrib/database/table/test_table_models.py
+++ b/backend/tests/baserow/contrib/database/table/test_table_models.py
@@ -161,12 +161,13 @@ def test_search_all_fields_queryset(data_fixture, user_tables_in_separate_db):
date_format="US", date_include_time=True,
date_time_format="24")
data_fixture.create_file_field(table=table, order=6, name='File')
- select = data_fixture.create_single_select_field(table=table, order=6,
+ select = data_fixture.create_single_select_field(table=table, order=7,
name='select')
option_a = data_fixture.create_select_option(field=select, value='Option A',
color='blue')
option_b = data_fixture.create_select_option(field=select, value='Option B',
color='red')
+ data_fixture.create_phone_number_field(table=table, order=8, name='PhoneNumber')
model = table.get_model(attribute_names=True)
row_1 = model.objects.create(
@@ -178,6 +179,7 @@ def test_search_all_fields_queryset(data_fixture, user_tables_in_separate_db):
datetime=make_aware(datetime(4006, 7, 8, 0, 0, 0), utc),
file=[{'visible_name': 'test_file.png'}],
select=option_a,
+ phonenumber='99999'
)
row_2 = model.objects.create(
name='Audi',
@@ -188,6 +190,7 @@ def test_search_all_fields_queryset(data_fixture, user_tables_in_separate_db):
datetime=make_aware(datetime(5, 5, 5, 0, 48, 0), utc),
file=[{'visible_name': 'other_file.png'}],
select=option_b,
+ phonenumber='++--999999'
)
row_3 = model.objects.create(
name='Volkswagen',
@@ -197,6 +200,7 @@ def test_search_all_fields_queryset(data_fixture, user_tables_in_separate_db):
date='9999-05-05',
datetime=make_aware(datetime(5, 5, 5, 9, 59, 0), utc),
file=[],
+ phonenumber=''
)
results = model.objects.all().search_all_fields('FASTEST')
@@ -260,6 +264,15 @@ def test_search_all_fields_queryset(data_fixture, user_tables_in_separate_db):
assert len(results) == 1
assert row_2 in results
+ results = model.objects.all().search_all_fields('999999')
+ assert len(results) == 1
+ assert row_2 in results
+
+ results = model.objects.all().search_all_fields('99999')
+ assert len(results) == 2
+ assert row_1 in results
+ assert row_2 in results
+
results = model.objects.all().search_all_fields('white car')
assert len(results) == 0
diff --git a/backend/tests/conftest.py b/backend/tests/conftest.py
index 3381133e9..0a6d9ae09 100644
--- a/backend/tests/conftest.py
+++ b/backend/tests/conftest.py
@@ -18,6 +18,26 @@ def api_client():
return APIClient()
+def pytest_addoption(parser):
+ parser.addoption(
+ "--runslow", action="store_true", default=False, help="run slow tests"
+ )
+
+
+def pytest_configure(config):
+ config.addinivalue_line("markers", "slow: mark test as slow to run")
+
+
+def pytest_collection_modifyitems(config, items):
+ if config.getoption("--runslow"):
+ # --runslow given in cli: do not skip slow tests
+ return
+ skip_slow = pytest.mark.skip(reason="need --runslow option to run")
+ for item in items:
+ if "slow" in item.keywords:
+ item.add_marker(skip_slow)
+
+
def run_non_transactional_raw_sql(sqls, dbinfo):
conn = psycopg2.connect(host=dbinfo['HOST'], user=dbinfo['USER'],
password=dbinfo['PASSWORD'],
diff --git a/backend/tests/fixtures/field.py b/backend/tests/fixtures/field.py
index ab9787411..b67ced4c4 100644
--- a/backend/tests/fixtures/field.py
+++ b/backend/tests/fixtures/field.py
@@ -3,7 +3,7 @@ from django.db import connections
from baserow.contrib.database.fields.models import (
TextField, LongTextField, NumberField, BooleanField, DateField, LinkRowField,
- FileField, SingleSelectField, SelectOption
+ FileField, SingleSelectField, SelectOption, URLField, EmailField, PhoneNumberField
)
@@ -167,3 +167,54 @@ class FieldFixtures:
self.create_model_field(kwargs['table'], field)
return field
+
+ def create_url_field(self, user=None, create_field=True, **kwargs):
+ if 'table' not in kwargs:
+ kwargs['table'] = self.create_database_table(user=user)
+
+ if 'name' not in kwargs:
+ kwargs['name'] = self.fake.url()
+
+ if 'order' not in kwargs:
+ kwargs['order'] = 0
+
+ field = URLField.objects.create(**kwargs)
+
+ if create_field:
+ self.create_model_field(kwargs['table'], field)
+
+ return field
+
+ def create_email_field(self, user=None, create_field=True, **kwargs):
+ if 'table' not in kwargs:
+ kwargs['table'] = self.create_database_table(user=user)
+
+ if 'name' not in kwargs:
+ kwargs['name'] = self.fake.email()
+
+ if 'order' not in kwargs:
+ kwargs['order'] = 0
+
+ field = EmailField.objects.create(**kwargs)
+
+ if create_field:
+ self.create_model_field(kwargs['table'], field)
+
+ return field
+
+ def create_phone_number_field(self, user=None, create_field=True, **kwargs):
+ if 'table' not in kwargs:
+ kwargs['table'] = self.create_database_table(user=user)
+
+ if 'name' not in kwargs:
+ kwargs['name'] = self.fake.phone_number()
+
+ if 'order' not in kwargs:
+ kwargs['order'] = 0
+
+ field = PhoneNumberField.objects.create(**kwargs)
+
+ if create_field:
+ self.create_model_field(kwargs['table'], field)
+
+ return field
diff --git a/changelog.md b/changelog.md
index 1cf1dc806..d37e12756 100644
--- a/changelog.md
+++ b/changelog.md
@@ -21,6 +21,7 @@
* Fixed SSRF bug in the file upload by URL by blocking urls to the private network.
* Fixed bug where an invalid date could be converted to 0001-01-01.
* The list_database_table_rows search query parameter now searches all possible field types.
+* Add Phone Number field.
## Released (2021-03-01)
diff --git a/docs/decisions/001-phone-number-field-validation.md b/docs/decisions/001-phone-number-field-validation.md
new file mode 100644
index 000000000..fc4102ec5
--- /dev/null
+++ b/docs/decisions/001-phone-number-field-validation.md
@@ -0,0 +1,107 @@
+# How to Validate and Format Phone Number Fields
+
+See this issue for background: https://gitlab.com/bramw/baserow/-/issues/339
+
+For the new number phone number field there are a number of options around how to
+validate and format the field. This document captures
+
+# Decision
+
+Option 1 was chosen because:
+
+- It is the simplest and there are no specific user requests for any of the more complex
+ options below, so instead we stick to the simple option.
+- We do not need to reason about the validation provided by two different open source
+ libraries and test that they agree which numbers are valid or not.
+
+## Option 1:
+
+Use a simple regex only allowing numbers, spaces and some punctuation to validate phone
+numbers. Assume every entered phone number which passes this regex is a valid number and
+display the number as a link (href="tel: {{value}}")
+when appropriate so the user can call the number.
+
+### Pros:
+
+- Simplest technically
+- No need to use external libraries
+- Most flexible for the user as they can enter any type of telephone format
+- We might be able to pass on this "validate the phone number" problem to the phone as
+ if it is formatted as a href="tel: xxx" link the phone might then format the opened
+ link
+
+### Cons:
+
+- Users can easily enter complete nonsense for phone numbers
+- Its upto the user to nicely format the phone numbers every single time
+
+## Option 2:
+
+Use a [python](https://github.com/daviddrysdale/python-phonenumbers)
+and [js library](https://github.com/catamphetamine/libphonenumber-js) both based
+off [google's phonenumberlib](https://github.com/google/libphonenumber) and validate
+that a number is "possible" which is a lower standard and less likely to change compared
+to "
+valid". Auto format the number based on it could be using any country code.
+
+### Pros
+
+- Dont need to store/allow configuring extra country code information
+- Nice formatting for international numbers
+- By using the least strict form of validation provided by the libraries we can be more
+ sure that the validations match between client and server.
+
+### Cons
+
+- Dont get nice country specific formatting unless the user enters a country code in the
+ number itself as otherwise the libraries cant detect what the country is.
+
+## Option 3:
+
+Option 2 but also allow user to specify a country code OR "international" on the whole
+field, format and validate numbers entered using this country code.
+
+### Pros
+
+- Get nice formatting for local numbers if you set the column
+
+### Cons
+
+- Limits users to only having one telephone format per column, cant mix numbers without
+ using international
+- Have to implement / design a country code select mechanism
+- Have to store and sync extra country code data on the field
+
+## Option 4:
+
+Option 2, but also allow user to set a default country code on the field and then let
+the user pick a country code per row which defaults to the fields default.
+
+### Pros
+
+- Users can mix every possible type of phone number in a single field and get nice
+ formatting and validation.
+- If they dont want to mix then they can fallback to option 2 by using a default code
+- If they dont want any country specific formatting they can fallback to option 1
+
+### Cons
+
+- Have to store country data per field and row
+- Have to design a row entry component which uses the default + lets users pick a
+ country code per field + column
+
+## Option 5:
+
+Only use a front-end library to format the entered phone numbers and check they are
+posible purely in the front-end, the backend does simple or no validation.
+
+### Pros
+
+- Don't need to worry about syncing the front and back end validation
+- Still get nice phone number entry and formatting
+
+### Cons
+
+- When a user converts a non-phone number field to being a phone number this happens on
+ the backend and hence no nice formatting will happen. Then when the user edits one
+ of these unformatted cells it will instantly change to be formatted.
diff --git a/web-frontend/modules/core/utils/string.js b/web-frontend/modules/core/utils/string.js
index d8c73a730..b400970fc 100644
--- a/web-frontend/modules/core/utils/string.js
+++ b/web-frontend/modules/core/utils/string.js
@@ -53,6 +53,15 @@ export const isValidEmail = (str) => {
return !!pattern.test(str)
}
+// Regex duplicated from
+// src/baserow/contrib/database/fields/field_types.py#PhoneNumberFieldType
+// Docs reference what characters are valid in PhoneNumberFieldType.getDocsDescription
+// Ensure they are kept in sync.
+export const isSimplePhoneNumber = (str) => {
+ const pattern = /^[0-9NnXx,+._*()#=;/ -]{1,100}$/
+ return pattern.test(str)
+}
+
export const isSecureURL = (str) => {
return str.toLowerCase().substr(0, 5) === 'https'
}
diff --git a/web-frontend/modules/database/components/row/RowEditFieldPhoneNumber.vue b/web-frontend/modules/database/components/row/RowEditFieldPhoneNumber.vue
new file mode 100644
index 000000000..7b736c15c
--- /dev/null
+++ b/web-frontend/modules/database/components/row/RowEditFieldPhoneNumber.vue
@@ -0,0 +1,27 @@
+<template>
+ <div class="control__elements">
+ <input
+ ref="input"
+ v-model="copy"
+ type="tel"
+ class="input input--large"
+ :class="{ 'input--error': !isValid() }"
+ @keyup.enter="$refs.input.blur()"
+ @focus="select()"
+ @blur="unselect()"
+ />
+ <div v-show="!isValid()" class="error">
+ {{ getError() }}
+ </div>
+ </div>
+</template>
+
+<script>
+import rowEditField from '@baserow/modules/database/mixins/rowEditField'
+import rowEditFieldInput from '@baserow/modules/database/mixins/rowEditFieldInput'
+import phoneNumberField from '@baserow/modules/database/mixins/phoneNumberField'
+
+export default {
+ mixins: [rowEditField, rowEditFieldInput, phoneNumberField],
+}
+</script>
diff --git a/web-frontend/modules/database/components/view/grid/fields/FunctionalGridViewFieldPhoneNumber.vue b/web-frontend/modules/database/components/view/grid/fields/FunctionalGridViewFieldPhoneNumber.vue
new file mode 100644
index 000000000..57096dbce
--- /dev/null
+++ b/web-frontend/modules/database/components/view/grid/fields/FunctionalGridViewFieldPhoneNumber.vue
@@ -0,0 +1,7 @@
+<template functional>
+ <div ref="cell" class="grid-view__cell">
+ <div class="grid-field-text">
+ {{ props.value }}
+ </div>
+ </div>
+</template>
diff --git a/web-frontend/modules/database/components/view/grid/fields/GridViewFieldPhoneNumber.vue b/web-frontend/modules/database/components/view/grid/fields/GridViewFieldPhoneNumber.vue
new file mode 100644
index 000000000..7a47ba782
--- /dev/null
+++ b/web-frontend/modules/database/components/view/grid/fields/GridViewFieldPhoneNumber.vue
@@ -0,0 +1,43 @@
+<template>
+ <div
+ class="grid-view__cell active"
+ :class="{
+ editing: editing,
+ invalid: editing && !isValid(),
+ }"
+ @contextmenu="stopContextIfEditing($event)"
+ >
+ <div v-show="!editing" class="grid-field-text">
+ <a :href="'tel:' + value" target="_blank">{{ value }}</a>
+ </div>
+ <template v-if="editing">
+ <input
+ ref="input"
+ v-model="copy"
+ type="tel"
+ class="grid-field-text__input"
+ />
+ <div v-show="!isValid()" class="grid-view__cell--error align-right">
+ {{ getError() }}
+ </div>
+ </template>
+ </div>
+</template>
+
+<script>
+import gridField from '@baserow/modules/database/mixins/gridField'
+import gridFieldInput from '@baserow/modules/database/mixins/gridFieldInput'
+import phoneNumberField from '@baserow/modules/database/mixins/phoneNumberField'
+
+export default {
+ mixins: [gridField, gridFieldInput, phoneNumberField],
+ methods: {
+ afterEdit() {
+ this.$nextTick(() => {
+ this.$refs.input.focus()
+ this.$refs.input.selectionStart = this.$refs.input.selectionEnd = 100000
+ })
+ },
+ },
+}
+</script>
diff --git a/web-frontend/modules/database/fieldTypes.js b/web-frontend/modules/database/fieldTypes.js
index 2d620d2af..9932f5fe4 100644
--- a/web-frontend/modules/database/fieldTypes.js
+++ b/web-frontend/modules/database/fieldTypes.js
@@ -1,7 +1,11 @@
import moment from 'moment'
import BigNumber from 'bignumber.js'
-import { isValidURL, isValidEmail } from '@baserow/modules/core/utils/string'
+import {
+ isValidURL,
+ isValidEmail,
+ isSimplePhoneNumber,
+} from '@baserow/modules/core/utils/string'
import { Registerable } from '@baserow/modules/core/registry'
import FieldNumberSubForm from '@baserow/modules/database/components/field/FieldNumberSubForm'
@@ -20,6 +24,7 @@ import GridViewFieldBoolean from '@baserow/modules/database/components/view/grid
import GridViewFieldDate from '@baserow/modules/database/components/view/grid/fields/GridViewFieldDate'
import GridViewFieldFile from '@baserow/modules/database/components/view/grid/fields/GridViewFieldFile'
import GridViewFieldSingleSelect from '@baserow/modules/database/components/view/grid/fields/GridViewFieldSingleSelect'
+import GridViewFieldPhoneNumber from '@baserow/modules/database/components/view/grid/fields/GridViewFieldPhoneNumber'
import FunctionalGridViewFieldText from '@baserow/modules/database/components/view/grid/fields/FunctionalGridViewFieldText'
import FunctionalGridViewFieldLongText from '@baserow/modules/database/components/view/grid/fields/FunctionalGridViewFieldLongText'
@@ -29,6 +34,7 @@ import FunctionalGridViewFieldBoolean from '@baserow/modules/database/components
import FunctionalGridViewFieldDate from '@baserow/modules/database/components/view/grid/fields/FunctionalGridViewFieldDate'
import FunctionalGridViewFieldFile from '@baserow/modules/database/components/view/grid/fields/FunctionalGridViewFieldFile'
import FunctionalGridViewFieldSingleSelect from '@baserow/modules/database/components/view/grid/fields/FunctionalGridViewFieldSingleSelect'
+import FunctionalGridViewFieldPhoneNumber from '@baserow/modules/database/components/view/grid/fields/FunctionalGridViewFieldPhoneNumber'
import RowEditFieldText from '@baserow/modules/database/components/row/RowEditFieldText'
import RowEditFieldLongText from '@baserow/modules/database/components/row/RowEditFieldLongText'
@@ -40,6 +46,7 @@ import RowEditFieldBoolean from '@baserow/modules/database/components/row/RowEdi
import RowEditFieldDate from '@baserow/modules/database/components/row/RowEditFieldDate'
import RowEditFieldFile from '@baserow/modules/database/components/row/RowEditFieldFile'
import RowEditFieldSingleSelect from '@baserow/modules/database/components/row/RowEditFieldSingleSelect'
+import RowEditFieldPhoneNumber from '@baserow/modules/database/components/row/RowEditFieldPhoneNumber'
import { trueString } from '@baserow/modules/database/utils/constants'
import {
@@ -1129,3 +1136,65 @@ export class SingleSelectFieldType extends FieldType {
}
}
}
+
+export class PhoneNumberFieldType extends FieldType {
+ static getType() {
+ return 'phone_number'
+ }
+
+ getIconClass() {
+ return 'phone'
+ }
+
+ getName() {
+ return 'Phone Number'
+ }
+
+ getGridViewFieldComponent() {
+ return GridViewFieldPhoneNumber
+ }
+
+ getFunctionalGridViewFieldComponent() {
+ return FunctionalGridViewFieldPhoneNumber
+ }
+
+ getRowEditFieldComponent() {
+ return RowEditFieldPhoneNumber
+ }
+
+ prepareValueForPaste(field, clipboardData) {
+ const value = clipboardData.getData('text')
+ return isSimplePhoneNumber(value) ? value : ''
+ }
+
+ getSort(name, order) {
+ return (a, b) => {
+ const stringA = a[name] === null ? '' : '' + a[name]
+ const stringB = b[name] === null ? '' : '' + b[name]
+
+ return order === 'ASC'
+ ? stringA.localeCompare(stringB)
+ : stringB.localeCompare(stringA)
+ }
+ }
+
+ getSortIndicator() {
+ return ['text', '0', '9']
+ }
+
+ getDocsDataType(field) {
+ return 'string'
+ }
+
+ getDocsDescription(field) {
+ return (
+ 'Accepts a phone number which has a maximum length of 100 characters' +
+ ' consisting solely of digits, spaces and the following characters: ' +
+ 'Nx,._+*()#=;/- .'
+ )
+ }
+
+ getDocsRequestExample(field) {
+ return '+1-541-754-3010'
+ }
+}
diff --git a/web-frontend/modules/database/mixins/phoneNumberField.js b/web-frontend/modules/database/mixins/phoneNumberField.js
new file mode 100644
index 000000000..bee7f6cac
--- /dev/null
+++ b/web-frontend/modules/database/mixins/phoneNumberField.js
@@ -0,0 +1,26 @@
+/**
+ * This mixin contains some method overrides for validating and formatting the
+ * phone number field. This mixin is used in both the GridViewFieldPhoneNumber and
+ * RowEditFieldPhoneNumber components.
+ */
+import { isSimplePhoneNumber } from '@baserow/modules/core/utils/string'
+
+export default {
+ methods: {
+ /**
+ * Generates a human readable error for the user if something is wrong.
+ */
+ getError() {
+ if (this.copy === null || this.copy === '') {
+ return null
+ }
+ if (!isSimplePhoneNumber(this.copy)) {
+ return 'Invalid Phone Number'
+ }
+ return null
+ },
+ isValid() {
+ return this.getError() === null
+ },
+ },
+}
diff --git a/web-frontend/modules/database/plugin.js b/web-frontend/modules/database/plugin.js
index f6153ec08..3f01af705 100644
--- a/web-frontend/modules/database/plugin.js
+++ b/web-frontend/modules/database/plugin.js
@@ -11,6 +11,7 @@ import {
DateFieldType,
FileFieldType,
SingleSelectFieldType,
+ PhoneNumberFieldType,
} from '@baserow/modules/database/fieldTypes'
import {
EqualViewFilterType,
@@ -73,6 +74,7 @@ export default ({ store, app }) => {
app.$registry.register('field', new EmailFieldType())
app.$registry.register('field', new FileFieldType())
app.$registry.register('field', new SingleSelectFieldType())
+ app.$registry.register('field', new PhoneNumberFieldType())
app.$registry.register('importer', new CSVImporterType())
app.$registry.register('importer', new PasteImporterType())
app.$registry.register('settings', new APITokenSettingsType())
diff --git a/web-frontend/modules/database/viewFilters.js b/web-frontend/modules/database/viewFilters.js
index 8eaf7dc11..90310a150 100644
--- a/web-frontend/modules/database/viewFilters.js
+++ b/web-frontend/modules/database/viewFilters.js
@@ -98,7 +98,7 @@ export class EqualViewFilterType extends ViewFilterType {
}
getCompatibleFieldTypes() {
- return ['text', 'long_text', 'url', 'email', 'number']
+ return ['text', 'long_text', 'url', 'email', 'number', 'phone_number']
}
matches(rowValue, filterValue) {
@@ -126,7 +126,7 @@ export class NotEqualViewFilterType extends ViewFilterType {
}
getCompatibleFieldTypes() {
- return ['text', 'long_text', 'url', 'email', 'number']
+ return ['text', 'long_text', 'url', 'email', 'number', 'phone_number']
}
matches(rowValue, filterValue) {
@@ -193,7 +193,7 @@ export class ContainsViewFilterType extends ViewFilterType {
}
getCompatibleFieldTypes() {
- return ['text', 'long_text', 'url', 'email']
+ return ['text', 'long_text', 'url', 'email', 'phone_number']
}
matches(rowValue, filterValue) {
@@ -217,7 +217,7 @@ export class ContainsNotViewFilterType extends ViewFilterType {
}
getCompatibleFieldTypes() {
- return ['text', 'long_text', 'url', 'email']
+ return ['text', 'long_text', 'url', 'email', 'phone_number']
}
matches(rowValue, filterValue) {
@@ -475,6 +475,7 @@ export class EmptyViewFilterType extends ViewFilterType {
'link_row',
'file',
'single_select',
+ 'phone_number',
]
}
@@ -517,6 +518,7 @@ export class NotEmptyViewFilterType extends ViewFilterType {
'link_row',
'file',
'single_select',
+ 'phone_number',
]
}