diff --git a/backend/src/baserow/contrib/database/fields/field_types.py b/backend/src/baserow/contrib/database/fields/field_types.py index c64e46123..9e27fc107 100644 --- a/backend/src/baserow/contrib/database/fields/field_types.py +++ b/backend/src/baserow/contrib/database/fields/field_types.py @@ -937,7 +937,7 @@ class LinkRowFieldType(FieldType): LinkRowTableNotInSameDatabase: ERROR_LINK_ROW_TABLE_NOT_IN_SAME_DATABASE, IncompatiblePrimaryFieldTypeError: ERROR_INCOMPATIBLE_PRIMARY_FIELD_TYPE, } - can_order_by = False + _can_order_by = False can_be_primary_field = False def enhance_queryset(self, queryset, field, name): @@ -2232,10 +2232,7 @@ class FormulaFieldType(FieldType): from baserow.contrib.database.fields.registries import field_type_registry field_type = field_type_registry.get_by_model(field.specific_class) - if ( - field_type.type == FormulaFieldType.type - or field_type.type == LookupFieldType.type - ): + if isinstance(field_type, FormulaFieldType): formula_type = field.specific.cached_formula_type return formula_type.type in compatible_formula_types else: @@ -2525,6 +2522,9 @@ class FormulaFieldType(FieldType): self._refresh_row_values(field, update_collector, via_path_to_starting_table) super().after_rows_imported(field, via_path_to_starting_table, update_collector) + def check_can_order_by(self, field): + return self.to_baserow_formula_type(field.specific).can_order_by + class LookupFieldType(FormulaFieldType): type = "lookup" diff --git a/backend/src/baserow/contrib/database/fields/registries.py b/backend/src/baserow/contrib/database/fields/registries.py index ec5d12879..bfb98d818 100644 --- a/backend/src/baserow/contrib/database/fields/registries.py +++ b/backend/src/baserow/contrib/database/fields/registries.py @@ -54,7 +54,7 @@ class FieldType( field_type_registry.register(ExampleFieldType()) """ - can_order_by = True + _can_order_by = True """Indicates whether it is possible to order by this field type.""" can_be_primary_field = True @@ -1028,6 +1028,20 @@ class FieldType( field, field, field, via_path_to_starting_table, update_collector ) + def check_can_order_by(self, field): + """ + Override this method if this field type can sometimes be ordered or sometimes + cannot be ordered depending on the individual field state. By default will just + return the bool property _can_order_by so if your field type doesn't depend + on the field state and is always just True or False just set _can_order_by + to the desired value. + + :param field: The field to check to see if it can be ordered by or not. + :return: True if a view can be ordered by this field, False otherwise. + """ + + return self._can_order_by + def before_field_options_update( self, field: Field, diff --git a/backend/src/baserow/contrib/database/formula/types/formula_type.py b/backend/src/baserow/contrib/database/formula/types/formula_type.py index 2a932277c..feb459859 100644 --- a/backend/src/baserow/contrib/database/formula/types/formula_type.py +++ b/backend/src/baserow/contrib/database/formula/types/formula_type.py @@ -100,6 +100,15 @@ class BaserowFormulaType(abc.ABC): """ pass + @property + @abc.abstractmethod + def can_order_by(self) -> bool: + """ + Return True if a formula field of this type can be ordered or False if not. + """ + + pass + @property @abc.abstractmethod def is_valid(self) -> bool: @@ -257,6 +266,7 @@ class BaserowFormulaType(abc.ABC): class BaserowFormulaInvalidType(BaserowFormulaType): is_valid = False + can_order_by = False comparable_types = [] limit_comparable_types = [] type = "invalid" @@ -277,6 +287,7 @@ class BaserowFormulaInvalidType(BaserowFormulaType): class BaserowFormulaValidType(BaserowFormulaType, abc.ABC): is_valid = True + can_order_by = True @property @abc.abstractmethod diff --git a/backend/src/baserow/contrib/database/formula/types/formula_types.py b/backend/src/baserow/contrib/database/formula/types/formula_types.py index 0a67c07ce..b87eb7099 100644 --- a/backend/src/baserow/contrib/database/formula/types/formula_types.py +++ b/backend/src/baserow/contrib/database/formula/types/formula_types.py @@ -363,6 +363,7 @@ class BaserowFormulaArrayType(BaserowFormulaValidType): user_overridable_formatting_option_fields = [ "array_formula_type", ] + can_order_by = False def __init__(self, sub_type: BaserowFormulaValidType): self.array_formula_type = sub_type.type @@ -500,6 +501,7 @@ class BaserowFormulaArrayType(BaserowFormulaValidType): class BaserowFormulaSingleSelectType(BaserowFormulaValidType): type = "single_select" baserow_field_type = "single_select" + can_order_by = False @property def comparable_types(self) -> List[Type["BaserowFormulaValidType"]]: diff --git a/backend/src/baserow/contrib/database/migrations/0057_fix_invalid_type_filters_and_sorts.py b/backend/src/baserow/contrib/database/migrations/0057_fix_invalid_type_filters_and_sorts.py new file mode 100644 index 000000000..71a6c12f1 --- /dev/null +++ b/backend/src/baserow/contrib/database/migrations/0057_fix_invalid_type_filters_and_sorts.py @@ -0,0 +1,34 @@ +# Generated by Django 3.2.6 on 2021-11-01 09:38 +from django.db import migrations + + +# noinspection PyPep8Naming + + +def reverse(apps, schema_editor): + pass + + +# noinspection PyPep8Naming +def forward(apps, schema_editor): + FormulaField = apps.get_model("database", "FormulaField") + invalid_pks = FormulaField.objects.filter( + formula_type__in=["invalid", "array", "single_select"] + ) + ViewFilter = apps.get_model("database", "ViewFilter") + ViewSort = apps.get_model("database", "ViewSort") + invalid_filters, _ = ViewFilter.objects.filter(field__in=invalid_pks).delete() + invalid_sorts, _ = ViewSort.objects.filter(field__in=invalid_pks).delete() + print(f"Fixed {invalid_filters} invalid filters") + print(f"Fixed {invalid_sorts} invalid sorts") + + +class Migration(migrations.Migration): + + dependencies = [ + ("database", "0056_galleryview_card_cover_image_field"), + ] + + operations = [ + migrations.RunPython(forward, reverse), + ] diff --git a/backend/src/baserow/contrib/database/table/models.py b/backend/src/baserow/contrib/database/table/models.py index 8c690aa22..62b5f8bdb 100644 --- a/backend/src/baserow/contrib/database/table/models.py +++ b/backend/src/baserow/contrib/database/table/models.py @@ -174,7 +174,7 @@ class TableModelQuerySet(models.QuerySet): user_field_name = field_object["field"].name error_display_name = user_field_name if user_field_names else field_name - if not field_object["type"].can_order_by: + if not field_object["type"].check_can_order_by(field_object["field"]): raise OrderByFieldNotPossible( error_display_name, field_type.type, diff --git a/backend/src/baserow/contrib/database/views/handler.py b/backend/src/baserow/contrib/database/views/handler.py index 7752a9d53..5000cbc06 100644 --- a/backend/src/baserow/contrib/database/views/handler.py +++ b/backend/src/baserow/contrib/database/views/handler.py @@ -280,7 +280,7 @@ class ViewHandler: # If the new field type does not support sorting then all sortings will be # removed. - if not field_type.can_order_by: + if not field_type.check_can_order_by(field): field.viewsort_set.all().delete() # Check which filters are not compatible anymore and remove those. @@ -660,7 +660,7 @@ class ViewHandler: # Check if the field supports sorting. field_type = field_type_registry.get_by_model(field.specific_class) - if not field_type.can_order_by: + if not field_type.check_can_order_by(field): raise ViewSortFieldNotSupported( f"The field {field.pk} does not support " f"sorting." ) @@ -718,7 +718,7 @@ class ViewHandler: # If the field has changed we need to check if the new field type supports # sorting. field_type = field_type_registry.get_by_model(field.specific_class) - if field.id != view_sort.field_id and not field_type.can_order_by: + if field.id != view_sort.field_id and not field_type.check_can_order_by(field): raise ViewSortFieldNotSupported( f"The field {field.pk} does not support " f"sorting." ) diff --git a/backend/src/baserow/contrib/database/views/view_filters.py b/backend/src/baserow/contrib/database/views/view_filters.py index 51c24c572..4f095e014 100644 --- a/backend/src/baserow/contrib/database/views/view_filters.py +++ b/backend/src/baserow/contrib/database/views/view_filters.py @@ -743,7 +743,13 @@ class EmptyViewFilterType(ViewFilterType): SingleSelectFieldType.type, PhoneNumberFieldType.type, MultipleSelectFieldType.type, - FormulaFieldType.type, + FormulaFieldType.compatible_with_formula_types( + BaserowFormulaTextType.type, + BaserowFormulaCharType.type, + BaserowFormulaNumberType.type, + BaserowFormulaDateType.type, + BaserowFormulaBooleanType.type, + ), ] def get_filter(self, field_name, value, model_field, field): diff --git a/backend/tests/baserow/contrib/database/api/rows/test_row_views.py b/backend/tests/baserow/contrib/database/api/rows/test_row_views.py index ca5ff2406..55212c15c 100644 --- a/backend/tests/baserow/contrib/database/api/rows/test_row_views.py +++ b/backend/tests/baserow/contrib/database/api/rows/test_row_views.py @@ -219,8 +219,8 @@ def test_list_rows(api_client, data_fixture): ) number_field_type = field_type_registry.get("number") - old_can_order_by = number_field_type.can_order_by - number_field_type.can_order_by = False + old_can_order_by = number_field_type._can_order_by + number_field_type._can_order_by = False url = reverse("api:database:rows:list", kwargs={"table_id": table.id}) response = api_client.get( f"{url}?order_by=-field_{field_2.id}", @@ -234,7 +234,7 @@ def test_list_rows(api_client, data_fixture): f"It is not possible to order by field_{field_2.id} because the field type " f"number does not support filtering." ) - number_field_type.can_order_by = old_can_order_by + number_field_type._can_order_by = old_can_order_by url = reverse("api:database:rows:list", kwargs={"table_id": table.id}) response = api_client.get( diff --git a/backend/tests/baserow/contrib/database/field/test_formula_field_type.py b/backend/tests/baserow/contrib/database/field/test_formula_field_type.py index 928424f45..e955435b0 100644 --- a/backend/tests/baserow/contrib/database/field/test_formula_field_type.py +++ b/backend/tests/baserow/contrib/database/field/test_formula_field_type.py @@ -11,7 +11,7 @@ from baserow.contrib.database.fields.dependencies.update_collector import ( from baserow.contrib.database.fields.field_cache import FieldCache from baserow.contrib.database.fields.field_types import FormulaFieldType from baserow.contrib.database.fields.handler import FieldHandler -from baserow.contrib.database.fields.models import FormulaField +from baserow.contrib.database.fields.models import FormulaField, LookupField from baserow.contrib.database.fields.registries import field_type_registry from baserow.contrib.database.formula import ( BaserowFormulaInvalidType, @@ -22,7 +22,13 @@ from baserow.contrib.database.formula import ( from baserow.contrib.database.formula.ast.tree import BaserowFunctionDefinition from baserow.contrib.database.formula.registries import formula_function_registry from baserow.contrib.database.rows.handler import RowHandler +from baserow.contrib.database.views.exceptions import ( + ViewFilterTypeNotAllowedForField, + ViewSortFieldNotSupported, +) from baserow.contrib.database.views.handler import ViewHandler +from baserow.contrib.database.views.models import SORT_ORDER_ASC, SORT_ORDER_DESC +from baserow.contrib.database.views.registries import view_filter_type_registry @pytest.mark.django_db @@ -406,7 +412,6 @@ def test_recalculate_formulas_according_to_version( def test_can_update_lookup_field_value( data_fixture, api_client, django_assert_num_queries ): - user, token = data_fixture.create_user_and_token() table = data_fixture.create_database_table(user=user) table2 = data_fixture.create_database_table(user=user, database=table.database) @@ -627,7 +632,6 @@ def test_nested_lookup_with_formula( def test_can_delete_lookup_field_value( data_fixture, api_client, django_assert_num_queries ): - user, token = data_fixture.create_user_and_token() table = data_fixture.create_database_table(user=user) table2 = data_fixture.create_database_table(user=user, database=table.database) @@ -733,7 +737,6 @@ def test_can_delete_lookup_field_value( def test_can_delete_double_link_lookup_field_value( data_fixture, api_client, django_assert_num_queries ): - user, token = data_fixture.create_user_and_token() table = data_fixture.create_database_table(user=user) table2 = data_fixture.create_database_table(user=user, database=table.database) @@ -1103,3 +1106,70 @@ def test_can_insert_and_update_rows_with_formula_referencing_single_select( row.refresh_from_db() result = getattr(row, f"field_{formula_field.id}") assert result is None + + +@pytest.mark.django_db +def test_cannot_create_view_filter_or_sort_on_invalid_field(data_fixture): + user = data_fixture.create_user() + table, other_table, link = data_fixture.create_two_linked_tables(user=user) + grid_view = data_fixture.create_grid_view(user, table=table) + first_formula_field = FieldHandler().create_field( + user, table, "formula", formula="1", name="source" + ) + broken_formula_field = FieldHandler().create_field( + user, table, "formula", formula="field('source')", name="a" + ) + FieldHandler().delete_field(user, first_formula_field) + + option_field = data_fixture.create_single_select_field( + table=table, name="option_field", order=1 + ) + data_fixture.create_select_option(field=option_field, value="A", color="blue") + data_fixture.create_select_option(field=option_field, value="B", color="red") + single_select_formula_field = FieldHandler().create_field( + user=user, + table=table, + type_name="formula", + name="2", + formula="field('option_field')", + ) + lookup_field = FieldHandler().create_field( + user=user, + table=table, + type_name="lookup", + name="lookup", + through_field_name=link.name, + target_field_name="primary", + ) + + broken_formula_field = FormulaField.objects.get(id=broken_formula_field.id) + single_select_formula_field = FormulaField.objects.get( + id=single_select_formula_field.id + ) + lookup_field = LookupField.objects.get(id=lookup_field.id) + assert broken_formula_field.formula_type == "invalid" + assert single_select_formula_field.formula_type == "single_select" + assert lookup_field.formula_type == "array" + + fields_which_cant_yet_be_sorted_or_filtered = [ + broken_formula_field, + single_select_formula_field, + lookup_field, + ] + for field in fields_which_cant_yet_be_sorted_or_filtered: + for view_filter_type in view_filter_type_registry.get_all(): + with pytest.raises(ViewFilterTypeNotAllowedForField): + ViewHandler().create_filter( + user, + grid_view, + field, + view_filter_type.type, + "", + ) + + for field in fields_which_cant_yet_be_sorted_or_filtered: + with pytest.raises(ViewSortFieldNotSupported): + ViewHandler().create_sort(user, grid_view, field, SORT_ORDER_ASC) + + with pytest.raises(ViewSortFieldNotSupported): + ViewHandler().create_sort(user, grid_view, field, SORT_ORDER_DESC) diff --git a/changelog.md b/changelog.md index 9fea7b794..d84872dec 100644 --- a/changelog.md +++ b/changelog.md @@ -23,6 +23,7 @@ * Improved performance by not rendering cells that are out of the view port. * Fix bug where field options in rare situations could have been duplicated. * Fixed order of fields in form preview. +* Fix the ability to make filters and sorts on invalid formula and lookup fields. ## Released (2021-11-25) diff --git a/web-frontend/modules/database/components/formula/array/FunctionalFormulaArrayItems.vue b/web-frontend/modules/database/components/formula/array/FunctionalFormulaArrayItems.vue index 096ad28b5..60f910a81 100644 --- a/web-frontend/modules/database/components/formula/array/FunctionalFormulaArrayItems.vue +++ b/web-frontend/modules/database/components/formula/array/FunctionalFormulaArrayItems.vue @@ -5,7 +5,7 @@ v-for="(item, index) in props.value || []" :key="index" :field="props.field" - :value="item.value" + :value="item && item.value" ></component> </div> </template> diff --git a/web-frontend/modules/database/components/view/ViewSortContext.vue b/web-frontend/modules/database/components/view/ViewSortContext.vue index 091f022a0..786bb3e9f 100644 --- a/web-frontend/modules/database/components/view/ViewSortContext.vue +++ b/web-frontend/modules/database/components/view/ViewSortContext.vue @@ -196,14 +196,17 @@ export default { * Calculates the total amount of available fields. */ availableFieldsLength() { - const fields = this.fields.filter( - (field) => field._.type.canSortInView + const fields = this.fields.filter((field) => + this.getCanSortInView(field) ).length - const primary = this.primary._.type.canSortInView ? 1 : 0 + const primary = this.getCanSortInView(this.primary) ? 1 : 0 return fields + primary }, }, methods: { + getCanSortInView(field) { + return this.$registry.get('field', field.type).getCanSortInView(field) + }, getField(fieldId) { if (this.primary.id === fieldId) { return this.primary @@ -217,7 +220,7 @@ export default { }, isFieldAvailable(field) { const allFieldIds = this.view.sortings.map((sort) => sort.field) - return field._.type.canSortInView && !allFieldIds.includes(field.id) + return this.getCanSortInView(field) && !allFieldIds.includes(field.id) }, async addSort(field) { this.$refs.addContext.hide() diff --git a/web-frontend/modules/database/components/view/grid/GridViewFieldType.vue b/web-frontend/modules/database/components/view/grid/GridViewFieldType.vue index 504c331f6..eae53783a 100644 --- a/web-frontend/modules/database/components/view/grid/GridViewFieldType.vue +++ b/web-frontend/modules/database/components/view/grid/GridViewFieldType.vue @@ -47,7 +47,7 @@ {{ $t('gridViewFieldType.createFilter') }} </a> </li> - <li v-if="field._.type.canSortInView"> + <li v-if="getCanSortInView(field)"> <a @click="createSort($event, view, field, 'ASC')"> <i class="context__menu-icon fas fa-fw fa-sort-amount-down-alt"></i> {{ $t('gridViewFieldType.sortField') }} @@ -70,7 +70,7 @@ ></i> </a> </li> - <li v-if="field._.type.canSortInView"> + <li v-if="getCanSortInView(field)"> <a @click="createSort($event, view, field, 'DESC')"> <i class="context__menu-icon fas fa-fw fa-sort-amount-down"></i> {{ $t('gridViewFieldType.sortField') }} @@ -256,6 +256,9 @@ export default { .get('field', field.type) .getSortIndicator(field, this.$registry)[index] }, + getCanSortInView(field) { + return this.$registry.get('field', field.type).getCanSortInView(field) + }, }, } </script> diff --git a/web-frontend/modules/database/components/view/grid/fields/FunctionalGridViewFieldSingleSelect.vue b/web-frontend/modules/database/components/view/grid/fields/FunctionalGridViewFieldSingleSelect.vue index 0ccb6bac3..7050a5bc6 100644 --- a/web-frontend/modules/database/components/view/grid/fields/FunctionalGridViewFieldSingleSelect.vue +++ b/web-frontend/modules/database/components/view/grid/fields/FunctionalGridViewFieldSingleSelect.vue @@ -1,5 +1,5 @@ <template functional> - <div ref="cell" class="grid-view__cell"> + <div ref="cell" class="grid-view__cell" :class="data.staticClass || ''"> <div class="grid-field-single-select"> <div v-if="props.value" diff --git a/web-frontend/modules/database/fieldTypes.js b/web-frontend/modules/database/fieldTypes.js index cc8d3001f..25daf587c 100644 --- a/web-frontend/modules/database/fieldTypes.js +++ b/web-frontend/modules/database/fieldTypes.js @@ -215,7 +215,7 @@ export class FieldType extends Registerable { /** * Indicates whether or not it is possible to sort in a view. */ - getCanSortInView() { + getCanSortInView(field) { return true } @@ -230,7 +230,6 @@ export class FieldType extends Registerable { super(...args) this.type = this.getType() this.iconClass = this.getIconClass() - this.canSortInView = this.getCanSortInView() this.canBePrimaryField = this.getCanBePrimaryField() this.isReadOnly = this.getIsReadOnly() @@ -263,7 +262,6 @@ export class FieldType extends Registerable { type: this.type, iconClass: this.iconClass, name: this.getName(), - canSortInView: this.canSortInView, isReadOnly: this.isReadOnly, } } @@ -672,7 +670,7 @@ export class LinkRowFieldType extends FieldType { return [] } - getCanSortInView() { + getCanSortInView(field) { return false } @@ -1586,7 +1584,7 @@ export class FileFieldType extends FieldType { return [] } - getCanSortInView() { + getCanSortInView(field) { return false } @@ -2007,7 +2005,7 @@ export class FormulaFieldType extends FieldType { static compatibleWithFormulaTypes(...formulaTypeStrings) { return (field) => { return ( - (field.type === this.getType() || field.type === 'lookup') && + field.type === this.getType() && formulaTypeStrings.includes(field.formula_type) ) } @@ -2134,6 +2132,11 @@ export class FormulaFieldType extends FieldType { canBeReferencedByFormulaField() { return true } + + getCanSortInView(field) { + const subType = this.app.$registry.get('formula_type', field.formula_type) + return subType.getCanSortInView() + } } export class LookupFieldType extends FormulaFieldType { diff --git a/web-frontend/modules/database/formula/formulaTypes.js b/web-frontend/modules/database/formula/formulaTypes.js index 86c362d67..869bc8b79 100644 --- a/web-frontend/modules/database/formula/formulaTypes.js +++ b/web-frontend/modules/database/formula/formulaTypes.js @@ -86,6 +86,10 @@ export class BaserowFormulaTypeDefinition extends Registerable { getFunctionalGridViewFieldArrayComponent() { return FunctionalFormulaArrayItem } + + getCanSortInView() { + return true + } } export class BaserowFormulaTextType extends BaserowFormulaTypeDefinition { @@ -308,6 +312,10 @@ export class BaserowFormulaInvalidType extends BaserowFormulaTypeDefinition { getSortOrder() { return 9 } + + getCanSortInView() { + return false + } } export class BaserowFormulaArrayType extends BaserowFormulaTypeDefinition { @@ -369,6 +377,10 @@ export class BaserowFormulaArrayType extends BaserowFormulaTypeDefinition { getDocsDataType(field) { return 'array' } + + getCanSortInView() { + return false + } } export class BaserowFormulaSingleSelectType extends BaserowFormulaTypeDefinition { @@ -399,4 +411,8 @@ export class BaserowFormulaSingleSelectType extends BaserowFormulaTypeDefinition getSortOrder() { return 8 } + + getCanSortInView() { + return false + } } diff --git a/web-frontend/modules/database/store/view.js b/web-frontend/modules/database/store/view.js index 2bdf9d8dc..7f9a1d50a 100644 --- a/web-frontend/modules/database/store/view.js +++ b/web-frontend/modules/database/store/view.js @@ -630,7 +630,7 @@ export const actions = { // Remove all the field sortings because the new field does not support sortings // at all. - if (!fieldType.canSortInView) { + if (!fieldType.getCanSortInView(field)) { dispatch('deleteFieldSortings', { field }) } }, diff --git a/web-frontend/modules/database/viewFilters.js b/web-frontend/modules/database/viewFilters.js index ef6d0b4b7..13aa3c63c 100644 --- a/web-frontend/modules/database/viewFilters.js +++ b/web-frontend/modules/database/viewFilters.js @@ -1042,7 +1042,13 @@ export class EmptyViewFilterType extends ViewFilterType { 'single_select', 'multiple_select', 'phone_number', - 'formula', + FormulaFieldType.compatibleWithFormulaTypes( + 'text', + 'char', + 'boolean', + 'date', + 'number' + ), ] } @@ -1090,7 +1096,13 @@ export class NotEmptyViewFilterType extends ViewFilterType { 'single_select', 'multiple_select', 'phone_number', - 'formula', + FormulaFieldType.compatibleWithFormulaTypes( + 'text', + 'char', + 'boolean', + 'date', + 'number' + ), ] }