From 88ee2b5aa4fe50c9b9d08482f322c6be3dfd7c76 Mon Sep 17 00:00:00 2001
From: jacobtoppm <jacobtm@torchbox.com>
Date: Thu, 8 Oct 2020 15:40:01 +0100
Subject: [PATCH 01/11] Only save revisions at end of import to avoid capturing
 intermediate states of child objects, preventing tags from importing or
 deleting properly

---
 wagtail_transfer/operations.py | 20 +++++++-------------
 1 file changed, 7 insertions(+), 13 deletions(-)

diff --git a/wagtail_transfer/operations.py b/wagtail_transfer/operations.py
index 8d8e68f..c5494e6 100644
--- a/wagtail_transfer/operations.py
+++ b/wagtail_transfer/operations.py
@@ -449,6 +449,13 @@ def run(self):
         with transaction.atomic():
             for operation in operation_order:
                 operation.run(self.context)
+            
+            # pages must only have revisions saved after all child objects have been updated, imported, or deleted, otherwise
+            # they will capture outdated versions of child objects in the revision
+            for operation in operation_order:
+                if isinstance(operation.instance, Page):
+                    operation.instance.save_revision()
+
 
     def _check_satisfiable(self, operation, statuses):
         # Check whether the given operation's dependencies are satisfiable. statuses is a dict of
@@ -685,10 +692,6 @@ def _save(self, context):
         # Add the page to the database as a child of parent
         parent.add_child(instance=self.instance)
 
-        if isinstance(self.instance, Page):
-            # Also save this as a revision, so that it exists in revision history
-            self.instance.save_revision(changed=False)
-
 
 class UpdateModel(SaveOperationMixin, Operation):
     def __init__(self, instance, object_data):
@@ -701,15 +704,6 @@ def run(self, context):
         self._save(context)
         self._populate_many_to_many_fields(context)
 
-    def _save(self, context):
-        super()._save(context)
-        if isinstance(self.instance, Page):
-            # Also save this as a revision, so that:
-            # * the edit-page view will pick up this imported version rather than any currently-existing drafts
-            # * it exists in revision history
-            # * the Page.draft_title field (as used in page listings in the admin) is updated to match the real title
-            self.instance.save_revision(changed=False)
-
 
 class DeleteModel(Operation):
     def __init__(self, instance):

From db2200bb4ce45da755ccc5645e999995bfaab8b0 Mon Sep 17 00:00:00 2001
From: jacobtoppm <jacobtm@torchbox.com>
Date: Wed, 14 Oct 2020 16:49:42 +0100
Subject: [PATCH 02/11] Import all references by default. Make update
 objectives replace exist objectives

---
 wagtail_transfer/operations.py | 58 +++++++++++++++++++++++++---------
 1 file changed, 43 insertions(+), 15 deletions(-)

diff --git a/wagtail_transfer/operations.py b/wagtail_transfer/operations.py
index c5494e6..a0babb8 100644
--- a/wagtail_transfer/operations.py
+++ b/wagtail_transfer/operations.py
@@ -1,4 +1,5 @@
 import json
+from copy import copy
 
 from django.conf import settings
 from django.core.exceptions import ImproperlyConfigured
@@ -206,12 +207,30 @@ def add_json(self, json_data):
         """
         data = json.loads(json_data)
 
-        # add source id -> uid mappings to the uids_by_source dict
+        # for each ID in the import list, add to base_import_ids as an object explicitly selected
+        # for import
+        for model_path, source_id in data['ids_for_import']:
+            model = get_base_model_for_path(model_path)
+            self.base_import_ids.add((model, source_id))
+
+        # add source id -> uid mappings to the uids_by_source dict, and add objectives 
+        # for importing referenced models
         for model_path, source_id, jsonish_uid in data['mappings']:
             model = get_base_model_for_path(model_path)
             uid = get_locator_for_model(model).uid_from_json(jsonish_uid)
             self.context.uids_by_source[(model, source_id)] = uid
 
+            base_import = (model, source_id) in self.base_import_ids
+
+            if base_import or model_path not in NO_FOLLOW_MODELS:
+                objective = Objective(
+                    model, source_id, self.context,
+                    must_update=(base_import or model_path in UPDATE_RELATED_MODELS)
+                )
+
+                # add to the set of objectives that need handling
+                self._add_objective(objective)
+
         # add object data to the object_data_by_source dict
         for obj_data in data['objects']:
             self._add_object_data_to_lookup(obj_data)
@@ -219,16 +238,6 @@ def add_json(self, json_data):
         # retry tasks that were previously postponed due to missing object data
         self._retry_tasks()
 
-        # for each ID in the import list, add to base_import_ids as an object explicitly selected
-        # for import, and add an objective to specify that we want an up-to-date copy of that
-        # object on the destination site
-        for model_path, source_id in data['ids_for_import']:
-            model = get_base_model_for_path(model_path)
-            self.base_import_ids.add((model, source_id))
-            objective = Objective(model, source_id, self.context, must_update=True)
-
-            # add to the set of objectives that need handling
-            self._add_objective(objective)
 
         # Process all unhandled objectives - which may trigger new objectives as dependencies of
         # the resulting operations - until no unhandled objectives remain
@@ -243,10 +252,29 @@ def _add_object_data_to_lookup(self, obj_data):
 
     def _add_objective(self, objective):
         # add to the set of objectives that need handling, unless it's one we've already seen
-        # (in which case it's either in the queue to be handled, or has been handled already)
-        if objective not in self.objectives:
-            self.objectives.add(objective)
-            self.unhandled_objectives.add(objective)
+        # (in which case it's either in the queue to be handled, or has been handled already).
+        # An objective to update a model supercedes an objective to ensure it exists
+
+        if not objective.must_update:
+            update_objective = copy(objective)
+            update_objective.must_update = True
+        else:
+            update_objective = objective
+
+        if update_objective in self.objectives:
+            # We're already updating the model, so this objective isn't relevant
+            return
+        elif objective.must_update:
+            # We're going to add a new objective to update the model
+            # so we should remove any existing objective that doesn't update the model
+            no_update_objective = copy(objective)
+            no_update_objective.must_update = False
+            self.objectives.discard(no_update_objective)
+            self.unhandled_objectives.discard(no_update_objective)
+        
+        self.objectives.add(objective)
+        self.unhandled_objectives.add(objective)
+
 
     def _handle_objective(self, objective):
         if not objective.exists_at_destination:

From a9a3aaabbad133cbbe2396346b6112c63e21b4c5 Mon Sep 17 00:00:00 2001
From: jacobtoppm <jacobtm@torchbox.com>
Date: Thu, 15 Oct 2020 10:32:04 +0100
Subject: [PATCH 03/11] Add WAGTAILTRANSFER_FOLLOWED_REVERSE_RELATIONS setting
 to optionally follow non-parental reverse relations. Remove logic performing
 certain field adapter operations only if a field was an instance of Field, as
 it led to confusing interactions with more complex relations like generic
 relations which don't have the usual inheritance: use the field adapter
 method, or the lack of a field adapter to perform this logic instead

---
 wagtail_transfer/field_adapters.py | 42 ++++++++++++++++++------------
 wagtail_transfer/operations.py     | 16 ++++++------
 wagtail_transfer/serializers.py    | 21 ++++-----------
 3 files changed, 38 insertions(+), 41 deletions(-)

diff --git a/wagtail_transfer/field_adapters.py b/wagtail_transfer/field_adapters.py
index 6d055fe..048a297 100644
--- a/wagtail_transfer/field_adapters.py
+++ b/wagtail_transfer/field_adapters.py
@@ -6,6 +6,8 @@
 from django.conf import settings
 from django.db import models
 from django.db.models.fields.reverse_related import ManyToOneRel
+from django.utils.functional import cached_property
+from modelcluster.fields import ParentalKey
 from taggit.managers import TaggableManager
 from wagtail.core import hooks
 from wagtail.core.fields import RichTextField, StreamField
@@ -20,6 +22,11 @@
 from django.utils.encoding import is_protected_type
 
 
+FOLLOWED_REVERSE_RELATIONS = [
+    (model_label.lower(), relation.lower()) for model_label, relation in getattr(settings, "WAGTAILTRANSFER_FOLLOWED_REVERSE_RELATIONS", [])
+]
+
+
 class FieldAdapter:
     def __init__(self, field):
         self.field = field
@@ -104,29 +111,34 @@ def update_object_references(self, value, destination_ids_by_source):
 class ManyToOneRelAdapter(FieldAdapter):
     def __init__(self, field):
         super().__init__(field)
-        self.related_field = field.field
+        self.related_field = getattr(field, 'field', None) or getattr(field, 'remote_field', None)
         self.related_model = field.related_model
 
-        from .serializers import get_model_serializer
-        self.related_model_serializer = get_model_serializer(self.related_model)
-
     def _get_related_objects(self, instance):
         return getattr(instance, self.name).all()
 
+    @cached_property
+    def related_model_serializer(self):
+        from .serializers import get_model_serializer
+        return get_model_serializer(self.related_model)
+
+
     def serialize(self, instance):
-        return [
-            self.related_model_serializer.serialize(obj)
-            for obj in self._get_related_objects(instance)
-        ]
+        if isinstance(self.related_field, ParentalKey):
+            return [
+                self.related_model_serializer.serialize(obj)
+                for obj in self._get_related_objects(instance)
+            ]
 
     def get_object_references(self, instance):
         refs = set()
-        for obj in self._get_related_objects(instance):
-            refs.update(self.related_model_serializer.get_object_references(obj))
+        if isinstance(self.related_field, ParentalKey) or (get_base_model(type(instance))._meta.label_lower, self.name) in FOLLOWED_REVERSE_RELATIONS:
+            for obj in self._get_related_objects(instance):
+                refs.update(self.related_model_serializer.get_object_references(obj))
         return refs
 
     def populate_field(self, instance, value, context):
-        raise Exception('populate_field is not supported on many-to-one relations')
+        pass
 
 
 class RichTextAdapter(FieldAdapter):
@@ -240,10 +252,8 @@ def populate_field(self, instance, value, context):
         pass
 
 
-class GenericRelationAdapter(FieldAdapter):
-    def populate_field(self, instance, value, context):
-        # TODO
-        pass
+class GenericRelationAdapter(ManyToOneRelAdapter):
+    pass
 
 
 class AdapterRegistry:
@@ -283,7 +293,5 @@ def get_field_adapter(self, field):
                 adapter_class = self.adapters_by_field_class[field_class]
                 return adapter_class(field)
 
-        raise ValueError("No adapter found for field: %r" % field)
-
 
 adapter_registry = AdapterRegistry()
diff --git a/wagtail_transfer/operations.py b/wagtail_transfer/operations.py
index a0babb8..eb51635 100644
--- a/wagtail_transfer/operations.py
+++ b/wagtail_transfer/operations.py
@@ -609,16 +609,15 @@ def base_model(self):
 
     def _populate_fields(self, context):
         for field in self.model._meta.get_fields():
-            if not isinstance(field, models.Field):
-                # populate data for actual fields only; ignore reverse relations
-                continue
-
             try:
                 value = self.object_data['fields'][field.name]
             except KeyError:
                 continue
 
-            adapter_registry.get_field_adapter(field).populate_field(self.instance, value, context)
+            adapter = adapter_registry.get_field_adapter(field)
+
+            if adapter:
+                adapter.populate_field(self.instance, value, context)
 
     def _populate_many_to_many_fields(self, context):
         save_needed = False
@@ -659,9 +658,10 @@ def dependencies(self):
         deps = super().dependencies
 
         for field in self.model._meta.get_fields():
-            if isinstance(field, models.Field):
-                val = self.object_data['fields'].get(field.name)
-                deps.update(adapter_registry.get_field_adapter(field).get_dependencies(val))
+            val = self.object_data['fields'].get(field.name)
+            adapter = adapter_registry.get_field_adapter(field)
+            if adapter:
+                deps.update(adapter.get_dependencies(val))
 
         return deps
 
diff --git a/wagtail_transfer/serializers.py b/wagtail_transfer/serializers.py
index e4fb60a..d64153a 100644
--- a/wagtail_transfer/serializers.py
+++ b/wagtail_transfer/serializers.py
@@ -82,25 +82,14 @@ def __init__(self, model):
             if field.name in self.ignored_fields:
                 continue
 
-            if isinstance(field, models.Field):
-                # this is a genuine field rather than a reverse relation
-
-                # ignore primary keys (including MTI parent pointers)
-                if field.primary_key:
-                    continue
-            else:
-                # this is probably a reverse relation, so fetch its related field
-                try:
-                    related_field = field.field
-                except AttributeError:
-                    # we don't know what sort of pseudo-field this is, so skip it
+            # ignore primary keys (including MTI parent pointers)
+            if getattr(field, 'primary_key', False):
                     continue
 
-                # ignore relations other than ParentalKey
-                if not isinstance(related_field, ParentalKey):
-                    continue
+            adapter = adapter_registry.get_field_adapter(field)
 
-            self.field_adapters.append(adapter_registry.get_field_adapter(field))
+            if adapter:
+                self.field_adapters.append(adapter)
 
     def get_objects_by_ids(self, ids):
         """

