Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: replace custom duplication logic with signal #256

Merged
merged 1 commit into from
Aug 30, 2023

Conversation

b1rger
Copy link
Contributor

@b1rger b1rger commented Aug 18, 2023

For duplicating entities, there was custom logic to copy ManyToManyField

  • which was tailored to TempTriples. This had the downside of simply
    creating new objects by hand.

This commit introduces a duplicate method in the Triple model, which
implements the functionality of duplicating itself and also triggers
pre_- and post_duplicate signals. This has the advantage that instances
that point to the Triple (like References) can also be duplicated using
signals.

@b1rger b1rger force-pushed the birger/implement-duplicate-for-relations branch 2 times, most recently from f5c94e3 to 6ea6ab3 Compare August 22, 2023 09:56
@b1rger b1rger marked this pull request as ready for review August 22, 2023 09:59
@b1rger b1rger requested a review from a team August 28, 2023 11:16
Copy link
Collaborator

@sennierer sennierer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good to me with two minor suggestions

self._state.adding = True
duplicate = self.save()

instance = origin.objects.get(pk=oldid)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wouldnt it be cheaper (database calls wise) to deep copy self before setting pk and id to None and using that as instance? instead of loading it again from the db? because I guess instance == self [pre line 320]

apis_core/apis_relations/signals.py Show resolved Hide resolved
For duplicating entities, there was custom logic to copy ManyToManyField
- which was tailored to TempTriples. This had the downside of simply
creating new objects by hand.

This commit introduces a `duplicate` method in the Triple model, which
implements the functionality of duplicating itself and also triggers
pre_- and post_duplicate signals. This has the advantage that instances
that point to the Triple (like References) can also be duplicated using
signals.
@b1rger b1rger force-pushed the birger/implement-duplicate-for-relations branch from 6ea6ab3 to 540dca9 Compare August 30, 2023 08:09
@b1rger b1rger merged commit 0b1c20d into main Aug 30, 2023
6 checks passed
@b1rger b1rger deleted the birger/implement-duplicate-for-relations branch September 8, 2023 09:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants