From 4a8613ce4c791099810a603dd4c00f15c4037480 Mon Sep 17 00:00:00 2001 From: Alan Rominger Date: Thu, 18 Aug 2022 11:39:29 -0400 Subject: [PATCH] Avoid updating modified_by from None to None (#11838) This should help the case of inventory updates in particular where imported hosts are managed by the system --- awx/main/models/base.py | 9 +++-- awx/main/tests/functional/models/test_base.py | 40 +++++++++++++++++++ 2 files changed, 45 insertions(+), 4 deletions(-) create mode 100644 awx/main/tests/functional/models/test_base.py diff --git a/awx/main/models/base.py b/awx/main/models/base.py index da12f603cb..571f37ccb9 100644 --- a/awx/main/models/base.py +++ b/awx/main/models/base.py @@ -316,16 +316,17 @@ class PrimordialModel(HasEditsMixin, CreatedModifiedModel): user = get_current_user() if user and not user.id: user = None - if not self.pk and not self.created_by: + if (not self.pk) and (user is not None) and (not self.created_by): self.created_by = user if 'created_by' not in update_fields: update_fields.append('created_by') # Update modified_by if any editable fields have changed new_values = self._get_fields_snapshot() if (not self.pk and not self.modified_by) or self._values_have_edits(new_values): - self.modified_by = user - if 'modified_by' not in update_fields: - update_fields.append('modified_by') + if self.modified_by != user: + self.modified_by = user + if 'modified_by' not in update_fields: + update_fields.append('modified_by') super(PrimordialModel, self).save(*args, **kwargs) self._prior_values_store = new_values diff --git a/awx/main/tests/functional/models/test_base.py b/awx/main/tests/functional/models/test_base.py new file mode 100644 index 0000000000..01d3b0e347 --- /dev/null +++ b/awx/main/tests/functional/models/test_base.py @@ -0,0 +1,40 @@ +from unittest import mock +import pytest + +from crum import impersonate + +from awx.main.models import Host + + +@pytest.mark.django_db +def test_modified_by_not_changed(inventory): + with impersonate(None): + host = Host.objects.create(name='foo', inventory=inventory) + assert host.modified_by == None + host.variables = {'foo': 'bar'} + with mock.patch('django.db.models.Model.save') as save_mock: + host.save(update_fields=['variables']) + save_mock.assert_called_once_with(update_fields=['variables']) + + +@pytest.mark.django_db +def test_modified_by_changed(inventory, alice): + with impersonate(None): + host = Host.objects.create(name='foo', inventory=inventory) + assert host.modified_by == None + with impersonate(alice): + host.variables = {'foo': 'bar'} + with mock.patch('django.db.models.Model.save') as save_mock: + host.save(update_fields=['variables']) + save_mock.assert_called_once_with(update_fields=['variables', 'modified_by']) + assert host.modified_by == alice + + +@pytest.mark.django_db +def test_created_by(inventory, alice): + with impersonate(alice): + host = Host.objects.create(name='foo', inventory=inventory) + assert host.created_by == alice + with impersonate(None): + host = Host.objects.create(name='bar', inventory=inventory) + assert host.created_by == None