From 4e5bbb83bd4688a7d878c04be4caa64a15a1714d Mon Sep 17 00:00:00 2001
From: jacobtoppm <jacobtm@torchbox.com>
Date: Fri, 16 Oct 2020 11:14:09 +0100
Subject: [PATCH 04/11] Add support for GenericForeignKeys. Allow field
 adapters to specify they manage other fields, which should not have their
 field adapters used in serialization

---
 wagtail_transfer/field_adapters.py | 71 +++++++++++++++++++++++++++++-
 wagtail_transfer/serializers.py    |  8 +++-
 2 files changed, 75 insertions(+), 4 deletions(-)

diff --git a/wagtail_transfer/field_adapters.py b/wagtail_transfer/field_adapters.py
index 048a297..e8817c0 100644
--- a/wagtail_transfer/field_adapters.py
+++ b/wagtail_transfer/field_adapters.py
@@ -4,6 +4,8 @@
 from urllib.parse import urlparse
 
 from django.conf import settings
+from django.contrib.contenttypes.fields import GenericForeignKey
+from django.contrib.contenttypes.models import ContentType
 from django.db import models
 from django.db.models.fields.reverse_related import ManyToOneRel
 from django.utils.functional import cached_property
@@ -13,7 +15,7 @@
 from wagtail.core.fields import RichTextField, StreamField
 
 from .files import File, FileTransferError, get_file_hash, get_file_size
-from .models import get_base_model
+from .models import get_base_model, get_base_model_for_path
 from .richtext import get_reference_handler
 from .streamfield import get_object_references, update_object_ids
 
@@ -80,6 +82,17 @@ def populate_field(self, instance, value, context):
         value = self.update_object_references(value, context.destination_ids_by_source)
         setattr(instance, self.field.get_attname(), value)
 
+    def get_managed_fields(self):
+        """
+        Normally, a FieldAdapter will adapt a single field. However, more complex fields like
+        GenericForeignKey may 'manage' several other fields. get_managed_fields returns a list of names
+        of managed fields, whose field adapters should not be used when serializing the model. Note
+        that if a managed field also has managed fields itself, these will also be ignored when
+        serializing the model - the current field adapter is expected to address all managed fields in
+        the chain.
+        """
+        return []
+
 
 class ForeignKeyAdapter(FieldAdapter):
     def __init__(self, field):
@@ -108,6 +121,60 @@ def update_object_references(self, value, destination_ids_by_source):
         return destination_ids_by_source.get((self.related_base_model, value))
 
 
