diff --git a/CHANGES.rst b/CHANGES.rst index 65faf08bb..6cb2cf91a 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -5,6 +5,8 @@ Unreleased ------------ - Added `bulk_update_with_history` utility function (gh-650) - Add default user and default change reason to `bulk_create_with_history` and `bulk_update_with_history` +- Start using `_change_reason` instead of `changeReason` to add change reasons to historical +objects. `changeReason` is deprecated and will be removed in version `3.0.0` 2.9.0 (2020-04-23) ------------------ diff --git a/docs/common_issues.rst b/docs/common_issues.rst index 9fe4ca3ce..4b6e4947d 100644 --- a/docs/common_issues.rst +++ b/docs/common_issues.rst @@ -33,19 +33,19 @@ history: 1000 If you want to specify a change reason or history user for each record in the bulk create, -you can add `changeReason` or `_history_user` on each instance: +you can add `_change_reason` or `_history_user` on each instance: .. code-block:: pycon >>> for poll in data: - poll.changeReason = 'reason' + poll._change_reason = 'reason' poll._history_user = my_user >>> objs = bulk_create_with_history(data, Poll, batch_size=500) >>> Poll.history.get(id=data[0].id).history_change_reason 'reason' You can also specify a default user or default change reason responsible for the change -(`_history_user` and `changeReason` take precedence). +(`_history_user` and `_change_reason` take precedence). .. code-block:: pycon diff --git a/docs/historical_model.rst b/docs/historical_model.rst index 0b3d4a065..26a0de3b0 100644 --- a/docs/historical_model.rst +++ b/docs/historical_model.rst @@ -151,10 +151,10 @@ As an example using the callable mechanism, the below changes the default prefix opinion = models.CharField(max_length=2000) register(Opinion, custom_model_name=lambda x:f'Audit{x}') - + The resulting history class names would be `AuditPoll` and `AuditOpinion`. If the app the models are defined in is `yoda` then the corresponding history table names would be `yoda_auditpoll` and `yoda_auditopinion` - + IMPORTANT: Setting `custom_model_name` to `lambda x:f'{x}'` is not permitted. An error will be generated and no history model created if they are the same. @@ -287,8 +287,8 @@ Change Reason Change reason is a message to explain why the change was made in the instance. It is stored in the field ``history_change_reason`` and its default value is ``None``. -By default, the django-simple-history gets the change reason in the field ``changeReason`` of the instance. Also, is possible to pass -the ``changeReason`` explicitly. For this, after a save or delete in an instance, is necessary call the +By default, the django-simple-history gets the change reason in the field ``_change_reason`` of the instance. Also, is possible to pass +the ``_change_reason`` explicitly. For this, after a save or delete in an instance, is necessary call the function ``utils.update_change_reason``. The first argument of this function is the instance and the second is the message that represents the change reason. @@ -308,7 +308,7 @@ You can create an instance with an implicit change reason. .. code-block:: python poll = Poll(question='Question 1') - poll.changeReason = 'Add a question' + poll._change_reason = 'Add a question' poll.save() Or you can pass the change reason explicitly: diff --git a/simple_history/manager.py b/simple_history/manager.py index d09286d80..9526655f2 100755 --- a/simple_history/manager.py +++ b/simple_history/manager.py @@ -3,6 +3,8 @@ from django.db import models from django.utils import timezone +from simple_history.utils import get_change_reason_from_object + class HistoryDescriptor(object): def __init__(self, model): @@ -121,9 +123,8 @@ def bulk_history_create( row = self.model( history_date=getattr(instance, "_history_date", timezone.now()), history_user=getattr(instance, "_history_user", default_user), - history_change_reason=getattr( - instance, "changeReason", default_change_reason - ), + history_change_reason=get_change_reason_from_object(instance) + or default_change_reason, history_type=history_type, **{ field.attname: getattr(instance, field.attname) diff --git a/simple_history/models.py b/simple_history/models.py index 31b2093bc..5e34e3d6f 100644 --- a/simple_history/models.py +++ b/simple_history/models.py @@ -24,6 +24,7 @@ from . import exceptions from .manager import HistoryDescriptor from .signals import post_create_historical_record, pre_create_historical_record +from .utils import get_change_reason_from_object if django.VERSION < (2,): from django.utils.translation import ugettext_lazy as _ @@ -479,7 +480,7 @@ def create_historical_record(self, instance, history_type, using=None): using = using if self.use_base_model_db else None history_date = getattr(instance, "_history_date", timezone.now()) history_user = self.get_history_user(instance) - history_change_reason = getattr(instance, "changeReason", None) + history_change_reason = get_change_reason_from_object(instance) manager = getattr(instance, self.manager_name) attrs = {} diff --git a/simple_history/tests/tests/test_admin.py b/simple_history/tests/tests/test_admin.py index 7c3e58c50..e9b48b41c 100644 --- a/simple_history/tests/tests/test_admin.py +++ b/simple_history/tests/tests/test_admin.py @@ -79,7 +79,7 @@ def test_history_list(self): self.assertEqual(model_name, "customuser") self.login() poll = Poll(question="why?", pub_date=today) - poll.changeReason = "A random test reason" + poll._change_reason = "A random test reason" poll._history_user = self.user poll.save() diff --git a/simple_history/tests/tests/test_manager.py b/simple_history/tests/tests/test_manager.py index a3128cab3..b1e4631a1 100644 --- a/simple_history/tests/tests/test_manager.py +++ b/simple_history/tests/tests/test_manager.py @@ -113,7 +113,7 @@ def test_simple_bulk_history_create(self): def test_bulk_history_create_with_change_reason(self): for poll in self.data: - poll.changeReason = "reason" + poll._change_reason = "reason" Poll.history.bulk_history_create(self.data) @@ -162,7 +162,7 @@ def test_bulk_history_create_history_user_overrides_default(self): def test_bulk_history_create_change_reason_overrides_default(self): for data in self.data: - data.changeReason = "my_reason" + data._change_reason = "my_reason" Poll.history.bulk_history_create(self.data, default_change_reason="test") @@ -236,7 +236,7 @@ def test_simple_bulk_history_create(self): def test_bulk_history_create_with_change_reason(self): for poll in self.data: - poll.changeReason = "reason" + poll._change_reason = "reason" Poll.history.bulk_history_create(self.data) diff --git a/simple_history/tests/tests/test_models.py b/simple_history/tests/tests/test_models.py index fcc1aed2e..4ffc8e21b 100644 --- a/simple_history/tests/tests/test_models.py +++ b/simple_history/tests/tests/test_models.py @@ -171,7 +171,7 @@ def test_update(self): def test_delete_verify_change_reason_implicitly(self): p = Poll.objects.create(question="what's up?", pub_date=today) poll_id = p.id - p.changeReason = "wrongEntry" + p._change_reason = "wrongEntry" p.delete() delete_record, create_record = Poll.history.all() self.assertRecordValues( diff --git a/simple_history/utils.py b/simple_history/utils.py index 9aa8c8c1e..ff1b7c829 100644 --- a/simple_history/utils.py +++ b/simple_history/utils.py @@ -1,3 +1,5 @@ +import warnings + import django from django.db import transaction from django.forms.models import model_to_dict @@ -123,3 +125,19 @@ def bulk_update_with_history( default_user=default_user, default_change_reason=default_change_reason, ) + + +def get_change_reason_from_object(obj): + if hasattr(obj, "_change_reason"): + return getattr(obj, "_change_reason") + + if hasattr(obj, "changeReason"): + warning_msg = ( + "Using the attr changeReason to populate history_change_reason is" + " deprecated in 2.10.0 and will be removed in 3.0.0. Use " + "_change_reason instead. " + ) + warnings.warn(warning_msg, DeprecationWarning) + return getattr(obj, "changeReason") + + return None