Resolve "Single Select Fields duplicated in other apps and imported to Baserow are buggy"

backend/src/baserow/contrib/database/airtable/airtable_column_types.py

@@ -15,6 +15,7 @@ from baserow.contrib.database.fields.models import (
     CreatedOnField,
     DateField,
     EmailField,
+    Field,
     FileField,
     LastModifiedField,
     LinkRowField,
@@ -338,10 +339,23 @@ class MultipleAttachmentAirtableColumnType(AirtableColumnType):
 class SelectAirtableColumnType(AirtableColumnType):
     type = "select"
 
+    def to_baserow_export_serialized_value(
+        self,
+        row_id_mapping: Dict[str, Dict[str, int]],
+        raw_airtable_column: dict,
+        baserow_field: Field,
+        value: Any,
+        files_to_download: Dict[str, str],
+    ):
+        # use field id and option id for uniqueness
+        return f"{raw_airtable_column.get('id')}_{value}"
+
     def to_baserow_field(self, raw_airtable_table, raw_airtable_column):
         field = SingleSelectField()
         field = set_select_options_on_field(
-            field, raw_airtable_column.get("typeOptions", {})
+            field,
+            raw_airtable_column.get("id", ""),
+            raw_airtable_column.get("typeOptions", {}),
         )
         return field
 
@@ -349,10 +363,24 @@ class SelectAirtableColumnType(AirtableColumnType):
 class MultiSelectAirtableColumnType(AirtableColumnType):
     type = "multiSelect"
 
+    def to_baserow_export_serialized_value(
+        self,
+        row_id_mapping: Dict[str, Dict[str, int]],
+        raw_airtable_column: dict,
+        baserow_field: Field,
+        value: Any,
+        files_to_download: Dict[str, str],
+    ):
+        # use field id and option id for uniqueness
+        column_id = raw_airtable_column.get("id")
+        return [f"{column_id}_{val}" for val in value]
+
     def to_baserow_field(self, raw_airtable_table, raw_airtable_column):
         field = MultipleSelectField()
         field = set_select_options_on_field(
-            field, raw_airtable_column.get("typeOptions", {})
+            field,
+            raw_airtable_column.get("id", ""),
+            raw_airtable_column.get("typeOptions", {}),
         )
         return field
 

backend/src/baserow/contrib/database/airtable/helpers.py

@@ -17,12 +17,14 @@ def import_airtable_date_type_options(type_options) -> dict:
     }
 
 
