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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use 'create' flag instead of instance.pk (#1274) #1362

Conversation

matthewhegarty
Copy link
Contributor

Problem

(issue #1274)

For bulk creates / updates, the instance.pk field was being used to determine whether a bulk update or create should be performed. This doesn't work when pk is dynamically generated (e.g. UUID).

This was originally reviewed in #1278, but that PR was closed when I force pushed following rebase 馃う

Solution

This fix uses the 'new' value returned from get_or_init_instance(), and passes this in as a kwarg to save_instance(). Hence the check against instance.pk is not required.

This will be an API breaking change.

Acceptance Criteria

  • Added new integration tests which verify the issue
  • Existing 'bulk' tests pass
  • Manually tested bulk imports using the import_book script (also made some changes here)

@coveralls
Copy link

coveralls commented Dec 23, 2021

Coverage Status

Coverage remained the same at 98.073% when pulling 9386fff on matthewhegarty:use-create-flag-instead-of-instance-pk into 0ccd089 on django-import-export:main.

@matthewhegarty matthewhegarty mentioned this pull request Dec 23, 2021
@matthewhegarty matthewhegarty changed the base branch from main to release-3-x December 24, 2021 09:35
changed 'is_create' to mandatory arg

added 'pk_attr' property

reverted self.save_instance() call params

added tests for UUID model field

fixed test_resources.py imports

added is_create flag

refactored migration name

removed django_extensions from settings

documented breaking change in changelog

removed unused pk_attr attribute

fixed indentation
@matthewhegarty matthewhegarty force-pushed the use-create-flag-instead-of-instance-pk branch from 4588f3e to 40b44a7 Compare December 24, 2021 09:41
@matthewhegarty matthewhegarty merged commit 6ed7b18 into django-import-export:release-3-x Dec 24, 2021
@matthewhegarty matthewhegarty deleted the use-create-flag-instead-of-instance-pk branch February 22, 2023 08:41
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.

None yet

2 participants