+class GenericForeignKeyAdapter(FieldAdapter):
+    def serialize(self, instance):
+        linked_instance = getattr(instance, self.field.name, None)
+        if linked_instance:
+            # here we do not use the base model, as the GFK could be pointing specifically at the child
+            # which needs to be represented accurately
+            return (linked_instance._meta.label_lower, linked_instance.pk)
+
+    def get_object_references(self, instance):
+        linked_instance = getattr(instance, self.field.name, None)
+        if linked_instance:
+            return {(get_base_model(linked_instance), linked_instance.pk)}
+        return set()
+
+    def get_dependencies(self, value):
+        if value is None:
+            return set()
+
+        model_path, model_id = value
+        base_model = get_base_model_for_path(model_path)
+
+        # GenericForeignKey itself has no blank or null properties, so we need to determine its nullable status
+        # from the underlying fields it uses
+        options = self.field.model._meta
+        ct_field = options.get_field(self.field.ct_field)
+        fk_field = options.get_field(self.field.ct_field)
+
+        if all((ct_field.blank, ct_field.null, fk_field.blank, fk_field.null)):
+            # field is nullable, so it's a soft dependency; we can leave the field empty in the
+            # case that the target object cannot be created
+            return {(base_model, model_id, False)}
+        else:
+            # this is a hard dependency
+            return {(base_model, model_id, True)}
+
+    def update_object_references(self, value, destination_ids_by_source):
+        if value:
+            model_path, model_id = value
+            base_model = get_base_model_for_path(model_path)
+            return (model_path, destination_ids_by_source.get((base_model, model_id)))
+
+    def populate_field(self, instance, value, context):
+        model_id, content_type = None, None
+        if value:
+            model_path, model_id = self.update_object_references(value, context.destination_ids_by_source)
+            content_type = ContentType.objects.get_by_natural_key(*model_path.split('.'))
+
+        setattr(instance, instance._meta.get_field(self.field.ct_field).get_attname(), content_type)
+        setattr(instance, self.field.fk_field, model_id)
+
+    def get_managed_fields(self):
+        return [self.field.fk_field, self.field.ct_field]
+
+
 class ManyToOneRelAdapter(FieldAdapter):
     def __init__(self, field):
         super().__init__(field)
@@ -122,7 +189,6 @@ def related_model_serializer(self):
         from .serializers import get_model_serializer
         return get_model_serializer(self.related_model)
 
-
     def serialize(self, instance):
         if isinstance(self.related_field, ParentalKey):
             return [
@@ -267,6 +333,7 @@ class AdapterRegistry:
         models.ManyToManyField: ManyToManyFieldAdapter,
         TaggableManager: TaggableManagerAdapter,
         GenericRelation: GenericRelationAdapter,
+        GenericForeignKey: GenericForeignKeyAdapter,
     }
 
     def __init__(self):
diff --git a/wagtail_transfer/serializers.py b/wagtail_transfer/serializers.py
index d64153a..4ecd286 100644
--- a/wagtail_transfer/serializers.py
+++ b/wagtail_transfer/serializers.py
@@ -77,7 +77,8 @@ def __init__(self, model):
         self.model = model
         self.base_model = get_base_model(model)
 
-        self.field_adapters = []
+        field_adapters = []
+        adapter_managed_fields = []
         for field in self.model._meta.get_fields():
             if field.name in self.ignored_fields:
                 continue
@@ -89,7 +90,10 @@ def __init__(self, model):
             adapter = adapter_registry.get_field_adapter(field)
 
             if adapter:
