diff --git a/backend/src/baserow/contrib/database/api/fields/errors.py b/backend/src/baserow/contrib/database/api/fields/errors.py index f721b8d4b..c70b3ecda 100644 --- a/backend/src/baserow/contrib/database/api/fields/errors.py +++ b/backend/src/baserow/contrib/database/api/fields/errors.py @@ -30,3 +30,8 @@ ERROR_ORDER_BY_FIELD_NOT_POSSIBLE = ( 'It is not possible to order by {e.field_name} because the field type ' '{e.field_type} does not support filtering.' ) +ERROR_INCOMPATIBLE_PRIMARY_FIELD_TYPE = ( + 'ERROR_INCOMPATIBLE_PRIMARY_FIELD_TYPE', + HTTP_400_BAD_REQUEST, + 'The field type {e.field_type} is not compatible with the primary field.' +) diff --git a/backend/src/baserow/contrib/database/fields/exceptions.py b/backend/src/baserow/contrib/database/fields/exceptions.py index c50d676db..c6aae66b5 100644 --- a/backend/src/baserow/contrib/database/fields/exceptions.py +++ b/backend/src/baserow/contrib/database/fields/exceptions.py @@ -59,3 +59,11 @@ class OrderByFieldNotPossible(Exception): self.field_name = field_name self.field_type = field_type super().__init__(*args, **kwargs) + + +class IncompatiblePrimaryFieldTypeError(Exception): + """Raised when the primary field is changed to an incompatible field type.""" + + def __init__(self, field_type=None, *args, **kwargs): + self.field_type = field_type + super().__init__(*args, **kwargs) diff --git a/backend/src/baserow/contrib/database/fields/field_types.py b/backend/src/baserow/contrib/database/fields/field_types.py index b46b64d9e..a09d60b9c 100644 --- a/backend/src/baserow/contrib/database/fields/field_types.py +++ b/backend/src/baserow/contrib/database/fields/field_types.py @@ -19,7 +19,8 @@ from baserow.contrib.database.api.fields.serializers import ( LinkRowValueSerializer, FileFieldRequestSerializer, FileFieldResponseSerializer ) from baserow.contrib.database.api.fields.errors import ( - ERROR_LINK_ROW_TABLE_NOT_IN_SAME_DATABASE, ERROR_LINK_ROW_TABLE_NOT_PROVIDED + ERROR_LINK_ROW_TABLE_NOT_IN_SAME_DATABASE, ERROR_LINK_ROW_TABLE_NOT_PROVIDED, + ERROR_INCOMPATIBLE_PRIMARY_FIELD_TYPE ) from .handler import FieldHandler @@ -28,7 +29,10 @@ from .models import ( NUMBER_TYPE_INTEGER, NUMBER_TYPE_DECIMAL, TextField, LongTextField, URLField, NumberField, BooleanField, DateField, LinkRowField, EmailField, FileField ) -from .exceptions import LinkRowTableNotInSameDatabase, LinkRowTableNotProvided +from .exceptions import ( + LinkRowTableNotInSameDatabase, LinkRowTableNotProvided, + IncompatiblePrimaryFieldTypeError +) class TextFieldType(FieldType): @@ -292,9 +296,11 @@ class LinkRowFieldType(FieldType): } api_exceptions_map = { LinkRowTableNotProvided: ERROR_LINK_ROW_TABLE_NOT_PROVIDED, - LinkRowTableNotInSameDatabase: ERROR_LINK_ROW_TABLE_NOT_IN_SAME_DATABASE + LinkRowTableNotInSameDatabase: ERROR_LINK_ROW_TABLE_NOT_IN_SAME_DATABASE, + IncompatiblePrimaryFieldTypeError: ERROR_INCOMPATIBLE_PRIMARY_FIELD_TYPE } can_order_by = False + can_be_primary_field = False def enhance_queryset(self, queryset, field, name): """ diff --git a/backend/src/baserow/contrib/database/fields/handler.py b/backend/src/baserow/contrib/database/fields/handler.py index 4d0504c27..a0778946d 100644 --- a/backend/src/baserow/contrib/database/fields/handler.py +++ b/backend/src/baserow/contrib/database/fields/handler.py @@ -12,7 +12,7 @@ from baserow.contrib.database.views.handler import ViewHandler from .exceptions import ( PrimaryFieldAlreadyExists, CannotDeletePrimaryField, CannotChangeFieldType, - FieldDoesNotExist + FieldDoesNotExist, IncompatiblePrimaryFieldTypeError ) from .registries import field_type_registry, field_converter_registry from .models import Field @@ -168,6 +168,10 @@ class FieldHandler: # to remove all view filters. if new_type_name and field_type.type != new_type_name: field_type = field_type_registry.get(new_type_name) + + if field.primary and not field_type.can_be_primary_field: + raise IncompatiblePrimaryFieldTypeError(new_type_name) + new_model_class = field_type.model_class field.change_polymorphic_type_to(new_model_class) diff --git a/backend/src/baserow/contrib/database/fields/registries.py b/backend/src/baserow/contrib/database/fields/registries.py index 8ec0cd8e7..60add44ec 100644 --- a/backend/src/baserow/contrib/database/fields/registries.py +++ b/backend/src/baserow/contrib/database/fields/registries.py @@ -39,6 +39,9 @@ class FieldType(MapAPIExceptionsInstanceMixin, APIUrlsInstanceMixin, can_order_by = True """Indicates whether it is possible to order by this field type.""" + can_be_primary_field = True + """Some field types cannot be the primary field.""" + def prepare_value_for_db(self, instance, value): """ When a row is created or updated all the values are going to be prepared for the diff --git a/backend/src/baserow/contrib/database/migrations/0020_fix_primary_link_row.py b/backend/src/baserow/contrib/database/migrations/0020_fix_primary_link_row.py new file mode 100644 index 000000000..354791027 --- /dev/null +++ b/backend/src/baserow/contrib/database/migrations/0020_fix_primary_link_row.py @@ -0,0 +1,76 @@ +# Generated by Django 2.2.11 on 2020-11-16 08:53 + +from django.db import migrations, connections +from django.db.models import Exists, OuterRef, Q +from django.conf import settings + +from baserow.contrib.database.table.models import Table as TableModel +from baserow.contrib.database.fields.models import Field as FieldModel + + +def forward(apps, schema_editor): + """ + This migration fixes the not allowed situations where link row fields are primary + fields or if a table doesn't have a primary field anymore (because it was + deleted by the related link row field). In both cases a new primary text field is + created because that is allowed. + """ + + Table = apps.get_model('database', 'Table') + Field = apps.get_model('database', 'Field') + LinkRowField = apps.get_model('database', 'LinkRowField') + TextField = apps.get_model('database', 'TextField') + ContentType = apps.get_model('contenttypes', 'ContentType') + text_field_content_type = ContentType.objects.get_for_model(TextField) + + # Check if there are tables without a primary field or where the primary field is + # a link row field, which is not allowed. + tables_without_primary = Table.objects.annotate( + has_primary=Exists(Field.objects.filter(table=OuterRef('pk'), primary=True)), + has_link_row_primary=Exists( + LinkRowField.objects.filter(table=OuterRef('pk'), primary=True) + ) + ).filter(Q(has_primary=False) | Q(has_link_row_primary=True)) + for table in tables_without_primary: + # If the table has a link row field as primary field it needs to be marked as + # normal field because they are not allowed to be primary. + if table.has_link_row_primary: + link_row_primary = LinkRowField.objects.get(table=table, primary=True) + link_row_primary.primary = False + link_row_primary.save() + + # It might be possible in the future that the get_model or db_column methods + # are going to disappear. If that is the case then the creation of the field + # cannot be executed, so we can skip that. + if ( + not hasattr(TableModel, 'get_model') or + not hasattr(FieldModel, 'db_column') + ): + continue + + # We now know for sure there isn't a primary field in the table, so we can + # create a new primary text field because the table expects one. + new_primary = TextField.objects.create( + table=table, + name='Primary (auto created)', + order=0, + content_type=text_field_content_type, + primary=True + ) + connection = connections[settings.USER_TABLE_DATABASE] + with connection.schema_editor() as tables_schema_editor: + to_model = TableModel.get_model(table, field_ids=[new_primary.id]) + field_name = FieldModel.db_column.__get__(new_primary, FieldModel) + model_field = to_model._meta.get_field(field_name) + tables_schema_editor.add_field(to_model, model_field) + + +class Migration(migrations.Migration): + + dependencies = [ + ('database', '0019_filefield'), + ] + + operations = [ + migrations.RunPython(forward, migrations.RunPython.noop), + ] diff --git a/backend/tests/baserow/contrib/database/api/fields/test_field_views.py b/backend/tests/baserow/contrib/database/api/fields/test_field_views.py index 50e2d1a41..4256f5294 100644 --- a/backend/tests/baserow/contrib/database/api/fields/test_field_views.py +++ b/backend/tests/baserow/contrib/database/api/fields/test_field_views.py @@ -168,7 +168,7 @@ def test_update_field(api_client, data_fixture): user_2, token_2 = data_fixture.create_user_and_token() table = data_fixture.create_database_table(user=user) table_2 = data_fixture.create_database_table(user=user_2) - text = data_fixture.create_text_field(table=table) + text = data_fixture.create_text_field(table=table, primary=True) text_2 = data_fixture.create_text_field(table=table_2) url = reverse('api:database:fields:item', kwargs={'field_id': text_2.id}) @@ -192,6 +192,22 @@ def test_update_field(api_client, data_fixture): assert response.status_code == HTTP_404_NOT_FOUND assert response.json()['error'] == 'ERROR_FIELD_DOES_NOT_EXIST' + # The primary field is not compatible with a link row field so that should result + # in an error. + url = reverse('api:database:fields:item', kwargs={'field_id': text.id}) + response = api_client.patch( + url, + {'type': 'link_row'}, + format='json', + HTTP_AUTHORIZATION=f'JWT {token}' + ) + assert response.status_code == HTTP_400_BAD_REQUEST + assert response.json()['error'] == 'ERROR_INCOMPATIBLE_PRIMARY_FIELD_TYPE' + assert ( + response.json()['detail'] == + 'The field type link_row is not compatible with the primary field.' + ) + url = reverse('api:database:fields:item', kwargs={'field_id': text.id}) response = api_client.patch( url, 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 50930ca4e..0ffba3aae 100644 --- a/backend/tests/baserow/contrib/database/field/test_field_handler.py +++ b/backend/tests/baserow/contrib/database/field/test_field_handler.py @@ -8,7 +8,7 @@ from baserow.contrib.database.fields.models import ( ) from baserow.contrib.database.fields.exceptions import ( FieldTypeDoesNotExist, PrimaryFieldAlreadyExists, CannotDeletePrimaryField, - FieldDoesNotExist + FieldDoesNotExist, IncompatiblePrimaryFieldTypeError ) @@ -165,6 +165,15 @@ def test_update_field(data_fixture): with pytest.raises(FieldTypeDoesNotExist): handler.update_field(user=user, field=field, new_type_name='NOT_EXISTING') + # The link row field is not compatible with a primary field so an exception + # is expected. + field.primary = True + field.save() + with pytest.raises(IncompatiblePrimaryFieldTypeError): + handler.update_field(user=user, field=field, new_type_name='link_row') + field.primary = False + field.save() + # Change some values of the text field and test if they have been changed. field = handler.update_field(user=user, field=field, name='Text field', text_default='Default value') diff --git a/changelog.md b/changelog.md index 709eb10e1..fb8a0f0d8 100644 --- a/changelog.md +++ b/changelog.md @@ -11,6 +11,8 @@ * Set un-secure lax cookie when public web frontend url isn't over a secure connection. * Fixed bug where the sort choose field item didn't have a hover effect. * Implemented a file field and user files upload. +* Made it impossible for the `link_row` field to be a primary field because that can + cause the primary field to be deleted. ## Released (2020-11-02) diff --git a/web-frontend/modules/database/components/field/FieldForm.vue b/web-frontend/modules/database/components/field/FieldForm.vue index fe1a7b7bd..4aeaf3fa0 100644 --- a/web-frontend/modules/database/components/field/FieldForm.vue +++ b/web-frontend/modules/database/components/field/FieldForm.vue @@ -29,6 +29,7 @@ :icon="fieldType.iconClass" :name="fieldType.name" :value="fieldType.type" + :disabled="primary && !fieldType.canBePrimaryField" ></DropdownItem> </Dropdown> <div v-if="$v.values.type.$error" class="error"> @@ -62,6 +63,11 @@ export default { type: Object, required: true, }, + primary: { + type: Boolean, + required: false, + default: false, + }, }, data() { return { diff --git a/web-frontend/modules/database/components/field/UpdateFieldContext.vue b/web-frontend/modules/database/components/field/UpdateFieldContext.vue index 8fe5de658..896d672d3 100644 --- a/web-frontend/modules/database/components/field/UpdateFieldContext.vue +++ b/web-frontend/modules/database/components/field/UpdateFieldContext.vue @@ -4,6 +4,7 @@ ref="form" :table="table" :default-values="field" + :primary="field.primary" @submitted="submit" > <div class="context__form-actions"> diff --git a/web-frontend/modules/database/fieldTypes.js b/web-frontend/modules/database/fieldTypes.js index a9b86dd69..2c1aefa61 100644 --- a/web-frontend/modules/database/fieldTypes.js +++ b/web-frontend/modules/database/fieldTypes.js @@ -96,6 +96,13 @@ export class FieldType extends Registerable { return true } + /** + * Indicates if is possible for the field type to be the primary field. + */ + getCanBePrimaryField() { + return true + } + constructor() { super() this.type = this.getType() @@ -103,6 +110,7 @@ export class FieldType extends Registerable { this.name = this.getName() this.sortIndicator = this.getSortIndicator() this.canSortInView = this.getCanSortInView() + this.canBePrimaryField = this.getCanBePrimaryField() if (this.type === null) { throw new Error('The type name of a view type must be set.') @@ -364,6 +372,10 @@ export class LinkRowFieldType extends FieldType { return false } + getCanBePrimaryField() { + return false + } + prepareValueForCopy(field, value) { return JSON.stringify({ tableId: field.link_row_table,