Skip to content

Commit

Permalink
Refactor AirtableMixin.create_record and update_record into a single …
Browse files Browse the repository at this point in the history
…method (#75)

* Reshuffle AirtableMixin.save to avoid redundant saves

Also, don't bother keeping track of the return value of `super().save()`, because there isn't one.

* Combine `create_record` and `update_record` methods into `save_to_airtable`

Fixes #2

The semantics of the two methods cross over (and call each other - a creation might actually be an update and vice versa) to the point that it isn't worth them being distinct methods. Regardless of whether this is a new Django record or not, the process will always be:

* if we have an `airtable_record_id` and it exists in Airtable, update that record
* otherwise, if we can find an Airtable record based on unique identifier, update that record and adopt its record ID
* otherwise, create a new Airtable record and adopt its record ID

* Update tests to cover all cases of save_to_airtable

* Add test for match_record with AIRTABLE_UNIQUE_IDENTIFIER defined as a dict
  • Loading branch information
gasman authored Dec 16, 2024
1 parent d5f4e04 commit ded5674
Show file tree
Hide file tree
Showing 6 changed files with 237 additions and 215 deletions.
124 changes: 83 additions & 41 deletions tests/mock_airtable.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
"""A mocked Airtable API wrapper."""
from unittest import mock
from requests.exceptions import HTTPError

def get_mock_airtable():
"""
Expand All @@ -14,21 +15,29 @@ def get_iter(self):
MockAirtable.table_name = "app_airtable_advert_base_key"

MockAirtable.get = mock.MagicMock("get")
MockAirtable.get.return_value = {
"id": "recNewRecordId",
"fields": {
"title": "Red! It's the new blue!",
"description": "Red is a scientifically proven color that moves faster than all other colors.",
"external_link": "https://example.com/",
"is_active": True,
"rating": "1.5",
"long_description": "<p>Lorem ipsum dolor sit amet, consectetur adipisicing elit. Veniam laboriosam consequatur saepe. Repellat itaque dolores neque, impedit reprehenderit eum culpa voluptates harum sapiente nesciunt ratione.</p>",
"points": 95,
"slug": "red-its-new-blue",
},
}

def get_fn(record_id):
if record_id == "recNewRecordId":
return {
"id": "recNewRecordId",
"fields": {
"title": "Red! It's the new blue!",
"description": "Red is a scientifically proven color that moves faster than all other colors.",
"external_link": "https://example.com/",
"is_active": True,
"rating": "1.5",
"long_description": "<p>Lorem ipsum dolor sit amet, consectetur adipisicing elit. Veniam laboriosam consequatur saepe. Repellat itaque dolores neque, impedit reprehenderit eum culpa voluptates harum sapiente nesciunt ratione.</p>",
"points": 95,
"slug": "red-its-new-blue",
},
}
else:
raise HTTPError("404 Client Error: Not Found")

MockAirtable.get.side_effect = get_fn

MockAirtable.insert = mock.MagicMock("insert")

MockAirtable.insert.return_value = {
"id": "recNewRecordId",
"fields": {
Expand Down Expand Up @@ -62,34 +71,67 @@ def get_iter(self):
MockAirtable.delete.return_value = {"deleted": True, "record": "recNewRecordId"}

MockAirtable.search = mock.MagicMock("search")
MockAirtable.search.return_value = [
{
"id": "recNewRecordId",
"fields": {
"title": "Red! It's the new blue!",
"description": "Red is a scientifically proven color that moves faster than all other colors.",
"external_link": "https://example.com/",
"is_active": True,
"rating": "1.5",
"long_description": "<p>Lorem ipsum dolor sit amet, consectetur adipisicing elit. Veniam laboriosam consequatur saepe. Repellat itaque dolores neque, impedit reprehenderit eum culpa voluptates harum sapiente nesciunt ratione.</p>",
"points": 95,
"slug": "red-its-new-blue",
},
},
{
"id": "Different record",
"fields": {
"title": "Not the used record.",
"description": "This is only used for multiple responses from MockAirtable",
"external_link": "https://example.com/",
"is_active": False,
"rating": "5.5",
"long_description": "",
"points": 1,
"slug": "not-the-used-record",
},
},
]
def search_fn(field, value):
if field == "slug" and value == "red-its-new-blue":
return [
{
"id": "recNewRecordId",
"fields": {
"title": "Red! It's the new blue!",
"description": "Red is a scientifically proven color that moves faster than all other colors.",
"external_link": "https://example.com/",
"is_active": True,
"rating": "1.5",
"long_description": "<p>Lorem ipsum dolor sit amet, consectetur adipisicing elit. Veniam laboriosam consequatur saepe. Repellat itaque dolores neque, impedit reprehenderit eum culpa voluptates harum sapiente nesciunt ratione.</p>",
"points": 95,
"slug": "red-its-new-blue",
},
},
{
"id": "Different record",
"fields": {
"title": "Not the used record.",
"description": "This is only used for multiple responses from MockAirtable",
"external_link": "https://example.com/",
"is_active": False,
"rating": "5.5",
"long_description": "",
"points": 1,
"slug": "not-the-used-record",
},
},
]
elif field == "slug" and value == "a-matching-slug":
return [
{
"id": "recMatchedRecordId",
"fields": {
"title": "Red! It's the new blue!",
"description": "Red is a scientifically proven color that moves faster than all other colors.",
"external_link": "https://example.com/",
"is_active": True,
"rating": "1.5",
"long_description": "<p>Lorem ipsum dolor sit amet, consectetur adipisicing elit. Veniam laboriosam consequatur saepe. Repellat itaque dolores neque, impedit reprehenderit eum culpa voluptates harum sapiente nesciunt ratione.</p>",
"points": 95,
"slug": "a-matching-slug",
},
},
]
elif field == "Page Slug" and value == "home":
return [
{
"id": "recHomePageId",
"fields": {
"title": "Home",
"Page Slug": "home",
"intro": "This is the home page.",
},
},
]
else:
return []

MockAirtable.search.side_effect = search_fn

MockAirtable.get_all = mock.MagicMock("get_all")
MockAirtable.get_all.return_value = [
Expand Down
4 changes: 2 additions & 2 deletions tests/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,15 @@ class SimplePage(AirtableMixin, Page):
def map_import_fields(cls):
mappings = {
"title": "title",
"slug": "slug",
"Page Slug": "slug",
"intro": "intro",
}
return mappings

def get_export_fields(self):
return {
"title": self.title,
"slug": self.slug,
"Page Slug": self.slug,
"intro": self.intro,
}

Expand Down
2 changes: 1 addition & 1 deletion tests/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@
'tests.SimplePage': {
'AIRTABLE_BASE_KEY': 'xxx',
'AIRTABLE_TABLE_NAME': 'xxx',
'AIRTABLE_UNIQUE_IDENTIFIER': 'slug',
'AIRTABLE_UNIQUE_IDENTIFIER': {'Page Slug': 'slug'},
'AIRTABLE_SERIALIZER': 'tests.serializers.SimplePageSerializer',
'PARENT_PAGE_ID': 'tests.models.get_import_parent_page',
},
Expand Down
10 changes: 5 additions & 5 deletions tests/test_import.py
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ def test_create_page(self, mixin_airtable):
"id": "test-created-page-id",
"fields": {
"title": "A simple page",
"slug": "a-simple-page",
"Page Slug": "a-simple-page",
"intro": "How much more simple can it get? And the answer is none. None more simple.",
},
}]
Expand Down Expand Up @@ -230,7 +230,7 @@ def test_create_and_publish_page(self, mixin_airtable):
"id": "test-created-page-id",
"fields": {
"title": "A simple page",
"slug": "a-simple-page",
"Page Slug": "a-simple-page",
"intro": "How much more simple can it get? And the answer is none. None more simple.",
},
}]
Expand Down Expand Up @@ -265,7 +265,7 @@ def test_update_page(self, mixin_airtable):
"id": "test-created-page-id",
"fields": {
"title": "A simple page",
"slug": "a-simple-page",
"Page Slug": "a-simple-page",
"intro": "How much more simple can it get? Oh, actually it can get more simple.",
},
}]
Expand Down Expand Up @@ -299,7 +299,7 @@ def test_skip_update_page_if_unchanged(self, mixin_airtable):
"id": "test-created-page-id",
"fields": {
"title": "A simple page",
"slug": "a-simple-page",
"Page Slug": "a-simple-page",
"intro": "How much more simple can it get? And the answer is none. None more simple.",
},
}]
Expand Down Expand Up @@ -330,7 +330,7 @@ def test_skip_update_page_if_locked(self, mixin_airtable):
"id": "test-created-page-id",
"fields": {
"title": "A simple page",
"slug": "a-simple-page",
"Page Slug": "a-simple-page",
"intro": "How much more simple can it get? Oh, actually it can get more simple.",
},
}]
Expand Down
Loading

0 comments on commit ded5674

Please sign in to comment.