-                self.field_adapters.append(adapter)
+                adapter_managed_fields = adapter_managed_fields + adapter.get_managed_fields()
+                field_adapters.append(adapter)
+
+        self.field_adapters = [adapter for adapter in field_adapters if adapter.name not in adapter_managed_fields]
 
     def get_objects_by_ids(self, ids):
         """

From b8ea2d36452b68fa790a3699c224bbaf0ed8d3d3 Mon Sep 17 00:00:00 2001
From: jacobtoppm <jacobtm@torchbox.com>
Date: Fri, 16 Oct 2020 11:35:21 +0100
Subject: [PATCH 05/11] Add image tag default to
 WAGTAILTRANSFER_FOLLOWED_REVERSE_RELATIONS, and add the new setting to the
 documentation

---
 README.md                          |  7 +++++++
 docs/settings.md                   | 11 +++++++++++
 wagtail_transfer/field_adapters.py |  2 +-
 3 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/README.md b/README.md
index 100b6f4..4816a53 100644
--- a/README.md
+++ b/README.md
@@ -65,6 +65,13 @@ The following settings are additionally recognised:
 
   By default, objects referenced within imported content will be recursively imported to ensure that those references are still valid on the destination site. However, this is not always desirable - for example, if this happened for the Page model, this would imply that any pages linked from an imported page would get imported as well, along with any pages linked from _those_ pages, and so on, leading to an unpredictable number of extra pages being added anywhere in the page tree as a side-effect of the import. Models listed in `WAGTAILTRANSFER_NO_FOLLOW_MODELS` will thus be skipped in this process, leaving the reference unresolved. The effect this has on the referencing page will vary according to the kind of relation: nullable foreign keys, one-to-many and many-to-many relations will simply omit the missing object; references in rich text and StreamField will become broken links (just as linking a page and then deleting it would); while non-nullable foreign keys will prevent the object from being created at all (meaning that any objects referencing _that_ object will end up with unresolved references, to be handled by the same set of rules).
 
+* `WAGTAILTRANSFER_FOLLOWED_REVERSE_RELATIONS = [('wagtailimages.image', 'tagged_items')]`
+
+  Specifies a list of models and their reverse relations to follow when identifying object references that should be imported to the destination site. Defaults to `[('wagtailimages.image', 'tagged_items')]`.
+
+  By default, Wagtail Transfer will not follow reverse relations (other than importing child models of `ClusterableModel` subclasses) when identifying referenced models. Specifying a `(model, reverse_relationship_name)` in `WAGTAILTRANSFER_FOLLOWED_REVERSE_RELATIONS` means that when
+  encountering that model and relation, Wagtail Transfer will follow the reverse relationship from the specified model and add the models found to the import if they do not exist on the destination site. This is typically useful in cases such as tags on non-Page models.
+
 Note that these settings do not accept models that are defined as subclasses through [multi-table inheritance](https://docs.djangoproject.com/en/stable/topics/db/models/#multi-table-inheritance) - in particular, they cannot be used to define behaviour that only applies to specific subclasses of Page.
 
 
diff --git a/docs/settings.md b/docs/settings.md
index fe9940b..3825683 100644
--- a/docs/settings.md
+++ b/docs/settings.md
@@ -87,6 +87,17 @@ that object will end up with unresolved references, to be handled by the same se
 Note that these settings do not accept models that are defined as subclasses through multi-table inheritance - in 
 particular, they cannot be used to define behaviour that only applies to specific subclasses of Page.
 
+### `WAGTAILTRANSFER_FOLLOWED_REVERSE_RELATIONS`
+
+```python
+WAGTAILTRANSFER_FOLLOWED_REVERSE_RELATIONS = [('wagtailimages.image', 'tagged_items')]
+```
+
+Specifies a list of models and their reverse relations to follow when identifying object references that should be imported to the destination site. Defaults to `[('wagtailimages.image', 'tagged_items')]`.
+
+By default, Wagtail Transfer will not follow reverse relations (other than importing child models of `ClusterableModel` subclasses) when identifying referenced models. Specifying a `(model, reverse_relationship_name)` in `WAGTAILTRANSFER_FOLLOWED_REVERSE_RELATIONS` means that when
+encountering that model and relation, Wagtail Transfer will follow the reverse relationship from the specified model and add the models found to the import if they do not exist on the destination site. This is typically useful in cases such as tags on non-Page models.
+
 ## Hooks
 
 ### `register_field_adapters`
diff --git a/wagtail_transfer/field_adapters.py b/wagtail_transfer/field_adapters.py
index e8817c0..114ce4a 100644
--- a/wagtail_transfer/field_adapters.py
+++ b/wagtail_transfer/field_adapters.py
@@ -25,7 +25,7 @@
 
 
 FOLLOWED_REVERSE_RELATIONS = [
-    (model_label.lower(), relation.lower()) for model_label, relation in getattr(settings, "WAGTAILTRANSFER_FOLLOWED_REVERSE_RELATIONS", [])
+    (model_label.lower(), relation.lower()) for model_label, relation in getattr(settings, "WAGTAILTRANSFER_FOLLOWED_REVERSE_RELATIONS", [('wagtailimages.image', 'tagged_items')])
 ]
 
 

From 325e5d8dd5e1d6a3fc3498cdeb89302a91248f65 Mon Sep 17 00:00:00 2001
From: jacobtoppm <jacobtm@torchbox.com>
Date: Fri, 16 Oct 2020 13:27:34 +0100
Subject: [PATCH 06/11] Add reverse relation and generic foreign key tests

---
 tests/migrations/0016_advert_tags.py | 20 ++++++++++++++++
 tests/models.py                      |  2 ++
 tests/settings.py                    |  2 ++
 tests/tests/test_api.py              | 16 +++++++++++++
 tests/tests/test_import.py           | 36 ++++++++++++++++++++++++++++
 5 files changed, 76 insertions(+)
 create mode 100644 tests/migrations/0016_advert_tags.py

diff --git a/tests/migrations/0016_advert_tags.py b/tests/migrations/0016_advert_tags.py
new file mode 100644
index 0000000..b704e29
--- /dev/null
+++ b/tests/migrations/0016_advert_tags.py
@@ -0,0 +1,20 @@
+# Generated by Django 3.0.5 on 2020-10-16 11:13
+
+from django.db import migrations
+import taggit.managers
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('taggit', '0003_taggeditem_add_unique_index'),
+        ('tests', '0015_longadvert'),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name='advert',
+            name='tags',
+            field=taggit.managers.TaggableManager(help_text='A comma-separated list of tags.', through='taggit.TaggedItem', to='taggit.Tag', verbose_name='Tags'),
+        ),
+    ]
diff --git a/tests/models.py b/tests/models.py
index 71333d7..be6ae5d 100644
--- a/tests/models.py
+++ b/tests/models.py
@@ -1,5 +1,6 @@
 from django.db import models
 from modelcluster.fields import ParentalKey, ParentalManyToManyField
+from taggit.managers import TaggableManager
 from wagtail.core.fields import RichTextField, StreamField
 from wagtail.core.models import Orderable, Page
 from wagtail.snippets.models import register_snippet
@@ -13,6 +14,7 @@ class SimplePage(Page):
 
 class Advert(models.Model):
     slogan = models.CharField(max_length=255)
+    tags = TaggableManager()
 
 
 class LongAdvert(Advert):
diff --git a/tests/settings.py b/tests/settings.py
index 9f0e682..98bd436 100644
--- a/tests/settings.py
+++ b/tests/settings.py
@@ -134,6 +134,8 @@
     }
 }
 
+WAGTAILTRANSFER_FOLLOWED_REVERSE_RELATIONS = [('wagtailimages.image', 'tagged_items'), ('tests.advert', 'tagged_items')]
+
 WAGTAILTRANSFER_SECRET_KEY = 'i-am-the-local-secret-key'
 
 WAGTAILTRANSFER_UPDATE_RELATED_MODELS = ['wagtailimages.Image', 'tests.advert']
diff --git a/tests/tests/test_api.py b/tests/tests/test_api.py
index 73f45f9..3f533b5 100644
--- a/tests/tests/test_api.py
+++ b/tests/tests/test_api.py
@@ -529,6 +529,22 @@ def test_model_with_multi_table_inheritance(self):
         self.assertEqual(data['objects'][0]['model'], 'tests.longadvert')
         # the child object should be serialized
 
+    def test_model_with_tags(self):
+        # test that a reverse relation such as tagged_items is followed to obtain references to the tags
+        # and tagged_items, if the model and relationship are specified in WAGTAILTRANSFER_FOLLOWED_REVERSE_RELATIONS
+        ad = Advert.objects.create(slogan='test')
+        ad.tags.add('test_tag')
+
+        response = self.get({
+            'tests.advert': [ad.pk]
+        })
+        self.assertEqual(response.status_code, 200)
+        data = json.loads(response.content)
+
+        mapped_models = {mapping[0] for mapping in data['mappings']}
+        self.assertIn('taggit.tag', mapped_models)
+        self.assertIn('taggit.taggeditem', mapped_models)
+
     def test_image(self):
         with open(os.path.join(FIXTURES_DIR, 'wagtail.jpg'), 'rb') as f:
             image = Image.objects.create(
diff --git a/tests/tests/test_import.py b/tests/tests/test_import.py
index 25a4edd..8965d88 100644
--- a/tests/tests/test_import.py
+++ b/tests/tests/test_import.py
@@ -1415,3 +1415,39 @@ def test_import_multi_table_model(self):
         self.assertIsNotNone(imported_ad)
         self.assertEqual(imported_ad.slogan, "test")
         self.assertEqual(imported_ad.description, "longertest")
+
+    def test_import_model_with_generic_foreign_key(self):
+        # test importing a model with a generic foreign key by importing a model that implements tagging using standard taggit (not ParentalKey)
+        data = """{
+            "ids_for_import": [["tests.advert", 4]],
+            "mappings": [
+                ["taggit.tag", 152, "ac92b2ba-0fa6-11eb-800b-287fcf66f689"],
+                ["tests.advert", 4, "ac931726-0fa6-11eb-800c-287fcf66f689"],
+                ["taggit.taggeditem", 150, "ac938e5a-0fa6-11eb-800d-287fcf66f689"]
+            ],
+            "objects": [
+                {
+                    "model": "tests.advert",
+                    "pk": 4,
+                    "fields": {"longadvert": null, "sponsoredpage": null, "slogan": "test", "tags": "[<Tag: test_tag>]", "tagged_items": null}
+                },
+                {
+                    "model": "taggit.taggeditem",
+                    "pk": 150,
+                    "fields": {"content_object": ["tests.advert", 4], "tag": 152}
+                },
+                {
+                    "model": "taggit.tag",
+                    "pk": 152,
+                    "fields": {"name": "test_tag", "slug": "testtag"}
+                }
+            ]
+        }"""
+
+        importer = ImportPlanner(root_page_source_pk=1, destination_parent_id=None)
+        importer.add_json(data)
+        importer.run()
+
+        imported_ad = Advert.objects.filter(id=4).first()
+        self.assertIsNotNone(imported_ad)
+        self.assertEqual(imported_ad.tags.first().name, "test_tag")

From 6bef7f10755147ab5b11172b70eadfcd55af8809 Mon Sep 17 00:00:00 2001
From: jacobtoppm <jacobtm@torchbox.com>
Date: Fri, 16 Oct 2020 13:28:30 +0100
Subject: [PATCH 07/11] fixup! Add support for GenericForeignKeys. Allow field
 adapters to specify they manage other fields, which should not have their
 field adapters used in serialization

---
 wagtail_transfer/field_adapters.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/wagtail_transfer/field_adapters.py b/wagtail_transfer/field_adapters.py
index 114ce4a..79d0fef 100644
--- a/wagtail_transfer/field_adapters.py
+++ b/wagtail_transfer/field_adapters.py
@@ -168,7 +168,7 @@ def populate_field(self, instance, value, context):
             model_path, model_id = self.update_object_references(value, context.destination_ids_by_source)
             content_type = ContentType.objects.get_by_natural_key(*model_path.split('.'))
 
-        setattr(instance, instance._meta.get_field(self.field.ct_field).get_attname(), content_type)
+        setattr(instance, instance._meta.get_field(self.field.ct_field).get_attname(), content_type.pk)
         setattr(instance, self.field.fk_field, model_id)
 
     def get_managed_fields(self):

From 6fb8f0457f6be02a746780afdfea38b6601e7a44 Mon Sep 17 00:00:00 2001
From: jacobtoppm <jacobtm@torchbox.com>
Date: Fri, 16 Oct 2020 17:42:28 +0100
Subject: [PATCH 08/11] Don't store child object data on the parent model, so
 references processed before the parent page won't identify the child object
 data as missing and re-import it. Instead, let the field adapter specify
 objects that should be serialized during initial export.

---
 tests/tests/test_api.py            | 15 +++---
 tests/tests/test_import.py         | 86 +++++++++++++++---------------
 wagtail_transfer/field_adapters.py | 38 +++++++------
 wagtail_transfer/operations.py     |  9 ++--
 wagtail_transfer/serializers.py    |  7 +++
 wagtail_transfer/views.py          | 39 +++++++++-----
 6 files changed, 110 insertions(+), 84 deletions(-)

diff --git a/tests/tests/test_api.py b/tests/tests/test_api.py
index 3f533b5..b39c3bf 100644
--- a/tests/tests/test_api.py
+++ b/tests/tests/test_api.py
@@ -195,15 +195,17 @@ def test_parental_keys(self):
         data = json.loads(response.content)
 
         page_data = None
+        section_data = []
         for obj in data['objects']:
             if obj['model'] == 'tests.sectionedpage' and obj['pk'] == page.pk:
                 page_data = obj
-                break
+            if obj['model'] == 'tests.sectionedpagesection':
+                section_data.append(obj)
 
         self.assertEqual(len(page_data['fields']['sections']), 2)
-        self.assertEqual(page_data['fields']['sections'][0]['model'], 'tests.sectionedpagesection')
-        self.assertEqual(page_data['fields']['sections'][0]['fields']['title'], "Create the universe")
-        section_id = page_data['fields']['sections'][0]['pk']
+        self.assertEqual(section_data[0]['model'], 'tests.sectionedpagesection')
+        self.assertTrue(section_data[0]['fields']['title'] == "Create the universe")
+        section_id = page_data['fields']['sections'][0]
 
         # there should also be a uid mapping for the section
         matching_uids = [
@@ -530,8 +532,8 @@ def test_model_with_multi_table_inheritance(self):
         # the child object should be serialized
 
     def test_model_with_tags(self):
-        # test that a reverse relation such as tagged_items is followed to obtain references to the tags
-        # and tagged_items, if the model and relationship are specified in WAGTAILTRANSFER_FOLLOWED_REVERSE_RELATIONS
+        # test that a reverse relation such as tagged_items is followed to obtain references to the
+        # tagged_items, if the model and relationship are specified in WAGTAILTRANSFER_FOLLOWED_REVERSE_RELATIONS
         ad = Advert.objects.create(slogan='test')
         ad.tags.add('test_tag')
 
@@ -542,7 +544,6 @@ def test_model_with_tags(self):
         data = json.loads(response.content)
 
         mapped_models = {mapping[0] for mapping in data['mappings']}
-        self.assertIn('taggit.tag', mapped_models)
         self.assertIn('taggit.taggeditem', mapped_models)
 
     def test_image(self):
diff --git a/tests/tests/test_import.py b/tests/tests/test_import.py
index 8965d88..8a14f9d 100644
--- a/tests/tests/test_import.py
+++ b/tests/tests/test_import.py
@@ -307,28 +307,27 @@ def test_import_page_with_child_models(self):
                         "live": true,
                         "slug": "how-to-boil-an-egg",
                         "intro": "This is how to boil an egg",
-                        "sections": [
-                            {
-                                "model": "tests.sectionedpagesection",
-                                "pk": 101,
-                                "fields": {
-                                    "sort_order": 0,
-                                    "title": "Boil the outside of the egg",
-                                    "body": "...",
-                                    "page": 100
-                                }
-                            },
-                            {
-                                "model": "tests.sectionedpagesection",
-                                "pk": 102,
-                                "fields": {
-                                    "sort_order": 1,
-                                    "title": "Boil the rest of the egg",
-                                    "body": "...",
-                                    "page": 100
-                                }
-                            }
-                        ]
+                        "sections": [101, 102]
+                    }
+                },
+                {
+                    "model": "tests.sectionedpagesection",
+                    "pk": 101,
+                    "fields": {
+                        "sort_order": 0,
+                        "title": "Boil the outside of the egg",
+                        "body": "...",
+                        "page": 100
+                    }
+                },
+                {
+                    "model": "tests.sectionedpagesection",
+                    "pk": 102,
+                    "fields": {
+                        "sort_order": 1,
+                        "title": "Boil the rest of the egg",
+                        "body": "...",
+                        "page": 100
                     }
                 }
             ]
@@ -370,28 +369,27 @@ def test_import_page_with_child_models(self):
                         "live": true,
                         "slug": "how-to-boil-an-egg",
                         "intro": "This is still how to boil an egg",
-                        "sections": [
-                            {
-                                "model": "tests.sectionedpagesection",
-                                "pk": 102,
-                                "fields": {
-                                    "sort_order": 0,
-                                    "title": "Boil the egg",
-                                    "body": "...",
-                                    "page": 100
-                                }
-                            },
-                            {
-                                "model": "tests.sectionedpagesection",
-                                "pk": 103,
-                                "fields": {
-                                    "sort_order": 1,
-                                    "title": "Eat the egg",
-                                    "body": "...",
-                                    "page": 100
-                                }
-                            }
-                        ]
+                        "sections": [102, 103]
+                    }
+                },
+                {
+                    "model": "tests.sectionedpagesection",
+                    "pk": 102,
+                    "fields": {
+                        "sort_order": 0,
+                        "title": "Boil the egg",
+                        "body": "...",
+                        "page": 100
+                    }
+                },
+                {
+                    "model": "tests.sectionedpagesection",
+                    "pk": 103,
+                    "fields": {
+                        "sort_order": 1,
+                        "title": "Eat the egg",
+                        "body": "...",
+                        "page": 100
                     }
                 }
             ]
diff --git a/wagtail_transfer/field_adapters.py b/wagtail_transfer/field_adapters.py
index 79d0fef..875fa93 100644
--- a/wagtail_transfer/field_adapters.py
+++ b/wagtail_transfer/field_adapters.py
@@ -93,6 +93,14 @@ def get_managed_fields(self):
         """
         return []
 
