Merge branch '2053-field-options-are-not-correctly-when-move_field_between_tables' into 'develop'

Resolve "Field options are not correctly deleted when a link row field is moved between tables"

Closes #2053

See merge request baserow/baserow!1823

backend/src/baserow/contrib/database/fields/handler.py

@@ -1078,6 +1078,7 @@ class FieldHandler(metaclass=baserow_trace_methods(tracer)):
         # its old model cache as this will not happen automatically.
         invalidate_table_in_model_cache(original_table_id)
         SearchHandler.after_field_moved_between_tables(field_to_move, original_table_id)
+        ViewHandler().after_field_moved_between_tables(field_to_move, original_table_id)
 
     def get_unique_row_values(
         self, field: Field, limit: int, split_comma_separated: bool = False

backend/src/baserow/contrib/database/migrations/0134_delete_stale_fieldoptions.py

@@ -0,0 +1,33 @@
+# Generated by Django 3.2.21 on 2023-11-02 15:31
+
+from django.db import migrations
+from django.db.models import F
+
+
+def forward(apps, schema_editor):
+    """
+    Delete all the ViewFieldOptions where the field has been moved to another table.
+    """
+
+    GridViewFieldOptions = apps.get_model("database", "GridViewFieldOptions")
+    FormViewFieldOptions = apps.get_model("database", "FormViewFieldOptions")
+    GalleryViewFieldOptions = apps.get_model("database", "GalleryViewFieldOptions")
+
+    for ViewFieldOptions, view_fk_field_name in (
+        (GridViewFieldOptions, "grid_view_id"),
+        (GalleryViewFieldOptions, "gallery_view_id"),
+        (FormViewFieldOptions, "form_view_id"),
+    ):
+        ViewFieldOptions.objects.exclude(
+            **{f"{view_fk_field_name}__table_id": F("field__table_id")}
+        ).delete()
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ("database", "0133_formviewfieldoptions_field_component"),
+    ]
+
+    operations = [
+        migrations.RunPython(forward, migrations.RunPython.noop),
+    ]

backend/src/baserow/contrib/database/views/handler.py

@@ -1190,6 +1190,18 @@ class ViewHandler(metaclass=baserow_trace_methods(tracer)):
 
         view_field_options_updated.send(self, view=view, user=user)
 