-def import_airtable_choices(type_options: dict) -> List[SelectOption]:
+def import_airtable_choices(field_id: str, type_options: dict) -> List[SelectOption]:
     order = type_options.get("choiceOrder", [])
     choices = type_options.get("choices", [])
     return [
         SelectOption(
-            id=choice["id"],
+            # Combine select id with choice id as choice id is not guaranteed to be
+            # unique across table
+            id=f"{field_id}_{choice['id']}",
             value=choice["name"],
             color=AIRTABLE_BASEROW_COLOR_MAPPING.get(
                 # The color isn't always provided, hence the fallback to an empty
@@ -36,7 +38,9 @@ def import_airtable_choices(type_options: dict) -> List[SelectOption]:
     ]
 
 
-def set_select_options_on_field(field: Field, type_options: dict) -> Field:
+def set_select_options_on_field(
+    field: Field, field_id: str, type_options: dict
+) -> Field:
     """
     Set the `select_options` of a field in the prefetched objects cache. Overriding
     this object makes sure that when later `field.select_options.all()` is executed,
@@ -44,6 +48,7 @@ def set_select_options_on_field(field: Field, type_options: dict) -> Field:
     `FieldType::export_serialized`.
 
     :param field: The field where the select must be set on.
+    :param field_id: id of airtable choice field
     :param type_options: The options where to extract the Airtable choices from.
     :return: The updated field object.
     """
@@ -52,6 +57,6 @@ def set_select_options_on_field(field: Field, type_options: dict) -> Field:
     # `field.select_options.all()` is executed, it will return Airtable choices.
     # This for example happens in `FieldType::export_serialized`.
     field._prefetched_objects_cache = {
-        "select_options": import_airtable_choices(type_options)
+        "select_options": import_airtable_choices(field_id, type_options)
     }
     return field

backend/tests/baserow/contrib/database/airtable/test_airtable_column_types.py

@@ -801,11 +801,11 @@ def test_airtable_import_multi_select_column(
         select_options = list(baserow_field.select_options.all())
 
     assert len(select_options) == 2
-    assert select_options[0].id == "selEOJmenvqEd6pndFQ"
+    assert select_options[0].id == "fldURNo0cvi6YWYcYj1_selEOJmenvqEd6pndFQ"
     assert select_options[0].value == "Option 1"
     assert select_options[0].color == "blue"
     assert select_options[0].order == 1
-    assert select_options[1].id == "sel5ekvuoNVvl03olMO"
+    assert select_options[1].id == "fldURNo0cvi6YWYcYj1_sel5ekvuoNVvl03olMO"
     assert select_options[1].value == "Option 2"
     assert select_options[1].color == "light-blue"
     assert select_options[1].order == 0
@@ -1062,11 +1062,11 @@ def test_airtable_import_select_column(
         select_options = list(baserow_field.select_options.all())
 
     assert len(select_options) == 2
-    assert select_options[0].id == "selbh6rEWaaiyQvWyfg"
+    assert select_options[0].id == "fldRd2Vkzgsf6X4z6B4_selbh6rEWaaiyQvWyfg"
     assert select_options[0].value == "Option A"
     assert select_options[0].color == "blue"
     assert select_options[0].order == 0
-    assert select_options[1].id == "selvZgpWhbkeRVphROT"
+    assert select_options[1].id == "fldRd2Vkzgsf6X4z6B4_selvZgpWhbkeRVphROT"
     assert select_options[1].value == "Option B"
     assert select_options[1].color == "light-blue"
     assert select_options[1].order == 1

backend/tests/baserow/contrib/database/airtable/test_airtable_handler.py

@@ -27,7 +27,9 @@ from baserow.core.utils import Progress
 @pytest.mark.django_db
 @responses.activate
 def test_fetch_publicly_shared_base():
-    base_path = os.path.join(settings.BASE_DIR, "../../../tests/airtable_responses")
+    base_path = os.path.join(
+        settings.BASE_DIR, "../../../tests/airtable_responses/basic"
+    )
     path = os.path.join(base_path, "airtable_base.html")
 
     with open(path, "rb") as file:
@@ -66,7 +68,9 @@ def test_fetch_publicly_shared_base_not_base_request_id_missing():
 @pytest.mark.django_db
 @responses.activate
 def test_fetch_table():
-    base_path = os.path.join(settings.BASE_DIR, "../../../tests/airtable_responses")
+    base_path = os.path.join(
+        settings.BASE_DIR, "../../../tests/airtable_responses/basic"
+    )
     path = os.path.join(base_path, "airtable_base.html")
     application_response_path = os.path.join(base_path, "airtable_application.json")
     table_response_path = os.path.join(base_path, "airtable_table.json")
@@ -133,7 +137,9 @@ def test_fetch_table():
 @pytest.mark.django_db
 @responses.activate
 def test_extract_schema():
-    base_path = os.path.join(settings.BASE_DIR, "../../../tests/airtable_responses")
+    base_path = os.path.join(
+        settings.BASE_DIR, "../../../tests/airtable_responses/basic"
+    )
     user_table_path = os.path.join(base_path, "airtable_application.json")
     data_table_path = os.path.join(base_path, "airtable_table.json")
     user_table_json = json.loads(Path(user_table_path).read_text())
@@ -152,7 +158,9 @@ def test_extract_schema():
 @pytest.mark.django_db
 @responses.activate
 def test_to_baserow_database_export():
-    base_path = os.path.join(settings.BASE_DIR, "../../../tests/airtable_responses")
+    base_path = os.path.join(
+        settings.BASE_DIR, "../../../tests/airtable_responses/basic"
+    )
     path = os.path.join(base_path, "airtable_base.html")
     user_table_path = os.path.join(base_path, "airtable_application.json")
     data_table_path = os.path.join(base_path, "airtable_table.json")
@@ -301,7 +309,9 @@ def test_to_baserow_database_export():
 @pytest.mark.django_db
 @responses.activate
 def test_to_baserow_database_export_without_primary_value():
-    base_path = os.path.join(settings.BASE_DIR, "../../../tests/airtable_responses")
+    base_path = os.path.join(
+        settings.BASE_DIR, "../../../tests/airtable_responses/basic"
+    )
     path = os.path.join(base_path, "airtable_base.html")
     user_table_path = os.path.join(base_path, "airtable_application.json")
     user_table_json = json.loads(Path(user_table_path).read_text())
@@ -356,7 +366,9 @@ def test_import_from_airtable_to_workspace(
     data_fixture, tmpdir, django_assert_num_queries
 ):
     workspace = data_fixture.create_workspace()
-    base_path = os.path.join(settings.BASE_DIR, "../../../tests/airtable_responses")
+    base_path = os.path.join(
+        settings.BASE_DIR, "../../../tests/airtable_responses/basic"
+    )
     storage = FileSystemStorage(location=(str(tmpdir)), base_url="http://localhost")
 
     with open(os.path.join(base_path, "file-sample.txt"), "rb") as file:
@@ -451,11 +463,124 @@ def test_import_from_airtable_to_workspace(
     assert row_1.checkbox is False
 
 
+@pytest.mark.django_db
+@responses.activate
+def test_import_from_airtable_to_workspace_duplicated_single_select(
+    data_fixture, tmpdir, django_assert_num_queries
+):
+    workspace = data_fixture.create_workspace()
+    base_path = os.path.join(
+        settings.BASE_DIR, "../../../tests/airtable_responses/single_select_duplicated"
+    )
+    storage = FileSystemStorage(location=(str(tmpdir)), base_url="http://localhost")
+
+    with open(os.path.join(base_path, "airtable_base.html"), "rb") as file:
+        responses.add(
+            responses.GET,
+            "https://airtable.com/shra2B9gmVj6kxvNz",
+            status=200,
+            body=file.read(),
+            headers={"Set-Cookie": "brw=test;"},
+        )
+
+    with open(os.path.join(base_path, "airtable_application.json"), "rb") as file:
+        responses.add(
+            responses.GET,
+            "https://airtable.com/v0.3/application/appHI27Un8BKJ9iKA/read",
+            status=200,
+            body=file.read(),
+        )
+
+    progress = Progress(1000)
+
+    database = AirtableHandler.import_from_airtable_to_workspace(
+        workspace,
+        "shra2B9gmVj6kxvNz",
+        storage=storage,
+        progress_builder=progress.create_child_builder(represents_progress=1000),
+    )
+
+    table = database.table_set.all()[0]
+    data = table.get_model(attribute_names=True)
+    row1, row2, row3, row4 = data.objects.all()
+    assert row1.so.value == "o1"
+    assert row1.so_copy.value == "o11"
+
+    assert row2.so.value == "o2"
+    assert row2.so_copy.value == "o21"
+
+    assert row3.so is None
+    assert row3.so_copy.value == "o31"
+
+    assert row4.so.value == "o4"
+    assert row4.so_copy is None
+
+
+@pytest.mark.django_db
+@responses.activate
+def test_import_from_airtable_to_workspace_duplicated_multi_select(
+    data_fixture, tmpdir, django_assert_num_queries
+):
+    workspace = data_fixture.create_workspace()
+    base_path = os.path.join(
+        settings.BASE_DIR, "../../../tests/airtable_responses/multi_select_duplicated"
+    )
+    storage = FileSystemStorage(location=(str(tmpdir)), base_url="http://localhost")
+
+    with open(os.path.join(base_path, "airtable_base.html"), "rb") as file:
+        responses.add(
+            responses.GET,
+            "https://airtable.com/shra2B9gmVj6kxvNz",
+            status=200,
+            body=file.read(),
+            headers={"Set-Cookie": "brw=test;"},
+        )
+
+    with open(os.path.join(base_path, "airtable_application.json"), "rb") as file:
+        responses.add(
+            responses.GET,
+            "https://airtable.com/v0.3/application/appHI27Un8BKJ9iKA/read",
+            status=200,
+            body=file.read(),
+        )
+
+    progress = Progress(1000)
+
+    database = AirtableHandler.import_from_airtable_to_workspace(
+        workspace,
+        "shra2B9gmVj6kxvNz",
+        storage=storage,
+        progress_builder=progress.create_child_builder(represents_progress=1000),
+    )
+
+    table = database.table_set.all()[0]
+    data = table.get_model(attribute_names=True)
+    row1, row2, row3, row4 = data.objects.all()
+
+    assert list(row1.mo.values_list("value", flat=True)) == ["mo1"]
+    assert list(row1.mo_copy.values_list("value", flat=True)) == ["mo11"]
+
+    assert list(row2.mo.values_list("value", flat=True)) == ["mo1", "mo3"]
+    assert list(row2.mo_copy.values_list("value", flat=True)) == ["mo11", "mo33"]
+
+    assert row3.mo.count() == 0
+    assert row3.mo_copy.count() == 0
+
+    assert list(row4.mo.values_list("value", flat=True)) == ["mo2"]
+    assert list(row4.mo_copy.values_list("value", flat=True)) == [
+        "mo22",
+        "mo33",
+        "mo11",
+    ]
+
+
 @pytest.mark.django_db
 @responses.activate
 def test_import_unsupported_publicly_shared_view(data_fixture, tmpdir):
     workspace = data_fixture.create_workspace()
-    base_path = os.path.join(settings.BASE_DIR, "../../../tests/airtable_responses")
+    base_path = os.path.join(
+        settings.BASE_DIR, "../../../tests/airtable_responses/basic"
+    )
     storage = FileSystemStorage(location=(str(tmpdir)), base_url="http://localhost")
 
     with open(os.path.join(base_path, "airtable_view.html"), "rb") as file:

changelog/entries/unreleased/bug/1235_fix_for_duplicated_select_fields_import_from_airtable.json

@@ -0,0 +1,7 @@
+{
+    "type": "bug",
+    "message": "Fix for duplicated select fields import from airtable",
+    "issue_number": 1235,
+    "bullet_points": [],
+    "created_at": "2024-05-10"
+}