+    def get_objects_to_serialize(self, instance):
+        """
+        Return a set of (model_class, id) pairs for objects that should be serialized on export, before
+        it is known whether or not they exist or should be updated at the destination site
+        """
+        return set()
+
+
 
 class ForeignKeyAdapter(FieldAdapter):
     def __init__(self, field):
@@ -179,29 +187,27 @@ class ManyToOneRelAdapter(FieldAdapter):
     def __init__(self, field):
         super().__init__(field)
         self.related_field = getattr(field, 'field', None) or getattr(field, 'remote_field', None)
-        self.related_model = field.related_model
-
-    def _get_related_objects(self, instance):
-        return getattr(instance, self.name).all()
+        self.related_base_model = get_base_model(field.related_model)
+        self.is_parental = isinstance(self.related_field, ParentalKey)
 
-    @cached_property
-    def related_model_serializer(self):
-        from .serializers import get_model_serializer
-        return get_model_serializer(self.related_model)
+    def _get_related_object_pks(self, instance):
+        return getattr(instance, self.name).all().values_list('pk', flat=True)
 
     def serialize(self, instance):
-        if isinstance(self.related_field, ParentalKey):
-            return [
-                self.related_model_serializer.serialize(obj)
-                for obj in self._get_related_objects(instance)
-            ]
+        if self.is_parental:
+            return list(self._get_related_object_pks(instance))
 
     def get_object_references(self, instance):
         refs = set()
-        if isinstance(self.related_field, ParentalKey) or (get_base_model(type(instance))._meta.label_lower, self.name) in FOLLOWED_REVERSE_RELATIONS:
-            for obj in self._get_related_objects(instance):
-                refs.update(self.related_model_serializer.get_object_references(obj))
+        if self.is_parental or (get_base_model(self.field.model)._meta.label_lower, self.name) in FOLLOWED_REVERSE_RELATIONS:
+            for pk in self._get_related_object_pks(instance):
+                refs.add((self.related_base_model, pk))
         return refs
+    
+    def get_objects_to_serialize(self, instance):
+        if self.is_parental:
+            return getattr(instance, self.name).all()
+        return set()
 
     def populate_field(self, instance, value, context):
         pass
diff --git a/wagtail_transfer/operations.py b/wagtail_transfer/operations.py
index eb51635..85c8417 100644
--- a/wagtail_transfer/operations.py
+++ b/wagtail_transfer/operations.py
@@ -383,19 +383,18 @@ def _handle_task(self, task):
                 related_base_model = get_base_model(rel.related_model)
                 child_uids = set()
 
-                for child_obj_data in object_data['fields'][rel.name]:
-                    # Add child object data to the object_data_by_source lookup
-                    self._add_object_data_to_lookup(child_obj_data)
+                for child_obj_pk in object_data['fields'][rel.name]:
 
                     # Add an objective for handling the child object. Regardless of whether
                     # this is a 'create' or 'update' task, we want the child objects to be at
                     # their most up-to-date versions, so set the objective to 'must update'
+
                     self._add_objective(
-                        Objective(related_base_model, child_obj_data['pk'], self.context, must_update=True)
+                        Objective(related_base_model, child_obj_pk, self.context, must_update=True)
                     )
 
                     # look up the child object's UID
-                    uid = self.context.uids_by_source[(related_base_model, child_obj_data['pk'])]
+                    uid = self.context.uids_by_source[(related_base_model, child_obj_pk)]
                     child_uids.add(uid)
 
                 if action == 'update':
diff --git a/wagtail_transfer/serializers.py b/wagtail_transfer/serializers.py
index 4ecd286..bac37fb 100644
--- a/wagtail_transfer/serializers.py
+++ b/wagtail_transfer/serializers.py
@@ -95,6 +95,7 @@ def __init__(self, model):
 
         self.field_adapters = [adapter for adapter in field_adapters if adapter.name not in adapter_managed_fields]
 
+
     def get_objects_by_ids(self, ids):
         """
         Given a list of IDs, return a list of model instances that we can
@@ -127,6 +128,12 @@ def get_object_references(self, instance):
             refs.update(f.get_object_references(instance))
         return refs
 
+    def get_objects_to_serialize(self, instance):
+        objects = set()
+        for f in self.field_adapters:
+            objects.update(f.get_objects_to_serialize(instance))
+        return objects
+
 
 class TreeModelSerializer(ModelSerializer):
     ignored_fields = ['path', 'depth', 'numchild']
diff --git a/wagtail_transfer/views.py b/wagtail_transfer/views.py
index 8b1ae44..db38f84 100644
--- a/wagtail_transfer/views.py
+++ b/wagtail_transfer/views.py
@@ -38,10 +38,15 @@ def pages_for_export(request, root_page_id):
     objects = []
     object_references = set()
 
-    for page in pages:
-        serializer = get_model_serializer(type(page))
-        objects.append(serializer.serialize(page))
-        object_references.update(serializer.get_object_references(page))
+    models_to_serialize = set(pages)
+    serialized_models = set()
+
+    while models_to_serialize:
+        model = models_to_serialize.pop()
+        serializer = get_model_serializer(type(model))
+        objects.append(serializer.serialize(model))
+        object_references.update(serializer.get_object_references(model))
+        models_to_serialize.update(serializer.get_objects_to_serialize(model).difference(serialized_models))
 
     mappings = []
     for model, pk in object_references:
@@ -82,10 +87,15 @@ def models_for_export(request, model_path, object_id=None):
     objects = []
     object_references = set()
 
-    for model_object in model_objects:
-        serializer = get_model_serializer(type(model_object))
-        objects.append(serializer.serialize(model_object))
-        object_references.update(serializer.get_object_references(model_object))
+    models_to_serialize = set(model_objects)
+    serialized_models = set()
+
+    while models_to_serialize:
+        model = models_to_serialize.pop()
+        serializer = get_model_serializer(type(model))
+        objects.append(serializer.serialize(model))
+        object_references.update(serializer.get_object_references(model))
+        models_to_serialize.update(serializer.get_objects_to_serialize(model).difference(serialized_models))
 
     mappings = []
     for model, pk in object_references:
@@ -119,15 +129,20 @@ def objects_for_export(request):
 
     objects = []
     object_references = set()
+    serialized_models = set()
+    models_to_serialize = set()
 
     for model_path, ids in request_data.items():
         model = get_model_for_path(model_path)
         serializer = get_model_serializer(model)
 
-        for obj in serializer.get_objects_by_ids(ids):
-            instance_serializer = get_model_serializer(type(obj))  # noqa
-            objects.append(instance_serializer.serialize(obj))
-            object_references.update(instance_serializer.get_object_references(obj))
+        models_to_serialize.update(serializer.get_objects_by_ids(ids))
+        while models_to_serialize:
+            instance = models_to_serialize.pop()
+            serializer = get_model_serializer(type(instance))
+            objects.append(serializer.serialize(instance))
+            object_references.update(serializer.get_object_references(instance))
+            models_to_serialize.update(serializer.get_objects_to_serialize(instance).difference(serialized_models))
 
     mappings = []
     for model, pk in object_references:

From 95e42cf6d467768d223987de777a1602798f911e Mon Sep 17 00:00:00 2001
From: jacobtoppm <jacobtm@torchbox.com>
Date: Mon, 19 Oct 2020 12:14:05 +0100
Subject: [PATCH 09/11] Allow reverse relations to specify deletions of
 'child-like' models as a new field adapter method, primarily useful for tags
 on non Page models. This behaviour is set using
 WAGTAILTRANSFER_FOLLOWED_REVERSE_RELATIONS, which now takes an additional
 value per tuple - a Boolean which sets whether or not referenced objects on
 the destination and not source sites should be deleted

---
 tests/settings.py                  |  2 +-
 tests/tests/test_import.py         |  8 ++---
 wagtail_transfer/field_adapters.py | 47 +++++++++++++++++++++++-------
 wagtail_transfer/operations.py     | 37 +++++++++++------------
 4 files changed, 61 insertions(+), 33 deletions(-)

diff --git a/tests/settings.py b/tests/settings.py
index 98bd436..c2c24a5 100644
--- a/tests/settings.py
+++ b/tests/settings.py
@@ -134,7 +134,7 @@
     }
 }
 
-WAGTAILTRANSFER_FOLLOWED_REVERSE_RELATIONS = [('wagtailimages.image', 'tagged_items'), ('tests.advert', 'tagged_items')]
+WAGTAILTRANSFER_FOLLOWED_REVERSE_RELATIONS = [('wagtailimages.image', 'tagged_items', True), ('tests.advert', 'tagged_items', True)]
 
 WAGTAILTRANSFER_SECRET_KEY = 'i-am-the-local-secret-key'
 
diff --git a/tests/tests/test_import.py b/tests/tests/test_import.py
index 8a14f9d..20a9218 100644
--- a/tests/tests/test_import.py
+++ b/tests/tests/test_import.py
@@ -620,7 +620,7 @@ def test_import_image_with_file(self, get):
                         "file_size": 18521,
                         "file_hash": "e4eab12cc50b6b9c619c9ddd20b61d8e6a961ada",
                         "tags": "[]",
-                        "tagged_items": "[]"
+                        "tagged_items": []
                     }
                 }
             ]
@@ -684,7 +684,7 @@ def test_import_image_with_file_without_root_collection_mapping(self, get):
                         "file_size": 18521,
                         "file_hash": "e4eab12cc50b6b9c619c9ddd20b61d8e6a961ada",
                         "tags": "[]",
-                        "tagged_items": "[]"
+                        "tagged_items": []
                     }
                 }
             ]
@@ -771,7 +771,7 @@ def test_existing_image_is_not_refetched(self, get):
                         "file_size": 1160,
                         "file_hash": "45c5db99aea04378498883b008ee07528f5ae416",
                         "tags": "[]",
-                        "tagged_items": "[]"
+                        "tagged_items": []
                     }
                 }
             ]
@@ -851,7 +851,7 @@ def test_replace_image(self, get):
                         "file_size": 27,
                         "file_hash": "e4eab12cc50b6b9c619c9ddd20b61d8e6a961ada",
                         "tags": "[]",
-                        "tagged_items": "[]"
+                        "tagged_items": []
                     }
                 }
             ]
diff --git a/wagtail_transfer/field_adapters.py b/wagtail_transfer/field_adapters.py
index 875fa93..6f014a3 100644
--- a/wagtail_transfer/field_adapters.py
+++ b/wagtail_transfer/field_adapters.py
@@ -15,6 +15,7 @@
 from wagtail.core.fields import RichTextField, StreamField
 
 from .files import File, FileTransferError, get_file_hash, get_file_size
+from .locators import get_locator_for_model
 from .models import get_base_model, get_base_model_for_path
 from .richtext import get_reference_handler
 from .streamfield import get_object_references, update_object_ids
@@ -23,10 +24,13 @@
 
 from django.utils.encoding import is_protected_type
 
-
-FOLLOWED_REVERSE_RELATIONS = [
-    (model_label.lower(), relation.lower()) for model_label, relation in getattr(settings, "WAGTAILTRANSFER_FOLLOWED_REVERSE_RELATIONS", [('wagtailimages.image', 'tagged_items')])
-]
+WAGTAILTRANSFER_FOLLOWED_REVERSE_RELATIONS = getattr(settings, "WAGTAILTRANSFER_FOLLOWED_REVERSE_RELATIONS", [('wagtailimages.image', 'tagged_items', True)])
+FOLLOWED_REVERSE_RELATIONS = {
+    (model_label.lower(), relation.lower()) for model_label, relation, _ in WAGTAILTRANSFER_FOLLOWED_REVERSE_RELATIONS
+}
+DELETED_REVERSE_RELATIONS = {
+    (model_label.lower(), relation.lower()) for model_label, relation, track_deletions in WAGTAILTRANSFER_FOLLOWED_REVERSE_RELATIONS if track_deletions
+}
 
 
 class FieldAdapter:
@@ -66,6 +70,12 @@ def get_dependencies(self, value):
         """
         return set()
 
+    def get_object_deletions(self, instance, value, context):
+        """
+        A set of (base_model_class, id) tuples for objects that must be deleted at the destination site
+        """
+        return set()
+
     def update_object_references(self, value, destination_ids_by_source):
         """
         Return a modified version of value with object references replaced by their corresponding
@@ -189,20 +199,37 @@ def __init__(self, field):
         self.related_field = getattr(field, 'field', None) or getattr(field, 'remote_field', None)
         self.related_base_model = get_base_model(field.related_model)
         self.is_parental = isinstance(self.related_field, ParentalKey)
+        self.is_followed = (get_base_model(self.field.model)._meta.label_lower, self.name) in FOLLOWED_REVERSE_RELATIONS
 
-    def _get_related_object_pks(self, instance):
-        return getattr(instance, self.name).all().values_list('pk', flat=True)
+    def _get_related_objects(self, instance):
+        return getattr(instance, self.name).all()
 
     def serialize(self, instance):
-        if self.is_parental:
-            return list(self._get_related_object_pks(instance))
+        if self.is_parental or self.is_followed:
+            return list(self._get_related_objects(instance).values_list('pk', flat=True))
 
     def get_object_references(self, instance):
         refs = set()
-        if self.is_parental or (get_base_model(self.field.model)._meta.label_lower, self.name) in FOLLOWED_REVERSE_RELATIONS:
-            for pk in self._get_related_object_pks(instance):
+        if self.is_parental or self.is_followed:
+            for pk in self._get_related_objects(instance).values_list('pk', flat=True):
                 refs.add((self.related_base_model, pk))
         return refs
+
+    def get_object_deletions(self, instance, value, context):
+        if (self.is_parental or (get_base_model(self.field.model)._meta.label_lower, self.name) in DELETED_REVERSE_RELATIONS):
+            value = value or []
+            uids = {context.uids_by_source[(self.related_base_model, pk)] for pk in value}
+            # delete any related objects on the existing object if they can't be mapped back
+            # to one of the uids in the new set
+            locator = get_locator_for_model(self.related_base_model)
+            matched_destination_ids = set()
+            for uid in uids:
+                child = locator.find(uid)
+                if child is not None:
+                    matched_destination_ids.add(child.pk)
+
+            return {child for child in self._get_related_objects(instance) if child.pk not in matched_destination_ids}
+        return set()
     
     def get_objects_to_serialize(self, instance):
         if self.is_parental:
diff --git a/wagtail_transfer/operations.py b/wagtail_transfer/operations.py
index 85c8417..058d0bd 100644
--- a/wagtail_transfer/operations.py
+++ b/wagtail_transfer/operations.py
@@ -393,24 +393,6 @@ def _handle_task(self, task):
                         Objective(related_base_model, child_obj_pk, self.context, must_update=True)
                     )
 
-                    # look up the child object's UID
-                    uid = self.context.uids_by_source[(related_base_model, child_obj_pk)]
-                    child_uids.add(uid)
-
-                if action == 'update':
-                    # delete any child objects on the existing object if they can't be mapped back
-                    # to one of the uids in the new set
-                    locator = get_locator_for_model(related_base_model)
-                    matched_destination_ids = set()
-                    for uid in child_uids:
-                        child = locator.find(uid)
-                        if child is not None:
-                            matched_destination_ids.add(child.pk)
-
-                    for child in getattr(obj, rel.name).all():
-                        if child.pk not in matched_destination_ids:
-                            self.operations.add(DeleteModel(child))
-
         if operation is not None:
             self.operations.add(operation)
 
@@ -437,6 +419,9 @@ def _handle_task(self, task):
                     Objective(model, source_id, self.context, must_update=(model._meta.label_lower in UPDATE_RELATED_MODELS))
                 )
 
+            for instance in operation.deletions(self.context):
+                self.operations.add(DeleteModel(instance))
+
     def _retry_tasks(self):
         """
         Retry tasks that were previously postponed due to missing object data
@@ -590,6 +575,10 @@ def dependencies(self):
         """
         return set()
 
+    def deletions(self, context):
+        # the set of objects that must be deleted when we import this object
+        return set()
+
 
 class SaveOperationMixin:
     """
@@ -664,6 +653,18 @@ def dependencies(self):
 
         return deps
 
+    def deletions(self, context):
+        # the set of objects that must be deleted when we import this object
+
+        deletions = super().deletions(context)
+        for field in self.model._meta.get_fields():
+            val = self.object_data['fields'].get(field.name)
+            adapter = adapter_registry.get_field_adapter(field)
+            if adapter:
+                deletions.update(adapter.get_object_deletions(self.instance, val, context))
+
+        return deletions
+
 
 class CreateModel(SaveOperationMixin, Operation):
     def __init__(self, model, object_data):

From 727f8315c1c498b2deb26824d9ddd026342dfc8b Mon Sep 17 00:00:00 2001
From: jacobtoppm <jacobtm@torchbox.com>
Date: Mon, 19 Oct 2020 12:22:02 +0100
Subject: [PATCH 10/11] Add new behaviour (deletion tracking) of
 WAGTAILTRANSFER_FOLLOWED_REVERSE_RELATIONS to settings reference

---
 README.md        |  8 +++++---
 docs/settings.md | 10 ++++++----
 2 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/README.md b/README.md
index 4816a53..9cc5038 100644
--- a/README.md
+++ b/README.md
@@ -67,10 +67,12 @@ The following settings are additionally recognised:
 
 * `WAGTAILTRANSFER_FOLLOWED_REVERSE_RELATIONS = [('wagtailimages.image', 'tagged_items')]`
 
-  Specifies a list of models and their reverse relations to follow when identifying object references that should be imported to the destination site. Defaults to `[('wagtailimages.image', 'tagged_items')]`.
+  Specifies a list of models, their reverse relations to follow, and whether deletions should be synced, when identifying object references that should be imported to the destination site. Defaults to `[('wagtailimages.image', 'tagged_items', True)]`.
 
