From c88c316d63e64774317e68562032764c3afe6edd Mon Sep 17 00:00:00 2001
From: Bram Wiepjes <bramw@protonmail.com>
Date: Fri, 14 Feb 2025 08:44:33 +0000
Subject: [PATCH] Airtable import currency field improvements

---
 .../airtable/airtable_column_types.py         |  66 ++++-
 .../contrib/database/airtable/constants.py    |   6 +
 .../airtable/test_airtable_column_types.py    | 249 +++++++++++++++++-
 ...ve_airtable_import_currency_precision.json |   7 +
 ...eserve_currency_symbol_and_formatting.json |   7 +
 5 files changed, 324 insertions(+), 11 deletions(-)
 create mode 100644 changelog/entries/unreleased/bug/1058_preserve_airtable_import_currency_precision.json
 create mode 100644 changelog/entries/unreleased/bug/3395_preserve_currency_symbol_and_formatting.json

diff --git a/backend/src/baserow/contrib/database/airtable/airtable_column_types.py b/backend/src/baserow/contrib/database/airtable/airtable_column_types.py
index 08eedd06b..ec0f77e19 100644
--- a/backend/src/baserow/contrib/database/airtable/airtable_column_types.py
+++ b/backend/src/baserow/contrib/database/airtable/airtable_column_types.py
@@ -1,5 +1,5 @@
 from datetime import datetime, timezone
-from decimal import Decimal
+from decimal import Decimal, InvalidOperation
 from typing import Any, Dict, Optional
 
 from django.core.exceptions import ValidationError
@@ -28,6 +28,7 @@ from baserow.contrib.database.fields.models import (
 from baserow.contrib.database.fields.registries import field_type_registry
 
 from .config import AirtableImportConfig
+from .constants import AIRTABLE_NUMBER_FIELD_SEPARATOR_FORMAT_MAPPING
 from .helpers import import_airtable_date_type_options, set_select_options_on_field
 from .import_report import (
     ERROR_TYPE_DATA_TYPE_MISMATCH,
@@ -155,17 +156,37 @@ class NumberAirtableColumnType(AirtableColumnType):
         self, raw_airtable_table, raw_airtable_column, config, import_report
     ):
         type_options = raw_airtable_column.get("typeOptions", {})
-        decimal_places = 0
+        options_format = type_options.get("format", "")
+        suffix = ""
 
-        if type_options.get("format", "integer") == "decimal":
-            # Minimum of 1 and maximum of 5 decimal places.
-            decimal_places = min(
-                max(1, type_options.get("precision", 1)), NUMBER_MAX_DECIMAL_PLACES
+        if "percent" in options_format:
+            suffix = "%"
+
+        decimal_places = min(
+            max(0, type_options.get("precision", 0)), NUMBER_MAX_DECIMAL_PLACES
+        )
+        prefix = type_options.get("symbol", "")
+        separator_format = type_options.get("separatorFormat", "")
+        number_separator = AIRTABLE_NUMBER_FIELD_SEPARATOR_FORMAT_MAPPING.get(
+            separator_format, ""
+        )
+
+        if separator_format != "" and number_separator == "":
+            import_report.add_failed(
+                f"Number field: \"{raw_airtable_column['name']}\"",
+                SCOPE_FIELD,
+                raw_airtable_table.get("name", ""),
+                ERROR_TYPE_UNSUPPORTED_FEATURE,
+                f"The field was imported, but the separator format "
+                f"{separator_format} was dropped because it doesn't exist in Baserow.",
             )
 
         return NumberField(
             number_decimal_places=decimal_places,
             number_negative=type_options.get("negative", True),
+            number_prefix=prefix,
+            number_suffix=suffix,
+            number_separator=number_separator,
         )
 
     def to_baserow_export_serialized_value(
@@ -180,13 +201,38 @@ class NumberAirtableColumnType(AirtableColumnType):
         config,
         import_report,
     ):
-        if value is not None:
+        if value is None:
+            return None
+
+        try:
             value = Decimal(value)
+        except InvalidOperation:
+            # If the value can't be parsed as decimal, then it might be corrupt, so we
+            # need to inform the user and skip the import.
+            row_name = get_airtable_row_primary_value(
+                raw_airtable_table, raw_airtable_row
+            )
+            import_report.add_failed(
+                f"Row: \"{row_name}\", field: \"{raw_airtable_column['name']}\"",
+                SCOPE_CELL,
+                raw_airtable_table["name"],
+                ERROR_TYPE_DATA_TYPE_MISMATCH,
+                f"Cell value was left empty because the numeric value {value} "
+                f'could not be parsed"',
+            )
+            return None
 
-        if value is not None and not baserow_field.number_negative and value < 0:
-            value = None
+        # Airtable stores 10% as 0.1, so we would need to multiply it by 100 so get the
+        # correct value in Baserow.
+        type_options = raw_airtable_column.get("typeOptions", {})
+        options_format = type_options.get("format", "")
+        if "percent" in options_format:
+            value = value * 100
 
-        return None if value is None else str(value)
+        if not baserow_field.number_negative and value < 0:
+            return None
+
+        return str(value)
 
 
 class RatingAirtableColumnType(AirtableColumnType):
diff --git a/backend/src/baserow/contrib/database/airtable/constants.py b/backend/src/baserow/contrib/database/airtable/constants.py
index 2f6da057b..e74182723 100644
--- a/backend/src/baserow/contrib/database/airtable/constants.py
+++ b/backend/src/baserow/contrib/database/airtable/constants.py
@@ -13,3 +13,9 @@ AIRTABLE_BASEROW_COLOR_MAPPING = {
     "purple": "dark-blue",
     "gray": "light-gray",
 }
+AIRTABLE_NUMBER_FIELD_SEPARATOR_FORMAT_MAPPING = {
+    "commaPeriod": "COMMA_PERIOD",
+    "periodComma": "PERIOD_COMMA",
+    "spaceComma": "SPACE_COMMA",
+    "spacePeriod": "SPACE_PERIOD",
+}
diff --git a/backend/tests/baserow/contrib/database/airtable/test_airtable_column_types.py b/backend/tests/baserow/contrib/database/airtable/test_airtable_column_types.py
index 41eba6ce2..7192deca9 100644
--- a/backend/tests/baserow/contrib/database/airtable/test_airtable_column_types.py
+++ b/backend/tests/baserow/contrib/database/airtable/test_airtable_column_types.py
@@ -1196,6 +1196,9 @@ def test_airtable_import_number_integer_column(data_fixture, api_client):
     assert isinstance(airtable_column_type, NumberAirtableColumnType)
     assert baserow_field.number_decimal_places == 0
     assert baserow_field.number_negative is False
+    assert baserow_field.number_separator == ""
+    assert baserow_field.number_prefix == ""
+    assert baserow_field.number_suffix == ""
 
     assert (
         airtable_column_type.to_baserow_export_serialized_value(
@@ -1269,6 +1272,50 @@ def test_airtable_import_number_integer_column(data_fixture, api_client):
     )
 
 
+@pytest.mark.django_db
+@responses.activate
+def test_airtable_import_number_invalid_number(data_fixture, api_client):
+    airtable_field = {
+        "id": "fldZBmr4L45mhjILhlA",
+        "name": "Number",
+        "type": "number",
+        "typeOptions": {
+            "format": "integer",
+            "negative": False,
+            "validatorName": "positive",
+        },
+    }
+    (
+        baserow_field,
+        airtable_column_type,
+    ) = airtable_column_type_registry.from_airtable_column_to_serialized(
+        {},
+        airtable_field,
+        AirtableImportConfig(),
+        AirtableImportReport(),
+    )
+
+    import_report = AirtableImportReport()
+    assert (
+        airtable_column_type.to_baserow_export_serialized_value(
+            {},
+            {"name": "Test"},
+            {"id": "row1"},
+            airtable_field,
+            baserow_field,
+            "INVALID_NUMBER",
+            {},
+            AirtableImportConfig(),
+            import_report,
+        )
+        is None
+    )
+    assert len(import_report.items) == 1
+    assert import_report.items[0].object_name == 'Row: "row1", field: "Number"'
+    assert import_report.items[0].scope == SCOPE_CELL
+    assert import_report.items[0].table == "Test"
+
+
 @pytest.mark.django_db
 @responses.activate
 def test_airtable_import_number_decimal_column(data_fixture, api_client):
@@ -1293,7 +1340,7 @@ def test_airtable_import_number_decimal_column(data_fixture, api_client):
     )
     assert isinstance(baserow_field, NumberField)
     assert isinstance(airtable_column_type, NumberAirtableColumnType)
-    assert baserow_field.number_decimal_places == 1
+    assert baserow_field.number_decimal_places == 0
     assert baserow_field.number_negative is False
 
     airtable_field = {
@@ -1319,6 +1366,9 @@ def test_airtable_import_number_decimal_column(data_fixture, api_client):
     assert isinstance(airtable_column_type, NumberAirtableColumnType)
     assert baserow_field.number_decimal_places == 2
     assert baserow_field.number_negative is True
+    assert baserow_field.number_separator == ""
+    assert baserow_field.number_prefix == ""
+    assert baserow_field.number_suffix == ""
 
     assert (
         airtable_column_type.to_baserow_export_serialized_value(
@@ -1414,6 +1464,203 @@ def test_airtable_import_number_decimal_column(data_fixture, api_client):
     assert isinstance(airtable_column_type, NumberAirtableColumnType)
     assert baserow_field.number_decimal_places == 10
     assert baserow_field.number_negative is True
+    assert baserow_field.number_separator == ""
+    assert baserow_field.number_prefix == ""
+    assert baserow_field.number_suffix == ""
+
+
+@pytest.mark.django_db
+@responses.activate
+def test_airtable_import_currency_column(data_fixture, api_client):
+    airtable_field = {
+        "id": "fldZBmr4L45mhjILhlA",
+        "name": "Currency",
+        "type": "number",
+        "typeOptions": {
+            "format": "currency",
+            "precision": 3,
+            "symbol": "$",
+            "separatorFormat": "commaPeriod",
+            "negative": False,
+        },
+    }
+    (
+        baserow_field,
+        airtable_column_type,
+    ) = airtable_column_type_registry.from_airtable_column_to_serialized(
+        {},
+        airtable_field,
+        AirtableImportConfig(),
+        AirtableImportReport(),
+    )
+    assert isinstance(baserow_field, NumberField)
+    assert isinstance(airtable_column_type, NumberAirtableColumnType)
+    assert baserow_field.number_decimal_places == 3
+    assert baserow_field.number_negative is False
+    assert baserow_field.number_separator == "COMMA_PERIOD"
+    assert baserow_field.number_prefix == "$"
+    assert baserow_field.number_suffix == ""
+
+    airtable_field = {
+        "id": "fldZBmr4L45mhjILhlA",
+        "name": "Currency",
+        "type": "number",
+        "typeOptions": {
+            "format": "currency",
+            "precision": 2,
+            "symbol": "€",
+            "separatorFormat": "spacePeriod",
+            "negative": True,
+        },
+    }
+    (
+        baserow_field,
+        airtable_column_type,
+    ) = airtable_column_type_registry.from_airtable_column_to_serialized(
+        {},
+        airtable_field,
+        AirtableImportConfig(),
+        AirtableImportReport(),
+    )
+    assert isinstance(baserow_field, NumberField)
+    assert isinstance(airtable_column_type, NumberAirtableColumnType)
+    assert baserow_field.number_decimal_places == 2
+    assert baserow_field.number_negative is True
+    assert baserow_field.number_separator == "SPACE_PERIOD"
+    assert baserow_field.number_prefix == "€"
+    assert baserow_field.number_suffix == ""
+
+
+@pytest.mark.django_db
+@responses.activate
+def test_airtable_import_currency_column_non_existing_separator_format(
+    data_fixture, api_client
+):
+    airtable_field = {
+        "id": "fldZBmr4L45mhjILhlA",
+        "name": "Currency",
+        "type": "number",
+        "typeOptions": {
+            "format": "currency",
+            "precision": 3,
+            "symbol": "$",
+            "separatorFormat": "TEST",
+            "negative": False,
+        },
+    }
+    import_report = AirtableImportReport()
+    airtable_column_type_registry.from_airtable_column_to_serialized(
+        {},
+        airtable_field,
+        AirtableImportConfig(),
+        import_report,
+    )
+    assert len(import_report.items) == 1
+    assert import_report.items[0].object_name == 'Number field: "Currency"'
+    assert import_report.items[0].scope == SCOPE_FIELD
+    assert import_report.items[0].table == ""
+
+
+@pytest.mark.django_db
+@responses.activate
+def test_airtable_import_percentage_column(data_fixture, api_client):
+    airtable_field = {
+        "id": "fldZBmr4L45mhjILhlA",
+        "name": "Currency",
+        "type": "number",
+        "typeOptions": {
+            "format": "percentage",
+            "precision": 1,
+            "negative": False,
+        },
+    }
+    (
+        baserow_field,
+        airtable_column_type,
+    ) = airtable_column_type_registry.from_airtable_column_to_serialized(
+        {},
+        airtable_field,
+        AirtableImportConfig(),
+        AirtableImportReport(),
+    )
+    assert isinstance(baserow_field, NumberField)
+    assert isinstance(airtable_column_type, NumberAirtableColumnType)
+    assert baserow_field.number_decimal_places == 1
+    assert baserow_field.number_negative is False
+    assert baserow_field.number_separator == ""
+    assert baserow_field.number_prefix == ""
+    assert baserow_field.number_suffix == "%"
+
+    assert (
+        airtable_column_type.to_baserow_export_serialized_value(
+            {},
+            {"name": "Test"},
+            {"id": "row1"},
+            airtable_field,
+            baserow_field,
+            0.5,
+            {},
+            AirtableImportConfig(),
+            AirtableImportReport(),
+        )
+        == "50.0"
+    )
+    assert (
+        airtable_column_type.to_baserow_export_serialized_value(
+            {},
+            {"name": "Test"},
+            {"id": "row1"},
+            airtable_field,
+            baserow_field,
+            0.5,
+            {},
+            AirtableImportConfig(),
+            AirtableImportReport(),
+        )
+        == "50.0"
+    )
+    assert (
+        airtable_column_type.to_baserow_export_serialized_value(
+            {},
+            {"name": "Test"},
+            {"id": "row1"},
+            airtable_field,
+            baserow_field,
+            "0.05",
+            {},
+            AirtableImportConfig(),
+            AirtableImportReport(),
+        )
+        == "5.00"
+    )
+    assert (
+        airtable_column_type.to_baserow_export_serialized_value(
+            {},
+            {"name": "Test"},
+            {"id": "row1"},
+            airtable_field,
+            baserow_field,
+            "",
+            {},
+            AirtableImportConfig(),
+            AirtableImportReport(),
+        )
+        is None
+    )
+    assert (
+        airtable_column_type.to_baserow_export_serialized_value(
+            {},
+            {"name": "Test"},
+            {"id": "row1"},
+            airtable_field,
+            baserow_field,
+            None,
+            {},
+            AirtableImportConfig(),
+            AirtableImportReport(),
+        )
+        is None
+    )
 
 
 @pytest.mark.django_db
diff --git a/changelog/entries/unreleased/bug/1058_preserve_airtable_import_currency_precision.json b/changelog/entries/unreleased/bug/1058_preserve_airtable_import_currency_precision.json
new file mode 100644
index 000000000..c64bc4b65
--- /dev/null
+++ b/changelog/entries/unreleased/bug/1058_preserve_airtable_import_currency_precision.json
@@ -0,0 +1,7 @@
+{
+  "type": "bug",
+  "message": "Preserve the precision of the currency field in the Airtable import.",
+  "issue_number": 1058,
+  "bullet_points": [],
+  "created_at": "2025-02-09"
+}
diff --git a/changelog/entries/unreleased/bug/3395_preserve_currency_symbol_and_formatting.json b/changelog/entries/unreleased/bug/3395_preserve_currency_symbol_and_formatting.json
new file mode 100644
index 000000000..6697fba0a
--- /dev/null
+++ b/changelog/entries/unreleased/bug/3395_preserve_currency_symbol_and_formatting.json
@@ -0,0 +1,7 @@
+{
+  "type": "bug",
+  "message": "Preserve the currency symbol and formatting of the currency field in Airtable import.",
+  "issue_number": 3395,
+  "bullet_points": [],
+  "created_at": "2025-02-09"
+}