Skip to content

Commit

Permalink
Allow empty fields to bulk_update_with_history
Browse files Browse the repository at this point in the history
This fixes a "hack" for when a user only wants to provide the
`custom_historical_attrs` argument, where a list with arbitrary field
names could be provided as a circumvention for the error that was raised
otherwise - even if no model fields were actually changed.
  • Loading branch information
ddabble committed Sep 25, 2023
1 parent 1032b40 commit c1ec612
Show file tree
Hide file tree
Showing 4 changed files with 15 additions and 4 deletions.
2 changes: 2 additions & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ Unreleased
- Added ``custom_historical_attrs`` to ``bulk_create_with_history()`` and
``bulk_update_with_history()`` for setting additional fields on custom history models
(gh-1248)
- Passing an empty list as the ``fields`` argument to ``bulk_update_with_history()`` is
now allowed; history records will still be created (gh-1248)


3.4.0 (2023-08-18)
Expand Down
2 changes: 1 addition & 1 deletion docs/common_issues.rst
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ A field ``session`` would be passed as ``custom_historical_attrs={'session': 'ja
.. code-block:: pycon
>>> bulk_update_with_history(
data, PollWithHistoricalSessionAttr,
data, PollWithHistoricalSessionAttr, [],
custom_historical_attrs={'session': 'jam'}
)
>>> data[0].history.latest().session
Expand Down
2 changes: 1 addition & 1 deletion simple_history/tests/tests/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -541,7 +541,7 @@ def test_bulk_update_history_with_custom_model_attributes(self):
bulk_update_with_history(
self.data,
PollWithHistoricalSessionAttr,
fields=["question"],
fields=[],
custom_historical_attrs={"session": "training"},
)

Expand Down
13 changes: 11 additions & 2 deletions simple_history/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,8 @@ def bulk_update_with_history(
their history (all in one transaction).
:param objs: List of objs of type model to be updated
:param model: Model class that should be updated
:param fields: The fields that are updated
:param fields: The fields that are updated. If empty, no model objects will be
changed, but history records will still be created.
:param batch_size: Number of objects that should be updated in each batch
:param default_user: Optional user to specify as the history_user in each historical
record
Expand All @@ -189,7 +190,15 @@ def bulk_update_with_history(
raise AlternativeManagerError("The given manager does not belong to the model.")

with transaction.atomic(savepoint=False):
rows_updated = model_manager.bulk_update(objs, fields, batch_size=batch_size)
if not fields:
# Allow not passing any fields if the user wants to bulk-create history
# records - e.g. with `custom_historical_attrs` provided
# (Calling `bulk_update()` with no fields would have raised an error)
rows_updated = 0
else:
rows_updated = model_manager.bulk_update(
objs, fields, batch_size=batch_size
)
history_manager.bulk_history_create(
objs,
batch_size=batch_size,
Expand Down

0 comments on commit c1ec612

Please sign in to comment.