-  By default, Wagtail Transfer will not follow reverse relations (other than importing child models of `ClusterableModel` subclasses) when identifying referenced models. Specifying a `(model, reverse_relationship_name)` in `WAGTAILTRANSFER_FOLLOWED_REVERSE_RELATIONS` means that when
-  encountering that model and relation, Wagtail Transfer will follow the reverse relationship from the specified model and add the models found to the import if they do not exist on the destination site. This is typically useful in cases such as tags on non-Page models.
+  By default, Wagtail Transfer will not follow reverse relations (other than importing child models of `ClusterableModel` subclasses) when identifying referenced models. Specifying a `(model, reverse_relationship_name, track_deletions)` in `WAGTAILTRANSFER_FOLLOWED_REVERSE_RELATIONS` means that when
+  encountering that model and relation, Wagtail Transfer will follow the reverse relationship from the specified model and add the models found to the import if they do not exist on the destination site. This is typically useful in cases such as tags on non-Page models. The `track_deletions` boolean,
+  if `True`, will delete any models in the reverse relation on the destination site that do not exist in the source site's reverse relation. As a result,
+  it should only be used for models that behave strictly like child models but do not use `ParentalKey` - for example, tags, where importing an image with deleted tags should delete those tag linking models on the destination site as well.
 
 Note that these settings do not accept models that are defined as subclasses through [multi-table inheritance](https://docs.djangoproject.com/en/stable/topics/db/models/#multi-table-inheritance) - in particular, they cannot be used to define behaviour that only applies to specific subclasses of Page.
 
diff --git a/docs/settings.md b/docs/settings.md
index 3825683..af66ee1 100644
--- a/docs/settings.md
+++ b/docs/settings.md
@@ -90,13 +90,15 @@ particular, they cannot be used to define behaviour that only applies to specifi
 ### `WAGTAILTRANSFER_FOLLOWED_REVERSE_RELATIONS`
 
 ```python
-WAGTAILTRANSFER_FOLLOWED_REVERSE_RELATIONS = [('wagtailimages.image', 'tagged_items')]
+WAGTAILTRANSFER_FOLLOWED_REVERSE_RELATIONS = [('wagtailimages.image', 'tagged_items', True)]
 ```
 
-Specifies a list of models and their reverse relations to follow when identifying object references that should be imported to the destination site. Defaults to `[('wagtailimages.image', 'tagged_items')]`.
+Specifies a list of models, their reverse relations to follow, and whether deletions should be synced, when identifying object references that should be imported to the destination site. Defaults to `[('wagtailimages.image', 'tagged_items', True)]`.
 
-By default, Wagtail Transfer will not follow reverse relations (other than importing child models of `ClusterableModel` subclasses) when identifying referenced models. Specifying a `(model, reverse_relationship_name)` in `WAGTAILTRANSFER_FOLLOWED_REVERSE_RELATIONS` means that when
-encountering that model and relation, Wagtail Transfer will follow the reverse relationship from the specified model and add the models found to the import if they do not exist on the destination site. This is typically useful in cases such as tags on non-Page models.
+By default, Wagtail Transfer will not follow reverse relations (other than importing child models of `ClusterableModel` subclasses) when identifying referenced models. Specifying a `(model, reverse_relationship_name, track_deletions)` in `WAGTAILTRANSFER_FOLLOWED_REVERSE_RELATIONS` means that when
+encountering that model and relation, Wagtail Transfer will follow the reverse relationship from the specified model and add the models found to the import if they do not exist on the destination site. This is typically useful in cases such as tags on non-Page models. The `track_deletions` boolean,
+if `True`, will delete any models in the reverse relation on the destination site that do not exist in the source site's reverse relation. As a result,
+it should only be used for models that behave strictly like child models but do not use `ParentalKey` - for example, tags, where importing an image with deleted tags should delete those tag linking models on the destination site as well.
 
 ## Hooks
 

From f438cad1bf3701c10443b82b8b861e25a91b076d Mon Sep 17 00:00:00 2001
From: jacobtoppm <jacobtm@torchbox.com>
Date: Mon, 19 Oct 2020 12:57:32 +0100
Subject: [PATCH 11/11] Add tests for tag deletion tracking

---
 tests/tests/test_import.py | 125 ++++++++++++++++++++++++++++++++++++-
 1 file changed, 124 insertions(+), 1 deletion(-)

diff --git a/tests/tests/test_import.py b/tests/tests/test_import.py
index 20a9218..f5ed40d 100644
--- a/tests/tests/test_import.py
+++ b/tests/tests/test_import.py
@@ -1,3 +1,4 @@
+import importlib
 import os.path
 import shutil
 from unittest import mock
@@ -5,7 +6,7 @@
 from django.conf import settings
 from django.contrib.contenttypes.models import ContentType
 from django.core.files.images import ImageFile
-from django.test import TestCase
+from django.test import TestCase, override_settings
 from wagtail.core.models import Collection, Page
 from wagtail.images.models import Image
 
@@ -1449,3 +1450,125 @@ def test_import_model_with_generic_foreign_key(self):
         imported_ad = Advert.objects.filter(id=4).first()
         self.assertIsNotNone(imported_ad)
         self.assertEqual(imported_ad.tags.first().name, "test_tag")
+
+    def test_import_model_with_deleted_reverse_related_models(self):
+        # test re-importing a model where WAGTAILTRANSFER_FOLLOWED_REVERSE_RELATIONS is used to track tag deletions
+        # will delete tags correctly
+        data = """{
+            "ids_for_import": [["tests.advert", 4]],
+            "mappings": [
+                ["taggit.tag", 152, "ac92b2ba-0fa6-11eb-800b-287fcf66f689"],
+                ["tests.advert", 4, "ac931726-0fa6-11eb-800c-287fcf66f689"],
+                ["taggit.taggeditem", 150, "ac938e5a-0fa6-11eb-800d-287fcf66f689"]
+            ],
+            "objects": [
+                {
+                    "model": "tests.advert",
+                    "pk": 4,
+                    "fields": {"longadvert": null, "sponsoredpage": null, "slogan": "test", "tags": "[<Tag: test_tag>]", "tagged_items": [150]}
+                },
+                {
+                    "model": "taggit.taggeditem",
+                    "pk": 150,
+                    "fields": {"content_object": ["tests.advert", 4], "tag": 152}
+                },
+                {
+                    "model": "taggit.tag",
+                    "pk": 152,
+                    "fields": {"name": "test_tag", "slug": "testtag"}
+                }
+            ]
+        }"""
+
+        importer = ImportPlanner(root_page_source_pk=1, destination_parent_id=None)
+        importer.add_json(data)
+        importer.run()
+
+        imported_ad = Advert.objects.filter(id=4).first()
+        self.assertIsNotNone(imported_ad)
+        self.assertEqual(imported_ad.tags.first().name, "test_tag")
+
+        data = """{
+            "ids_for_import": [["tests.advert", 4]],
+            "mappings": [
+                ["tests.advert", 4, "ac931726-0fa6-11eb-800c-287fcf66f689"]
+            ],
+            "objects": [
+                {
+                    "model": "tests.advert",
+                    "pk": 4,
+                    "fields": {"longadvert": null, "sponsoredpage": null, "slogan": "test", "tags": "[]", "tagged_items": []}
+                }
+            ]
+        }"""
+
+        importer = ImportPlanner(root_page_source_pk=1, destination_parent_id=None)
+        importer.add_json(data)
+        importer.run()
+
+        imported_ad = Advert.objects.filter(id=4).first()
+        self.assertIsNotNone(imported_ad)
+        self.assertIsNone(imported_ad.tags.first())
+
+    @override_settings(WAGTAILTRANSFER_FOLLOWED_REVERSE_RELATIONS=[('tests.advert', 'tagged_items', False)])
+    def test_import_model_with_untracked_deleted_reverse_related_models(self):
+        # test re-importing a model where WAGTAILTRANFER_FOLLOWED_REVERSE_RELATIONS is not used to track tag deletions
+        # will not delete tags
+        from wagtail_transfer import field_adapters
+        importlib.reload(field_adapters)
+        # force reload field adapters as followed/deleted variables are set on module load, so will not get new setting
+        data = """{
+            "ids_for_import": [["tests.advert", 4]],
+            "mappings": [
+                ["taggit.tag", 152, "ac92b2ba-0fa6-11eb-800b-287fcf66f689"],
+                ["tests.advert", 4, "ac931726-0fa6-11eb-800c-287fcf66f689"],
+                ["taggit.taggeditem", 150, "ac938e5a-0fa6-11eb-800d-287fcf66f689"]
+            ],
+            "objects": [
+                {
+                    "model": "tests.advert",
+                    "pk": 4,
+                    "fields": {"longadvert": null, "sponsoredpage": null, "slogan": "test", "tags": "[<Tag: test_tag>]", "tagged_items": [150]}
+                },
+                {
+                    "model": "taggit.taggeditem",
+                    "pk": 150,
+                    "fields": {"content_object": ["tests.advert", 4], "tag": 152}
+                },
+                {
+                    "model": "taggit.tag",
+                    "pk": 152,
+                    "fields": {"name": "test_tag", "slug": "testtag"}
+                }
+            ]
+        }"""
+
+        importer = ImportPlanner(root_page_source_pk=1, destination_parent_id=None)
+        importer.add_json(data)
+        importer.run()
+
+        imported_ad = Advert.objects.filter(id=4).first()
+        self.assertIsNotNone(imported_ad)
+        self.assertEqual(imported_ad.tags.first().name, "test_tag")
+
+        data = """{
+            "ids_for_import": [["tests.advert", 4]],
+            "mappings": [
+                ["tests.advert", 4, "ac931726-0fa6-11eb-800c-287fcf66f689"]
+            ],
+            "objects": [
+                {
+                    "model": "tests.advert",
+                    "pk": 4,
+                    "fields": {"longadvert": null, "sponsoredpage": null, "slogan": "test", "tags": "[]", "tagged_items": []}
+                }
+            ]
+        }"""
+
+        importer = ImportPlanner(root_page_source_pk=1, destination_parent_id=None)
+        importer.add_json(data)
+        importer.run()
+
+        imported_ad = Advert.objects.filter(id=4).first()
+        self.assertIsNotNone(imported_ad)
+        self.assertIsNotNone(imported_ad.tags.first())