Repository: gtalarico/pyairtable (python)

Issue:
ORM make fields optional
Hi,
as you can see I am playing around with ORM.

I have a table that has lots of attributes, some of them are computed fields. I know we have a PR about read-only computed fields, but that is not totally related here.

So I just want to access a subset of the tables fields. Like I want to ignore the computed fields at all. Currently the code fails with a `KeyError` exception.

It should be possible to also only update the fields that were specified. The docs say:

> To update Event Datenbank records, issue a request to the Event Datenbank endpoint. Table names and table ids can be used interchangeably. Using table ids means table name changes do not require modifications to your API request. A PATCH request will only update the fields you specify, leaving the rest as they were. A PUT request will perform a destructive update and clear all unspecified cell values. 

Pull request description:
Ignore extra fields in ORM (fixes #190, option 1)
I'm not sure there's a valid use case for the ORM to fail when it gets a field it doesn't recognize. If someone has a use case in mind, we can always make this a configuration flag on `Meta` later, but I think it should be permissive by default.

Patch:
diff --git a/pyairtable/orm/model.py b/pyairtable/orm/model.py
index 0ae71eb..e58c8aa 100644
--- a/pyairtable/orm/model.py
+++ b/pyairtable/orm/model.py
@@ -216,7 +216,7 @@ class Model(metaclass=abc.ABCMeta):
             record = table.create(fields, typecast=self.typecast)
             did_create = True
         else:
-            record = table.update(self.id, fields, replace=True, typecast=self.typecast)
+            record = table.update(self.id, fields, typecast=self.typecast)
             did_create = False
 
         self.id = record["id"]
@@ -260,15 +260,14 @@ class Model(metaclass=abc.ABCMeta):
         """Create instance from record dictionary"""
         name_attr_map = cls._field_name_attribute_map()
         name_field_map = cls._field_name_descriptor_map()
-        try:
-            # Convert Column Names into model field names
-            # Use field's to_internal to cast into model fields
-            kwargs = {
-                name_attr_map[k]: name_field_map[k].to_internal_value(v)
-                for k, v in record["fields"].items()
-            }
-        except KeyError as exc:
-            raise ValueError("Invalid Field Name: {} for model {}".format(exc, cls))
+        # Convert Column Names into model field names
+        kwargs = {
+            # Use field's to_internal_value to cast into model fields
+            name_attr_map[k]: name_field_map[k].to_internal_value(v)
+            for k, v in record["fields"].items()
+            # Silently proceed if Airtable returns fields we don't recognize
+            if k in name_attr_map
+        }
         instance = cls(**kwargs)
         instance.created_time = record["createdTime"]
         instance.id = record["id"]


Test patch:
diff --git a/pyairtable/testing.py b/pyairtable/testing.py
new file mode 100644
index 0000000..a60c3da
--- /dev/null
+++ b/pyairtable/testing.py
@@ -0,0 +1,58 @@
+"""
+Helper functions for writing tests that use the pyairtable library.
+"""
+import datetime
+import random
+import string
+
+
+def fake_id(type="rec", value=None):
+    """
+    Generates a fake Airtable-style ID.
+
+    Keyword Args:
+        type: the object type prefix, defaults to "rec"
+        value: any value to use as the ID, defaults to random letters and digits
+
+    >>> fake_id()
+    'rec...'
+    >>> fake_id('tbl')
+    'tbl...'
+    >>> fake_id(value='12345')
+    'rec00000000012345'
+    """
+    if value is None:
+        value = "".join(random.sample(string.ascii_letters + string.digits, 14))
+    return type + f"{value:0>14}"[:14]
+
+
+def fake_meta(
+    base_id="appFakeTestingApp",
+    table_name="tblFakeTestingTbl",
+    api_key="patFakePersonalAccessToken",
+):
+    """
+    Returns a ``Meta`` class for inclusion in a ``Model`` subclass.
+    """
+    attrs = {"base_id": base_id, "table_name": table_name, "api_key": api_key}
+    return type("Meta", (), attrs)
+
+
+def fake_record(fields=None, id=None, **other_fields):
+    """
+    Returns a fake record dict with the given field values.
+
+    >>> fake_record({"Name": "Alice"})
+    {'id': '...', 'createdTime': '...', 'fields': {'Name': 'Alice'}}
+
+    >>> fake_record(name='Alice', address='123 Fake St')
+    {'id': '...', 'createdTime': '...', 'fields': {'name': 'Alice', 'address': '123 Fake St'}}
+
+    >>> fake_record(name='Alice', id='123')
+    {'id': 'rec00000000000123', 'createdTime': '...', 'fields': {'name': 'Alice'}}
+    """
+    return {
+        "id": fake_id(value=id),
+        "createdTime": datetime.datetime.now().isoformat() + "Z",
+        "fields": {**(fields or {}), **other_fields},
+    }
diff --git a/tests/integration/test_integration_orm.py b/tests/integration/test_integration_orm.py
index 7b77306..7620b83 100644
--- a/tests/integration/test_integration_orm.py
+++ b/tests/integration/test_integration_orm.py
@@ -88,3 +88,37 @@ def test_integration_orm(Contact, Address):
     assert not rv_address_2.street
     rv_address_2.fetch()
     assert rv_address_2.street == rv_address.street == STREET
+
+
+@pytest.mark.integration
+def test_undeclared_fields():
+    """
+    Test that if our ORM is missing some fields, it does not fail on retrieval
+    and does not clobber their values on save.
+    """
+
+    class Contact(Model):
+        first_name = f.TextField("First Name")
+        last_name = f.TextField("Last Name")
+
+        class Meta:
+            base_id = BASE_ID
+            api_key = os.environ["AIRTABLE_API_KEY"]
+            table_name = "Contact"
+
+    table = Contact.get_table()
+    record = table.create(
+        {
+            "First Name": "Alice",
+            "Last Name": "Arnold",
+            "Email": "alice@example.com",
+            "Birthday": "1970-01-01",
+        }
+    )
+
+    # This should not raise an exception
+    contact = Contact.from_id(record["id"])
+
+    # This should not clobber the values in 'Email' or 'Birthday'
+    contact.save()
+    assert table.get(record["id"]) == record
diff --git a/tests/test_orm.py b/tests/test_orm.py
index c551505..a9d8685 100644
--- a/tests/test_orm.py
+++ b/tests/test_orm.py
@@ -7,6 +7,7 @@ from requests_mock import Mocker
 from pyairtable import Table
 from pyairtable.orm import Model
 from pyairtable.orm import fields as f
+from pyairtable.testing import fake_meta, fake_record
 
 
 def test_model_missing_meta():
@@ -26,25 +27,18 @@ def test_model_overlapping():
     with pytest.raises(ValueError):
 
         class Address(Model):
+            Meta = fake_meta()
             exists = f.TextField("Exists")  # clases with Model.exists()
 
-            class Meta:
-                base_id = "required"
-                table_name = "required"
-                api_key = "required"
-
 
 def test_model():
     class Address(Model):
+        Meta = fake_meta(table_name="Address")
         street = f.TextField("Street")
         number = f.TextField("Number")
 
-        class Meta:
-            base_id = "address_base_id"
-            table_name = "Address"
-            api_key = "fake"
-
     class Contact(Model):
+        Meta = fake_meta(table_name="Contact")
         first_name = f.TextField("First Name")
         last_name = f.TextField("Last Name")
         email = f.EmailField("Email")
@@ -52,11 +46,6 @@ def test_model():
         link = f.LinkField("Link", Address, lazy=True)
         birthday = f.DateField("Birthday")
 
-        class Meta:
-            base_id = "contact_base_id"
-            table_name = "Contact"
-            api_key = "fake"
-
     contact = Contact(
         first_name="Gui",
         last_name="Talarico",
@@ -92,14 +81,10 @@ def test_model():
 
 def test_from_record():
     class Contact(Model):
+        Meta = fake_meta()
         first_name = f.TextField("First Name")
         timestamp = f.DatetimeField("Timestamp")
 
-        class Meta:
-            base_id = "contact_base_id"
-            table_name = "Contact"
-            api_key = "fake"
-
     # Fetch = True
     with mock.patch.object(Table, "get") as m_get:
         m_get.return_value = {
@@ -124,21 +109,13 @@ def test_from_record():
 
 def test_linked_record():
     class Address(Model):
+        Meta = fake_meta(table_name="Address")
         street = f.TextField("Street")
 
-        class Meta:
-            base_id = "address_base_id"
-            table_name = "Address"
-            api_key = "fake"
-
     class Contact(Model):
+        Meta = fake_meta(table_name="Contact")
         address = f.LinkField("Link", Address, lazy=True)
 
-        class Meta:
-            base_id = "contact_base_id"
-            table_name = "Contact"
-            api_key = "fake"
-
     record = {"id": "recFake", "createdTime": "", "fields": {"Street": "A"}}
     address = Address.from_id("recFake", fetch=False)
 
@@ -153,3 +130,38 @@ def test_linked_record():
         contact.address[0].fetch()
 
     assert contact.address[0].street == "A"
+
+
+def test_undeclared_field__from_id(requests_mock):
+    """
+    Test that Model.from_id ignores any fields which are missing from the Model definition.
+    See https://github.com/gtalarico/pyairtable/issues/190
+    """
+
+    class JustName(Model):
+        Meta = fake_meta()
+        name = f.TextField("Name")
+
+    record = fake_record({"Name": "Alice", "Address": "123 Fake St"})
+    requests_mock.get(
+        JustName.get_table().get_record_url(record["id"]),
+        status_code=200,
+        json=record,
+    )
+
+    instance = JustName.from_id(record["id"])
+    assert instance.to_record()["fields"] == {"Name": "Alice"}
+
+
+def test_undeclared_field__all():
+    """
+    Test that Model.all ignores any fields which are missing from the Model definition.
+    """
+    pytest.skip("To be implemented in 2.0.0; see #249")  # TODO
+
+
+def test_undeclared_field__first():
+    """
+    Test that Model.first ignores any fields which are missing from the Model definition.
+    """
+    pytest.skip("To be implemented in 2.0.0; see #249")  # TODO