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 bulk create with history with duplicates #1037

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

WestonChan
Copy link

Describe the bug
When using bulk_create_with_history, if there is an item that is a duplicate (either created earlier in the same batch or already existing in the database) the returned object includes both the newly created and the existing duplicated object.
In addition, history will be generated for both items, even though only the newer item should have generated history. This will happen multiple times if the duplicates are created in the same batch.

In summary:

If a duplicate item already exists in the database:

  • All duplicate items will be returned
  • History will be generated for all duplicate items

To Reproduce

  1. Use a database which doesn't return ids
  2. Save an object to the database
  3. Create duplicate object(s) using bulk_create_with_history
  4. Inspect returned result and historical entry

Expected behavior

  • Only items that were created should be returned
  • Duplicated objects that were not created should not be returned
  • History should be generated only for objects that were created

Environment:

  • Django Simple History Version: 3.0.0
  • Django Version: 3.2.15
  • Database Version: MySQL 8.0.29

@codecov
Copy link

codecov bot commented Sep 19, 2022

Codecov Report

Merging #1037 (c3a01d4) into master (529dbe2) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

❗ Current head c3a01d4 differs from pull request most recent head 7e9cb15. Consider uploading reports for the commit 7e9cb15 to get more accurate results

@@            Coverage Diff             @@
##           master    #1037      +/-   ##
==========================================
- Coverage   97.65%   97.64%   -0.01%     
==========================================
  Files          23       23              
  Lines        1149     1146       -3     
  Branches      222      221       -1     
==========================================
- Hits         1122     1119       -3     
  Misses         12       12              
  Partials       15       15              
Impacted Files Coverage Δ
simple_history/utils.py 100.00% <100.00%> (ø)

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

@WestonChan WestonChan force-pushed the fix-bulk-create-with-history-with-duplicates branch from c3a01d4 to 7e9cb15 Compare September 19, 2022 14:03
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.

1 participant