[Feature] o2o clone doesnt call make_clone

[saschahofmann]

Is this feature missing in the latest version?

• I'm using the latest release

Is your feature request related to a problem? Please describe.

I am trying to clone a model with a deep relationship tree. Some of the copying steps have side_effects e.g. children need to replace data based on their new clone parent. To achieve this, I thought I could overwrite the make_clone function on the models.

 def make_clone(self, attrs=None, sub_clone=False, using=None):
       # ... edit attrs here based on parent that's contained in attrs
       return super().make_clone(attrs=attrs, sub_clone=sub_clone, using=using)

This works fine for m2m and o2m/m2o but the function is called when the relationship is an o2o. I had a look at mixin.py and think the reason is that functions like __duplicate_o2m_fields actually try to call make_clone if possible while (__duplicate_o2m_fields)[https://github.com/tj-django/django-clone/blob/main/model_clone/mixin.py#L432] doesn't

Describe the solution you'd like?

I hope that adding something like this could work:

def __duplicate_o2o_fields(self, duplicate, using=None):
      for f in self._meta.related_objects:
          if f.one_to_one:
              if any(
                  [
                      f.name in self._clone_o2o_fields
                      and f not in self._meta.concrete_fields,
                      self._clone_excluded_o2o_fields
                      and f.name not in self._clone_excluded_o2o_fields
                      and f not in self._meta.concrete_fields,
                  ]
              ):
                  rel_object = getattr(self, f.name, None)
                  if rel_object:
                      if hasattr(item, "make_clone"):
                          new_rel_object = rel_object.make_clone(...)
                     else:
                          new_rel_object = CloneMixin._create_copy_of_instance(
                              rel_object,
                              force=True,
                              sub_clone=True,
                           )
                      setattr(new_rel_object, f.remote_field.name, duplicate)
                      new_rel_object.save(using=using)

      return duplicate

Not sure how exactly to propagate the attrs and whether the relationship will replaced properly?

Describe alternatives you've considered?

I think for now I will call make_clone on the object explicitly in the parent but I think above might be more consistent and maybe more the expected behaviour.

Anything else?

Do you think this is the right approach to this anyway? I thought it might be quite neat to put the logic where it belongs especially if the logic is the same no matter from where the cloning is started. The alternative would be to clone all objects and then change the things that need to be changed.

A probably much bigger ask and a long stretch

As mentioned above, we have a quite complicated dependency logic. I am going to try to simplify it here:
All models are part of an Organisation which maybe has Offices, and Employees. Within Offices there are Desks that have an o2o to Employees. Now if we set up everything correctly and we clone an Organisation it will clone all Employees and Offices and even the Desks with the correct upstream dependencies only the relationship from Desk will still point to the original Employee. It's probably beyond the scope of the library but if that would be handled that'll be amazing. I have no idea how that would be done consistently since it's tricky to do even if you know the structure!

Cheers for a great library ✌️

Code of Conduct

• I agree to follow this project's Code of Conduct

[github-actions]

Thanks for reporting this issue.

[saschahofmann]

@jackton1 I thought about the last point some more.

As I mention there we actually struggle with some recursion issues when cloning a complex tree of models. Also, as I describe above, it'd be amazing if the library would recognise that a dependency of an object has already been cloned via a different branch somewhere upstream. I think this could actually be doable by maintaining a simple mapping of old to already cloned instances and using them whenever another instance has a dependency to those, instead of making a second copy. This would also kill some of the recursion issues.

I could have a go at that if that's something you'd like to include in the library

[jackton1]

Yep agreed the major hurdle is keeping track of the parent object and preventing duplicate clones of the parent from the child instance.