+    def after_field_moved_between_tables(self, field: Field, original_table_id: int):
+        """
+        This method is called to properly update the view field options when a field
+        is moved between tables.
+
+        :param field: The new field object.
+        :param original_table_id: The id of the table where the field was moved from.
+        """
+
+        for view_type in view_type_registry.get_all():
+            view_type.after_field_moved_between_tables(field, original_table_id)
+
     def field_type_changed(self, field: Field):
         """
         This method is called by the FieldHandler when the field type of a field has

backend/src/baserow/contrib/database/views/models.py

@@ -269,7 +269,9 @@ class View(
 
         def get_queryset():
             return view_type.enhance_field_options_queryset(
-                through_model.objects.filter(**{field_name: self})
+                through_model.objects.filter(
+                    **{field_name: self, "field__table_id": self.table_id}
+                )
             )
 
         field_options = get_queryset()

backend/src/baserow/contrib/database/views/registries.py

@@ -36,6 +36,7 @@ from baserow.core.registry import (
     ModelRegistryMixin,
     Registry,
 )
+from baserow.core.utils import get_model_reference_field_name
 
 from .exceptions import (
     AggregationTypeAlreadyRegistered,
@@ -750,6 +751,28 @@ class ViewType(
             "`get_hidden_fields`"
         )
 
+    def after_field_moved_between_tables(self, field: "Field", original_table_id: int):
+        """
+        This hook is called after a field has been moved between tables. It gives the
+        view type the opportunity to react on the move to, for example, remove the
+        field options have been created for the original table but are not needed
+        anymore.
+
+        :param field: The field that has been moved.
+        :param original_table_id: The id of the table where the field was moved from.
+        """
+
+        from baserow.contrib.database.views.models import View
+
+        field_options_model = self.field_options_model_class
+        field_name = get_model_reference_field_name(field_options_model, View)
+        field_options_model.objects.filter(
+            **{
+                f"{field_name}__table_id": original_table_id,
+                "field_id": field.id,
+            }
+        ).delete()
+
 
 class ViewTypeRegistry(
     APIUrlsRegistryMixin, CustomFieldsRegistryMixin, ModelRegistryMixin, Registry

backend/tests/baserow/contrib/database/table/test_table_handler.py

@@ -679,6 +679,38 @@ def test_duplicate_interesting_table(data_fixture):
             )
 
 
+@pytest.mark.django_db()
+def test_duplicate_table_after_link_row_field_moved_to_another_table(data_fixture):
+    user = data_fixture.create_user()
+    database = data_fixture.create_database_application(user=user)
+    _, table_b, link_field = data_fixture.create_two_linked_tables(
+        user=user, database=database
+    )
+    table_c = data_fixture.create_database_table(user=user, database=database)
+
+    grid_view = data_fixture.create_grid_view(table=table_b)
+
+    assert grid_view.get_field_options().count() == 2
+
+    FieldHandler().update_field(
+        user,
+        link_field,
+        name=link_field.name,
+        new_type_name="link_row",
+        link_row_table_id=table_c.id,
+        link_row_table=table_c,
+        has_related_field=True,
+    )
+
+    # the field option should be removed from the grid view
+    assert grid_view.get_field_options().count() == 1
+
+    try:
+        TableHandler().duplicate_table(user, table_b)
+    except Exception as exc:
+        pytest.fail("Duplicating table failed: %s" % exc)
+
+
 @pytest.mark.django_db()
 def test_create_needs_background_update_column(data_fixture):
     system_updated_on_columns = [

backend/tests/baserow/contrib/database/view/test_view_models.py

@@ -1,7 +1,10 @@
+from unittest.mock import patch
+
 from django.db import IntegrityError
 
 import pytest
 
+from baserow.contrib.database.fields.handler import FieldHandler
 from baserow.contrib.database.views.models import (
     FormViewFieldOptions,
     GalleryViewFieldOptions,
@@ -10,6 +13,7 @@ from baserow.contrib.database.views.models import (
     ViewFilter,
     ViewSort,
 )
+from baserow.contrib.database.views.view_types import GridViewType
 
 
 @pytest.mark.django_db
@@ -367,3 +371,47 @@ def test_migration_remove_duplicate_fieldoptions(
 
     with pytest.raises(IntegrityError):
         GalleryViewFieldOptions.objects.create(gallery_view=gallery_view, field=field)
+
+
+@pytest.mark.once_per_day_in_ci
+@patch.object(GridViewType, "after_field_moved_between_tables")
+def test_migration_remove_stale_fieldoptions(
+    mocked_func, data_fixture, migrator, teardown_table_metadata
+):
+    # The correct behavior for after_field_moved_between_tables has been implemented in
+    # the same MR when the migrations was added, so let's just mock it out to make sure
+    # we create the data the migration expects to delete.
+
+    migrate_from = [
+        ("database", "0133_formviewfieldoptions_field_component"),
+    ]
+    migrate_to = [("database", "0134_delete_stale_fieldoptions")]
+
+    migrator.migrate(migrate_from)
+
+    user = data_fixture.create_user()
+    database = data_fixture.create_database_application(user=user)
+    _, table_b, link_field = data_fixture.create_two_linked_tables(
+        user=user, database=database
+    )
+    table_c = data_fixture.create_database_table(user=user, database=database)
+
+    grid_view = data_fixture.create_grid_view(table=table_b)
+
+    FieldHandler().update_field(
+        user,
+        link_field,
+        name=link_field.name,
+        new_type_name="link_row",
+        link_row_table_id=table_c.id,
+        link_row_table=table_c,
+        has_related_field=True,
+    )
+
+    assert GridViewFieldOptions.objects.filter(grid_view=grid_view).count() == 2
+    assert mocked_func.call_count == 1
+
+    migrator.migrate(migrate_to)
+
+    # Only the stale field option should be deleted.
+    assert GridViewFieldOptions.objects.filter(grid_view=grid_view).count() == 1

changelog/entries/unreleased/bug/2053_fixed_a_bug_that_prevented_table_duplication_when_a_link_to_.json

@@ -0,0 +1,7 @@
+{
+    "type": "bug",
+    "message": "Fixed a bug that prevented table duplication when a 'Link to Table' field is modified to link to a different table.",
+    "issue_number": 2053,
+    "bullet_points": [],
+    "created_at": "2023-10-30"
+}

premium/backend/src/baserow_premium/migrations/0015_delete_stale_fieldoptions.py

@@ -0,0 +1,33 @@
+# Generated by Django 3.2.21 on 2023-11-02 15:31
+
+from django.db import migrations
+from django.db.models import F
+
+
+def forward(apps, schema_editor):
+    """
+    Delete all the ViewFieldOptions where the field has been moved to another table.
+    """
+
+    CalendarViewFieldOptions = apps.get_model(
+        "baserow_premium", "CalendarViewFieldOptions"
+    )
+    KanbanViewFieldOptions = apps.get_model("baserow_premium", "KanbanViewFieldOptions")
+
+    for ViewFieldOptions, view_fk_field_name in (
+        (KanbanViewFieldOptions, "kanban_view_id"),
+        (CalendarViewFieldOptions, "calendar_view_id"),
+    ):
+        ViewFieldOptions.objects.exclude(
+            **{f"{view_fk_field_name}__table_id": F("field__table_id")}
+        ).delete()
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ("baserow_premium", "0014_add_unique_constraint_viewfieldoptions"),
+    ]
+
+    operations = [
+        migrations.RunPython(forward, migrations.RunPython.noop),
+    ]