Keep reference of copied items for cross referencing

[saschahofmann]

As explained in #544, I am trying to keep track of instances that have been cloned before and reuse that copy for later references that otherwise would be cloned again.

I added on example test, are you ok with me adding cross-references between the tables or should I create new models and do you prefer one big test for all cases or individual ones?

[saschahofmann]

@jackton1 Closed the other in favor of this. I fixed the linting errors and added another assert in the test. Ready for review IMO.

[saschahofmann]

Ok. Nvm there is an issue with the o2o cloning. Will fix and test

[saschahofmann]

Ok the issue only appears if I make __duplicate_o2m_fields call make_clone which I want to add as well. I'll do it here as well. I hoped to separate them but I think it makes sense to do them together

[saschahofmann]

Ok fixed and now also closes #544

[pep8speaks]

Hello @saschahofmann! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2022-04-07 13:52:08 UTC

[codecov]

Codecov Report

Merging #579 (7796cf9) into main (2a99a51) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##              main      #579   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            5         5           
  Lines          285       297   +12     
=========================================
+ Hits           285       297   +12     

Impacted Files	Coverage Δ
model_clone/mixin.py	100.00% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2a99a51...7796cf9. Read the comment docs.

[jackton1]

Hello @saschahofmann! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

• In the file model_clone/mixin.py:

Line 230:101: E501 line too long (109 > 100 characters)
Line 299:101: E501 line too long (109 > 100 characters)
Line 314:26: E225 missing whitespace around operator
Line 469:23: E271 multiple spaces after keyword
Line 472:101: E501 line too long (139 > 100 characters)
Line 475:25: E122 continuation line missing indentation or outdented
Line 476:25: E122 continuation line missing indentation or outdented
Line 477:25: E122 continuation line missing indentation or outdented
Line 478:25: E122 continuation line missing indentation or outdented
Line 479:21: E122 continuation line missing indentation or outdented

• In the file model_clone/tests/test_clone_mixin.py:

Line 887:12: W291 trailing whitespace

@saschahofmann Can you also resolve these errors.

[saschahofmann]

Hello @saschahofmann! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2022-03-24 09:50:03 UTC

Done

[saschahofmann]

Two suggestions:

• I would turn off the code scanner seems like it's just cluttering the diff preview hard to find the codecov prompts among all these useless suggestions
• Maybe we can bump min python dependency to 3.6 or even 3.8?

[jackton1]

Two suggestions:

• I would turn off the code scanner seems like it's just cluttering the diff preview hard to find the codecov prompts among all these useless suggestions
• Maybe we can bump min python dependency to 3.6 or even 3.8?

Seems we need to improve the test coverage it’s currently under 100%

[saschahofmann]

Yeah exactly and it's super annoying to find which lines are covered because half the file is flagged by codescanner

[saschahofmann]

@jackton1 I actually discovered a few problems with cross-references. I think it boils down to this: _create_copy_of_instance creates and copies one_to_onefields before make_clone can replace the objects.

Example:

class Sentence(CloneMixin, models.Model):
    value = models.TextField()

    _clone_o2o_fields = {"ending",}


class Ending(CloneMixin, models.Model):
    sentence = models.OneToOneField(
        Sentence, on_delete=models.CASCADE, related_name="ending"
    )
    _clone_o2o_fields = {"sentence"}


class Triangle(CloneMixin, models.Model):
    sentence = models.OneToOneField(
        Sentence, on_delete=models.CASCADE, related_name="triangle"
    )
    ending = models.OneToOneField(
        Ending, on_delete=models.CASCADE, related_name="triangle"
    )

    _clone_o2o_fields = {"sentence", "ending"}

This is a triangular relationship

When it copies ending it will call _create_copy_of_instance and that will _create_copy_of_instance for sentence
but this time with sub_clone=True which leads to an IntegrityError because it tries to keep the sentence_id. I think this error should only happen if one of these models wasn't a CloneMixin or maybe not even then. Anyway, is there a way we can set the one_to_one fields of the duplicate to null and replace them in the actual make_clone command?

[jackton1]

@saschahofmann See: #669