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 for crash when deleting via admin site #1347

Conversation

matthewhegarty
Copy link
Contributor

@matthewhegarty matthewhegarty commented Nov 14, 2021

Problem

I have verified that there is a crash if one tries to delete records using the admin site (see #432).

Solution

The fix is simple - to add instance attributes to the RowResult.

This introduced some code duplication, so I added a minor refactor of RowResult to set attributes via a new method.

  • I also fixed sort order for conf.py attributes (raised by make lint).
  • I added the pytz dependency which is needed for testing with python 3.9 (see Added python3.10 to tox environment list #1336).
  • Added test file books-for-delete.csv

Acceptance Criteria

  • Integration test to prove the issue exists and is fixed (test_admin_integration.py)
  • Added unit tests for the new RowResult method

@coveralls
Copy link

coveralls commented Nov 14, 2021

Coverage Status

Coverage increased (+0.009%) to 98.072% when pulling 0c3ab1e on matthewhegarty:error-on-delete-from-admin-432 into 19aa03d on django-import-export:main.

Copy link
Contributor

@manelclos manelclos left a comment

Choose a reason for hiding this comment

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

LGTM

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.

None yet

3 participants