Repository: gtalarico/pyairtable (python)

Issue:
Model.from_id() returns empty fields when a model is defined using field ids rather than field names (and use_field_ids = True)
Hi there, 

This seems a similar to [Issue #421 ](https://github.com/gtalarico/pyairtable/issues/421). 

Essentially when retrieving a record via Model.from_id() the field values are empty when using field ids in the definition of the model. 

```

class FreeTvDestinationRecord(Model):

    class Meta:
        base_id = 'app1234'
        table_name = 'tbl1234'
        api_key = os.environ.get('AIRTABLE_TOKEN')
        use_field_ids = True

    houseNumber = "fld1234"

test =  FreeTvDestinationRecord.from_id('rec12345')

```

The above has an empty string when data is present in the airtable record. 
Using the actual field name works as does using _from_ids() method.

If I change the instance:

```
test.houseNumber = 'TESTING'
test.save()
```
The field will update correctly in airtable, 

however, if I subsequently call .fetch()

`test.fetch()`

the field will now appear empty. 

So I guess the issue here is actually the underlying fetch() method? 

Many thanks for the library in its current form! 

Cheers
H 


Pull request description:
Release 3.1.1
Pass returnFieldsByFieldId everywhere in the ORM; fixes #430

Patch:
diff --git a/docs/source/changelog.rst b/docs/source/changelog.rst
index bb4cd36..f77b470 100644
--- a/docs/source/changelog.rst
+++ b/docs/source/changelog.rst
@@ -2,6 +2,12 @@
 Changelog
 =========
 
+3.1.1 (2025-04-07)
+------------------------
+
+* Fix a bug affecting :meth:`~pyairtable.orm.Model.from_id` when ``use_field_ids=True``.
+  - `PR #431 <https://github.com/gtalarico/pyairtable/pull/431>`_
+
 3.1.0 (2025-04-07)
 ------------------------
 
diff --git a/pyairtable/__init__.py b/pyairtable/__init__.py
index d090541..51c4acf 100644
--- a/pyairtable/__init__.py
+++ b/pyairtable/__init__.py
@@ -1,4 +1,4 @@
-__version__ = "3.1.0"
+__version__ = "3.1.1"
 
 from pyairtable.api import Api, Base, Table
 from pyairtable.api.enterprise import Enterprise
diff --git a/pyairtable/orm/model.py b/pyairtable/orm/model.py
index 188c9b6..c2651ad 100644
--- a/pyairtable/orm/model.py
+++ b/pyairtable/orm/model.py
@@ -245,7 +245,11 @@ class Model:
         field_values = self.to_record(only_writable=True)["fields"]
 
         if not self.id:
-            record = self.meta.table.create(field_values, typecast=self.meta.typecast)
+            record = self.meta.table.create(
+                field_values,
+                typecast=self.meta.typecast,
+                use_field_ids=self.meta.use_field_ids,
+            )
             self.id = record["id"]
             self.created_time = datetime_from_iso_str(record["createdTime"])
             self._changed.clear()
@@ -260,7 +264,12 @@ class Model:
                 if self._changed.get(field_name)
             }
 
-        self.meta.table.update(self.id, field_values, typecast=self.meta.typecast)
+        self.meta.table.update(
+            self.id,
+            field_values,
+            typecast=self.meta.typecast,
+            use_field_ids=self.meta.use_field_ids,
+        )
         self._changed.clear()
         return SaveResult(
             self.id, forced=force, updated=True, field_names=set(field_values)
@@ -407,7 +416,7 @@ class Model:
         if not self.id:
             raise ValueError("cannot be fetched because instance does not have an id")
 
-        record = self.meta.table.get(self.id)
+        record = self.meta.table.get(self.id, **self.meta.request_kwargs)
         unused = self.from_record(record, memoize=False)
         self._fields = unused._fields
         self._changed.clear()
@@ -478,12 +487,21 @@ class Model:
             if (record := model.to_record(only_writable=True))
         ]
 
-        table = cls.meta.table
-        table.batch_update(update_records, typecast=cls.meta.typecast)
-        created_records = table.batch_create(create_records, typecast=cls.meta.typecast)
-        for model, record in zip(create_models, created_records):
-            model.id = record["id"]
-            model.created_time = datetime_from_iso_str(record["createdTime"])
+        if update_records:
+            cls.meta.table.batch_update(
+                update_records,
+                typecast=cls.meta.typecast,
+                use_field_ids=cls.meta.use_field_ids,
+            )
+        if create_records:
+            created_records = cls.meta.table.batch_create(
+                create_records,
+                typecast=cls.meta.typecast,
+                use_field_ids=cls.meta.use_field_ids,
+            )
+            for model, record in zip(create_models, created_records):
+                model.id = record["id"]
+                model.created_time = datetime_from_iso_str(record["createdTime"])
 
     @classmethod
     def batch_delete(cls, models: List[SelfType]) -> None:


Test patch:
diff --git a/tests/integration/test_integration_api.py b/tests/integration/test_integration_api.py
index 72291ff..1f43ad6 100644
--- a/tests/integration/test_integration_api.py
+++ b/tests/integration/test_integration_api.py
@@ -56,11 +56,16 @@ def test_use_field_ids(table: Table, cols):
     Test that we can get, create, and update records by field ID vs. name.
 
     See https://github.com/gtalarico/pyairtable/issues/194
+    and https://github.com/gtalarico/pyairtable/issues/430
     """
     # Create one record with use_field_ids=True
     record = table.create({cols.TEXT_ID: "Hello"}, use_field_ids=True)
     assert record["fields"][cols.TEXT_ID] == "Hello"
 
+    # Fetch one record with use_field_ids=True
+    fetched = table.get(record["id"], use_field_ids=True)
+    assert fetched["fields"][cols.TEXT_ID] == "Hello"
+
     # Update one record with use_field_ids=True
     updated = table.update(
         record["id"],
@@ -82,6 +87,13 @@ def test_use_field_ids(table: Table, cols):
     assert records[1]["fields"][cols.TEXT_ID] == "Bravo"
     assert records[2]["fields"][cols.TEXT_ID] == "Charlie"
 
+    # Fetch multiple records with use_field_ids=True
+    formula = OR(RECORD_ID().eq(record["id"]) for record in records)
+    fetched_many = {r["id"]: r for r in table.all(formula=formula, use_field_ids=True)}
+    assert fetched_many[records[0]["id"]]["fields"][cols.TEXT_ID] == "Alpha"
+    assert fetched_many[records[1]["id"]]["fields"][cols.TEXT_ID] == "Bravo"
+    assert fetched_many[records[2]["id"]]["fields"][cols.TEXT_ID] == "Charlie"
+
     # Update multiple records with use_field_ids=True
     updates = [
         dict(
diff --git a/tests/test_orm.py b/tests/test_orm.py
index 1ead27b..1551201 100644
--- a/tests/test_orm.py
+++ b/tests/test_orm.py
@@ -175,6 +175,7 @@ def test_unmodified_field_not_saved(contact_record):
             contact.id,
             {"Email": "john.doe@example.com"},
             typecast=True,
+            use_field_ids=False,
         )
 
     # Once saved, the field is no longer marked as changed
@@ -195,6 +196,7 @@ def test_unmodified_field_not_saved(contact_record):
                 "Birthday": "1970-01-01",
             },
             typecast=True,
+            use_field_ids=False,
         )
 
 
@@ -339,6 +341,7 @@ def test_batch_save(mock_update, mock_create):
             {"Number": 456, "Street": "Fake St"},
         ],
         typecast=True,
+        use_field_ids=False,
     )
     mock_update.assert_called_once_with(
         [
@@ -348,9 +351,28 @@ def test_batch_save(mock_update, mock_create):
             },
         ],
         typecast=True,
+        use_field_ids=False,
     )
 
 
+@mock.patch("pyairtable.Table.batch_create")
+@mock.patch("pyairtable.Table.batch_update")
+def test_batch_save__only_create(mock_update, mock_create):
+    Address.batch_save([Address(), Address()])
+    assert mock_create.call_count == 1
+    assert mock_update.call_count == 0
+
+
+@mock.patch("pyairtable.Table.batch_create")
+@mock.patch("pyairtable.Table.batch_update")
+def test_batch_save__only_update(mock_update, mock_create):
+    a1 = Address.from_record(fake_record())
+    a2 = Address.from_record(fake_record())
+    Address.batch_save([a1, a2])
+    assert mock_create.call_count == 0
+    assert mock_update.call_count == 1
+
+
 @mock.patch("pyairtable.Table.batch_create")
 @mock.patch("pyairtable.Table.batch_update")
 def test_batch_save__invalid_class(mock_update, mock_create):
diff --git a/tests/test_orm_fields.py b/tests/test_orm_fields.py
index 8702fbd..4b0f1f1 100644
--- a/tests/test_orm_fields.py
+++ b/tests/test_orm_fields.py
@@ -845,7 +845,7 @@ def test_single_link_field():
 
     with mock.patch("pyairtable.Table.get", return_value=alice.to_record()) as m:
         book.author.fetch()
-        m.assert_called_once_with(alice.id)
+        m.assert_called_once_with(alice.id, **Author.meta.request_kwargs)
 
     assert book.author.id == alice.id
     assert book.author.name == "Alice"
@@ -856,16 +856,29 @@ def test_single_link_field():
 
     with mock.patch("pyairtable.Table.create", return_value=fake_record()) as m:
         book.author.save()
-        m.assert_called_once_with({"Name": "Bob"}, typecast=True)
+        m.assert_called_once_with(
+            {"Name": "Bob"},
+            typecast=True,
+            use_field_ids=False,
+        )
 
     with mock.patch("pyairtable.Table.create", return_value=fake_record()) as m:
         book.save()
-        m.assert_called_once_with({"Author": [bob.id]}, typecast=True)
+        m.assert_called_once_with(
+            {"Author": [bob.id]},
+            typecast=True,
+            use_field_ids=False,
+        )
 
     with mock.patch("pyairtable.Table.update", return_value=book.to_record()) as m:
         book.author = None
         book.save()
-        m.assert_called_once_with(book.id, {"Author": None}, typecast=True)
+        m.assert_called_once_with(
+            book.id,
+            {"Author": None},
+            typecast=True,
+            use_field_ids=False,
+        )
 
 
 def test_single_link_field__multiple_values():
@@ -901,13 +914,23 @@ def test_single_link_field__multiple_values():
     # if book.author.__set__ not called, the entire list will be sent back to the API
     with mock.patch("pyairtable.Table.update", return_value=book.to_record()) as m:
         book.save(force=True)
-        m.assert_called_once_with(book.id, {"Author": [a1, a2, a3]}, typecast=True)
+        m.assert_called_once_with(
+            book.id,
+            {"Author": [a1, a2, a3]},
+            typecast=True,
+            use_field_ids=False,
+        )
 
     # if we modify the field value, it will drop items 2-N
     book.author = Author.from_record(fake_record())
     with mock.patch("pyairtable.Table.update", return_value=book.to_record()) as m:
         book.save()
-        m.assert_called_once_with(book.id, {"Author": [book.author.id]}, typecast=True)
+        m.assert_called_once_with(
+            book.id,
+            {"Author": [book.author.id]},
+            typecast=True,
+            use_field_ids=False,
+        )
 
 
 def test_single_link_field__raise_if_many():
@@ -1114,7 +1137,12 @@ def test_select_field(fields, expected):
 
     with mock.patch("pyairtable.Table.update", return_value=obj.to_record()) as m:
         obj.save(force=True)
-        m.assert_called_once_with(obj.id, fields, typecast=True)
+        m.assert_called_once_with(
+            obj.id,
+            fields,
+            typecast=True,
+            use_field_ids=False,
+        )
 
 
 @pytest.mark.parametrize(
diff --git a/tests/test_orm_model.py b/tests/test_orm_model.py
index a5d7977..6729d8f 100644
--- a/tests/test_orm_model.py
+++ b/tests/test_orm_model.py
@@ -22,8 +22,8 @@ class FakeModel(Model):
 
 class FakeModelByIds(Model):
     Meta = fake_meta(use_field_ids=True, table_name="Apartments")
-    Name = f.TextField("fld1VnoyuotSTyxW1")
-    Age = f.NumberField("fld2VnoyuotSTy4g6")
+    name = f.TextField("fld1VnoyuotSTyxW1")
+    age = f.NumberField("fld2VnoyuotSTy4g6")
 
 
 @pytest.fixture(autouse=True)
@@ -215,6 +215,17 @@ def test_from_id(mock_get):
     assert contact.name == "Alice"
 
 
+@mock.patch("pyairtable.Table.get")
+def test_from_id__use_field_ids(mock_get, fake_records_by_id):
+    # Use the FakeModelByIds class to test the use_field_ids option.
+    fake_contact = fake_records_by_id[0]
+    mock_get.return_value = fake_contact
+    model = FakeModelByIds.from_id(fake_contact["id"])
+    assert model.name == "Alice"
+    assert mock_get.call_count == 1
+    assert mock_get.mock_calls[-1].kwargs["use_field_ids"] is True
+
+
 @mock.patch("pyairtable.Api.iterate_requests")
 def test_from_ids(mock_api):
     fake_records = [fake_record() for _ in range(10)]
@@ -257,10 +268,21 @@ def test_from_ids__no_fetch(mock_all):
 @mock.patch("pyairtable.Table.all")
 def test_from_ids__use_field_ids(mock_all):
     fake_ids = [fake_id() for _ in range(10)]
-    mock_all.return_value = [fake_record(id=id) for id in fake_ids]
-    FakeModelByIds.from_ids(fake_ids)
+    mock_all.return_value = [
+        fake_record(
+            id=record_id,
+            fld1VnoyuotSTyxW1=f"Name {idx}",
+            fld2VnoyuotSTy4g6=(idx + 40),
+        )
+        for idx, record_id in enumerate(fake_ids)
+    ]
+    models = FakeModelByIds.from_ids(fake_ids)
     assert mock_all.call_count == 1
     assert mock_all.mock_calls[-1].kwargs["use_field_ids"] is True
+    assert models[0].name == "Name 0"
+    assert models[0].age == 40
+    assert models[1].name == "Name 1"
+    assert models[1].age == 41
 
 
 @pytest.mark.parametrize(
@@ -292,12 +314,10 @@ def test_passthrough(methodname, returns):
 
 @pytest.fixture
 def fake_records_by_id():
-    return {
-        "records": [
-            fake_record(fld1VnoyuotSTyxW1="Alice", fld2VnoyuotSTy4g6=25),
-            fake_record(Name="Jack", Age=30),
-        ]
-    }
+    return [
+        fake_record(fld1VnoyuotSTyxW1="Alice"),
+        fake_record(Name="Jack"),  # values for negative test
+    ]
 
 
 def test_get_fields_by_id(fake_records_by_id):
@@ -310,20 +330,14 @@ def test_get_fields_by_id(fake_records_by_id):
                 returnFieldsByFieldId=1,
                 cellFormat="json",
             ),
-            json=fake_records_by_id,
+            json={"records": fake_records_by_id},
             complete_qs=True,
             status_code=200,
         )
         fake_models = FakeModelByIds.all()
 
-    assert fake_models[0].Name == "Alice"
-    assert fake_models[0].Age == 25
-
-    assert fake_models[1].Name != "Jack"
-    assert fake_models[1].Age != 30
-
-    with pytest.raises(KeyError):
-        _ = getattr(fake_models[1], fake_records_by_id[0]["Age"])
+    assert fake_models[0].name == "Alice"
+    assert fake_models[1].name == ""
 
 
 def test_meta_wrapper():
@@ -412,7 +426,11 @@ def test_save__create(mock_create):
     assert result.field_names == {"one", "two"}
     assert not result.updated
     assert not result.forced
-    mock_create.assert_called_once_with({"one": "ONE", "two": "TWO"}, typecast=True)
+    mock_create.assert_called_once_with(
+        {"one": "ONE", "two": "TWO"},
+        typecast=True,
+        use_field_ids=False,
+    )
 
 
 @mock.patch("pyairtable.Table.update")
@@ -428,7 +446,12 @@ def test_save__update(mock_update):
     assert result.updated
     assert result.field_names == {"one"}
     assert not result.forced
-    mock_update.assert_called_once_with(obj.id, {"one": "new value"}, typecast=True)
+    mock_update.assert_called_once_with(
+        obj.id,
+        {"one": "new value"},
+        typecast=True,
+        use_field_ids=False,
+    )
 
 
 @mock.patch("pyairtable.Table.update")
@@ -446,7 +469,7 @@ def test_save__update_force(mock_update):
     assert result.forced
     assert result.field_names == {"one", "two"}
     mock_update.assert_called_once_with(
-        obj.id, {"one": "new value", "two": "TWO"}, typecast=True
+        obj.id, {"one": "new value", "two": "TWO"}, typecast=True, use_field_ids=False
     )
 
 
@@ -465,6 +488,39 @@ def test_save__noop(mock_update):
     mock_update.assert_not_called()
 
 
+@mock.patch("pyairtable.Table.create")
+def test_save__use_field_ids__create(mock_create):
+    """
+    Test that we can correctly save a model which uses field IDs.
+    """
+    mock_create.return_value = fake_record(**{FakeModelByIds.name.field_name: "Alice"})
+    obj = FakeModelByIds(name="Alice")
+    obj.save()
+    mock_create.assert_called_once_with(
+        {FakeModelByIds.name.field_name: "Alice"},
+        typecast=True,
+        use_field_ids=True,
+    )
+
+
+@mock.patch("pyairtable.Table.update")
+def test_save__use_field_ids__update(mock_update):
+    """
+    Test that we can correctly save a model which uses field IDs.
+    """
+    record = fake_record(**{FakeModelByIds.name.field_name: "Alice"})
+    mock_update.return_value = record
+    obj = FakeModelByIds.from_record(record)
+    obj.name = "Bob"
+    obj.save()
+    mock_update.assert_called_once_with(
+        obj.id,
+        {FakeModelByIds.name.field_name: "Bob"},
+        typecast=True,
+        use_field_ids=True,
+    )
+
+
 def test_save_bool_deprecated():
     """
     Test that SaveResult instances can be used as booleans, but emit a deprecation warning.