Skip to content

Conversation

@Akollek
Copy link
Contributor

@Akollek Akollek commented Sep 7, 2022

The problem

It seems that when copying a many to one relationship a duplicate of the other object is created, but the foreign key change on the original object isn't saved. This evaded being caught by tests since the unsaved object was used for comparison.

The solution

Save the object again after coping all foreign keys.

@Akollek Akollek changed the title Save duplicate after m2o fields updated Fix: Save duplicated objects after foreign keys updated Sep 7, 2022
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Thanks for implementing a fix, could you ensure that the test covers your changes.

@Akollek
Copy link
Contributor Author

Akollek commented Sep 7, 2022

Hey @jackton1 , came across this bug while using django-clone, let me know what you think of the fix

@codecov
Copy link

codecov bot commented Sep 9, 2022

Codecov Report

Merging #633 (9fe1202) into main (ec9a0ac) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##              main      #633   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            5         5           
  Lines          287       288    +1     
=========================================
+ Hits           287       288    +1     
Impacted Files Coverage Δ
model_clone/mixin.py 100.00% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@Akollek Akollek requested a review from jackton1 September 9, 2022 09:50
@jackton1
Copy link
Member

jackton1 commented Sep 9, 2022

@all-contributors please add @Akollek for code bug test

@allcontributors
Copy link
Contributor

@jackton1

I've put up a pull request to add @Akollek! 🎉

@jackton1 jackton1 merged commit 1ed88ef into tj-django:main Sep 9, 2022
@jackton1
Copy link
Member

jackton1 commented Sep 9, 2022

Hey @jackton1 , came across this bug while using django-clone, let me know what you think of the fix

LGTM, Thanks for updating the test

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants