From 049a5e71b1dd169f0c273882450962e54f3a8853 Mon Sep 17 00:00:00 2001 From: Ross Mechanic Date: Mon, 15 Apr 2019 16:40:10 -0400 Subject: [PATCH] Fix non-backward compatible issue from 2.7.0 from GH-507 --- CHANGES.rst | 18 ++--- docs/multiple_dbs.rst | 2 +- simple_history/models.py | 2 +- simple_history/tests/models.py | 14 +++- simple_history/tests/tests/test_models.py | 95 ++++++++++++++--------- 5 files changed, 81 insertions(+), 50 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index e1adfc814..0d0bbc1a5 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -1,20 +1,16 @@ Changes ======= -- remove reference to vendored library ``django.utils.six`` in favor of ``six`` (gh-526) -- Fixed hardcoded history manager (gh-540) - Unreleased ----------- -- Added the possibility to create a relation to the original model - -2.7.1 (2019-03-26) ------------------ -- Added routing for saving historical records to separate databases if necessary. +- Added the possibility to create a relation to the original model (gh-536) +- Fix router backward-compatibility issue with 2.7.0 (gh-539) +- Fix hardcoded history manager (gh-542) +- Replace deprecated `django.utils.six` with `six` (gh-526) 2.7.0 (2019-01-16) ------------------ -- Add support for ``using`` chained manager method and save/delete keyword argument (gh-507) +- \* Add support for ``using`` chained manager method and save/delete keyword argument (gh-507) - Added management command ``clean_duplicate_history`` to remove duplicate history entries (gh-483) - Updated most_recent to work with excluded_fields (gh-477) - Fixed bug that prevented self-referential foreign key from using ``'self'`` (gh-513) @@ -22,6 +18,10 @@ Unreleased - Don't resolve relationships for history objects (gh-479) - Reorganization of docs (gh-510) +\* NOTE: This change was not backward compatible for users using routers to write +history tables to a separate database from their base tables. This issue is fixed in +2.7.1. + 2.6.0 (2018-12-12) ------------------ - Add ``app`` parameter to the constructor of ``HistoricalRecords`` (gh-486) diff --git a/docs/multiple_dbs.rst b/docs/multiple_dbs.rst index c5cb74caf..2737568be 100644 --- a/docs/multiple_dbs.rst +++ b/docs/multiple_dbs.rst @@ -53,4 +53,4 @@ class MyModel(models.Model): If set to `True`, migrations and audit events will be sent to the same database as the base model. If `False`, they -will be sent to the place specified by the database router. +will be sent to the place specified by the database router. The default value is `False`. diff --git a/simple_history/models.py b/simple_history/models.py index ef07c815e..6643716e5 100644 --- a/simple_history/models.py +++ b/simple_history/models.py @@ -79,7 +79,7 @@ def __init__( history_user_getter=_history_user_getter, history_user_setter=_history_user_setter, related_name=None, - use_base_model_db=True, + use_base_model_db=False, ): self.user_set_verbose_name = verbose_name self.user_related_name = user_related_name diff --git a/simple_history/tests/models.py b/simple_history/tests/models.py index 9e909d410..25f9d1245 100644 --- a/simple_history/tests/models.py +++ b/simple_history/tests/models.py @@ -364,7 +364,19 @@ class ModelWithHistoryInDifferentApp(models.Model): class ModelWithHistoryInDifferentDb(models.Model): name = models.CharField(max_length=30) - history = HistoricalRecords(use_base_model_db=False) + history = HistoricalRecords() + + +class ModelWithHistoryUseBaseModelDb(models.Model): + name = models.CharField(max_length=30) + history = HistoricalRecords(use_base_model_db=True) + + +class ModelWithFkToModelWithHistoryUseBaseModelDb(models.Model): + fk = models.ForeignKey( + ModelWithHistoryUseBaseModelDb, on_delete=models.CASCADE, null=True + ) + history = HistoricalRecords(use_base_model_db=True) ############################################################################### diff --git a/simple_history/tests/tests/test_models.py b/simple_history/tests/tests/test_models.py index 8fa5344bf..7dc73bb89 100644 --- a/simple_history/tests/tests/test_models.py +++ b/simple_history/tests/tests/test_models.py @@ -1,11 +1,11 @@ from __future__ import unicode_literals import unittest - -import django import uuid import warnings from datetime import datetime, timedelta + +import django from django.apps import apps from django.contrib.auth import get_user_model from django.core.exceptions import ObjectDoesNotExist @@ -18,22 +18,19 @@ from simple_history import register from simple_history.exceptions import RelatedNameConflictError from simple_history.models import HistoricalRecords, ModelChange -from simple_history.signals import ( - pre_create_historical_record, - post_create_historical_record, -) +from simple_history.signals import pre_create_historical_record from simple_history.tests.custom_user.models import CustomUser from simple_history.tests.tests.utils import ( database_router_override_settings, - middleware_override_settings, database_router_override_settings_history_in_diff_db, + middleware_override_settings, ) from simple_history.utils import get_history_model_for_model from simple_history.utils import update_change_reason from ..external.models import ( ExternalModel, - ExternalModelWithCustomUserIdField, ExternalModelRegistered, + ExternalModelWithCustomUserIdField, ) from ..models import ( AbstractBase, @@ -66,7 +63,9 @@ HistoricalPollWithHistoricalIPAddress, HistoricalState, Library, + ModelWithFkToModelWithHistoryUseBaseModelDb, ModelWithHistoryInDifferentDb, + ModelWithHistoryUseBaseModelDb, MultiOneToOne, Person, Place, @@ -1262,78 +1261,98 @@ class MultiDBWithUsingTest(TestCase): db_name = "other" def test_multidb_with_using_not_on_default(self): - book = Book.objects.using(self.db_name).create(isbn="1-84356-028-1") - self.assertRaises(ObjectDoesNotExist, book.history.get, isbn="1-84356-028-1") + model = ModelWithHistoryUseBaseModelDb.objects.using(self.db_name).create( + name="1-84356-028-1" + ) + self.assertRaises(ObjectDoesNotExist, model.history.get, name="1-84356-028-1") def test_multidb_with_using_is_on_dbtwo(self): - book = Book.objects.using(self.db_name).create(isbn="1-84356-028-1") + model = ModelWithHistoryUseBaseModelDb.objects.using(self.db_name).create( + name="1-84356-028-1" + ) try: - book.history.using(self.db_name).get(isbn="1-84356-028-1") + model.history.using(self.db_name).get(name="1-84356-028-1") except ObjectDoesNotExist: self.fail("ObjectDoesNotExist unexpectedly raised.") def test_multidb_with_using_and_fk_not_on_default(self): - book = Book.objects.using(self.db_name).create(isbn="1-84356-028-1") - library = Library.objects.using(self.db_name).create(book=book) - self.assertRaises(ObjectDoesNotExist, library.history.get, book=book) + model = ModelWithHistoryUseBaseModelDb.objects.using(self.db_name).create( + name="1-84356-028-1" + ) + parent_model = ModelWithFkToModelWithHistoryUseBaseModelDb.objects.using( + self.db_name + ).create(fk=model) + self.assertRaises(ObjectDoesNotExist, parent_model.history.get, fk=model) def test_multidb_with_using_and_fk_on_dbtwo(self): - book = Book.objects.using(self.db_name).create(isbn="1-84356-028-1") - library = Library.objects.using(self.db_name).create(book=book) + model = ModelWithHistoryUseBaseModelDb.objects.using(self.db_name).create( + name="1-84356-028-1" + ) + parent_model = ModelWithFkToModelWithHistoryUseBaseModelDb.objects.using( + self.db_name + ).create(fk=model) try: - library.history.using(self.db_name).get(book=book) + parent_model.history.using(self.db_name).get(fk=model) except ObjectDoesNotExist: self.fail("ObjectDoesNotExist unexpectedly raised.") def test_multidb_with_using_keyword_in_save_not_on_default(self): - book = Book(isbn="1-84356-028-1") - book.save(using=self.db_name) - self.assertRaises(ObjectDoesNotExist, book.history.get, isbn="1-84356-028-1") + model = ModelWithHistoryUseBaseModelDb(name="1-84356-028-1") + model.save(using=self.db_name) + self.assertRaises(ObjectDoesNotExist, model.history.get, name="1-84356-028-1") def test_multidb_with_using_keyword_in_save_on_dbtwo(self): - book = Book(isbn="1-84356-028-1") - book.save(using=self.db_name) + model = ModelWithHistoryUseBaseModelDb(name="1-84356-028-1") + model.save(using=self.db_name) try: - book.history.using(self.db_name).get(isbn="1-84356-028-1") + model.history.using(self.db_name).get(name="1-84356-028-1") except ObjectDoesNotExist: self.fail("ObjectDoesNotExist unexpectedly raised.") def test_multidb_with_using_keyword_in_save_with_fk(self): - book = Book(isbn="1-84356-028-1") - book.save(using=self.db_name) - library = Library(book=book) - library.save(using=self.db_name) + model = ModelWithHistoryUseBaseModelDb(name="1-84356-028-1") + model.save(using=self.db_name) + parent_model = ModelWithFkToModelWithHistoryUseBaseModelDb(fk=model) + parent_model.save(using=self.db_name) # assert not created on default - self.assertRaises(ObjectDoesNotExist, library.history.get, book=book) + self.assertRaises(ObjectDoesNotExist, parent_model.history.get, fk=model) # assert created on dbtwo try: - library.history.using(self.db_name).get(book=book) + parent_model.history.using(self.db_name).get(fk=model) except ObjectDoesNotExist: self.fail("ObjectDoesNotExist unexpectedly raised.") def test_multidb_with_using_keyword_in_save_and_update(self): - book = Book.objects.using(self.db_name).create(isbn="1-84356-028-1") - book.save(using=self.db_name) + model = ModelWithHistoryUseBaseModelDb.objects.using(self.db_name).create( + name="1-84356-028-1" + ) + model.save(using=self.db_name) self.assertEqual( ["+", "~"], [ obj.history_type - for obj in book.history.using(self.db_name) + for obj in model.history.using(self.db_name) .all() .order_by("history_date") ], ) def test_multidb_with_using_keyword_in_save_and_delete(self): - HistoricalBook = get_history_model_for_model(Book) - book = Book.objects.using(self.db_name).create(isbn="1-84356-028-1") - book.save(using=self.db_name) - book.delete(using=self.db_name) + HistoricalModelWithHistoryUseBaseModelDb = get_history_model_for_model( + ModelWithHistoryUseBaseModelDb + ) + model = ModelWithHistoryUseBaseModelDb.objects.using(self.db_name).create( + name="1-84356-028-1" + ) + model.save(using=self.db_name) + model.delete(using=self.db_name) self.assertEqual( ["+", "~", "-"], [ obj.history_type - for obj in HistoricalBook.objects.using(self.db_name) + for obj in HistoricalModelWithHistoryUseBaseModelDb.objects.using( + self.db_name + ) .all() .order_by("history_date") ],