diff --git a/netbox_custom_objects/api/serializers.py b/netbox_custom_objects/api/serializers.py index 97407361..ffcdd53a 100644 --- a/netbox_custom_objects/api/serializers.py +++ b/netbox_custom_objects/api/serializers.py @@ -331,6 +331,7 @@ class Meta: "slug", "version", "group_name", + "changelog_enabled", "description", "tags", "created", @@ -343,6 +344,15 @@ class Meta: read_only_fields = ("schema_document",) brief_fields = ("id", "url", "name", "slug", "description") + def validate(self, data): + # changelog_enabled is locked after creation. + if self.instance and self.instance.pk: + if 'changelog_enabled' in data and data['changelog_enabled'] != self.instance.changelog_enabled: + raise ValidationError({ + 'changelog_enabled': _("Cannot be changed after creation.") + }) + return super().validate(data) + def get_table_model_name(self, obj): return obj.get_table_model_name(obj.id) @@ -542,6 +552,13 @@ def get__context(self, obj): def create(self, validated_data): ModelClass = self.Meta.model + # taggit's TaggableManager is not detected by model_meta.get_field_info() as a + # standard M2M relation. If left in validated_data it gets passed to + # Model._default_manager.create(), where Django's __init__ sets it as a plain + # instance attribute (shadowing the manager), giving the illusion of success but + # never persisting to the DB. Pop it here and save via taggit's own API. + tags = validated_data.pop('tags', None) + info = model_meta.get_field_info(ModelClass) many_to_many = {} for field_name, relation_info in info.relations.items(): @@ -562,6 +579,12 @@ def create(self, validated_data): instance = ModelClass._default_manager.create(**validated_data) + if tags is not None: + instance._tags = tags + instance.tags.set([t.name for t in tags]) + else: + instance._tags = [] + if many_to_many: for field_name, value in many_to_many.items(): field = getattr(instance, field_name) @@ -580,6 +603,11 @@ def create(self, validated_data): # Stock DRF update() with custom field.set() for M2M def update(self, instance, validated_data): + # Pop tags before the setattr loop — taggit's manager has no __set__, so + # leaving tags in validated_data would shadow the manager with a plain list. + tags = validated_data.pop('tags', None) + instance._tags = tags or [] + info = model_meta.get_field_info(instance) # Pop polymorphic GFK fields @@ -606,6 +634,10 @@ def update(self, instance, validated_data): instance.save() + if tags is not None: + instance._tags = tags + instance.tags.set([t.name for t in tags]) + for attr, value in m2m_fields: field = getattr(instance, attr) field.set(value, clear=True) diff --git a/netbox_custom_objects/branching.py b/netbox_custom_objects/branching.py index 667dd1b1..1b98c4ae 100644 --- a/netbox_custom_objects/branching.py +++ b/netbox_custom_objects/branching.py @@ -5,19 +5,36 @@ def supports_branching_resolver(model): - """Mark CustomObject M2M through models as branchable. + """Branching support resolver for Custom Object models. - Through models are plain ``models.Model`` subclasses (no ChangeLoggingMixin), - so the default heuristic would route their queries to main even inside a - branch — and the FK to the parent CO would resolve against main's rows. - Returning ``True`` pulls them into the branch connection routing. + Two cases handled: + + 1. **M2M through models** — plain ``models.Model`` subclasses with no + ``ChangeLoggingMixin``, so the default heuristic would route their + queries to main even inside a branch. Return ``True`` to pull them + into branch routing (their parent CO FK must resolve within the branch). + + 2. **Non-changelog COT models** — when ``changelog_enabled=False`` on the + parent ``CustomObjectType``, the model has no change tracking and must + not be isolated to a branch schema. Objects created in a branch context + should land in main (always visible regardless of active branch), matching + the high-frequency / non-audited use case this flag is designed for. + Return ``False`` to route all reads/writes to the default DB. """ meta = getattr(model, '_meta', None) if meta is None or meta.app_label != 'netbox_custom_objects': return None name = meta.model_name or '' + + # M2M through models: opt in to branching. if name.startswith('through_custom_objects_'): return True + + # Generated COT models: opt out of branching when changelog is disabled. + cot = getattr(model, 'custom_object_type', None) + if cot is not None and not getattr(cot, 'changelog_enabled', True): + return False + return None diff --git a/netbox_custom_objects/filtersets.py b/netbox_custom_objects/filtersets.py index da96dbd0..39fabb48 100644 --- a/netbox_custom_objects/filtersets.py +++ b/netbox_custom_objects/filtersets.py @@ -263,6 +263,7 @@ class Meta: "id", "name", "group_name", + "changelog_enabled", ) diff --git a/netbox_custom_objects/forms.py b/netbox_custom_objects/forms.py index d029ba70..0e68e8a7 100644 --- a/netbox_custom_objects/forms.py +++ b/netbox_custom_objects/forms.py @@ -61,6 +61,7 @@ class CustomObjectTypeForm(NetBoxModelForm): "name", "verbose_name", "verbose_name_plural", "slug", "version", "description", "group_name", "tags", ), + FieldSet("changelog_enabled", name=_("Options")), ) comments = CommentField() @@ -68,9 +69,20 @@ class Meta: model = CustomObjectType fields = ( "name", "verbose_name", "verbose_name_plural", "slug", "version", "description", - "group_name", "comments", "tags", + "group_name", "changelog_enabled", "comments", "tags", ) + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + if self.instance and self.instance.pk: + # changelog_enabled is locked after creation to prevent mid-lifecycle + # branching inconsistencies (objects already in a branch, partial + # changelog entries, etc.). + self.fields['changelog_enabled'].disabled = True + self.fields['changelog_enabled'].help_text = _( + "Cannot be changed after creation." + ) + class CustomObjectTypeBulkEditForm(NetBoxModelBulkEditForm): description = forms.CharField( diff --git a/netbox_custom_objects/migrations/0015_customobjecttype_changelog_enabled.py b/netbox_custom_objects/migrations/0015_customobjecttype_changelog_enabled.py new file mode 100644 index 00000000..19df5447 --- /dev/null +++ b/netbox_custom_objects/migrations/0015_customobjecttype_changelog_enabled.py @@ -0,0 +1,18 @@ +# Generated by Django 6.0.5 on 2026-06-12 01:23 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('netbox_custom_objects', '0014_fix_mixed_case_field_names'), + ] + + operations = [ + migrations.AddField( + model_name='customobjecttype', + name='changelog_enabled', + field=models.BooleanField(default=True), + ), + ] diff --git a/netbox_custom_objects/models.py b/netbox_custom_objects/models.py index e802b691..40f99c71 100644 --- a/netbox_custom_objects/models.py +++ b/netbox_custom_objects/models.py @@ -1028,6 +1028,17 @@ class CustomObjectType(NetBoxModel): blank=True, help_text=_("Used to group similar custom object types in the navigation menu") ) + changelog_enabled = models.BooleanField( + verbose_name=_('changelog enabled'), + default=True, + help_text=_( + "If disabled, changes to objects of this type will not be recorded in the changelog. " + "Useful for high-frequency updates where audit history is not required. " + "Note: disabling changelog also exempts objects of this type from branch isolation — " + "they are always written to the main database regardless of the active branch. " + "This setting cannot be changed after the Custom Object Type is created." + ), + ) schema_document = models.JSONField( blank=True, null=True, @@ -1543,6 +1554,22 @@ def get_model( "custom_object_type_id": self.id, } + # If changelog is disabled for this COT, override to_objectchange() so + # that NetBox's change-logging signal skips writing ObjectChange rows + # for create/update operations. + # + # Delete is intentionally allowed through: core/signals.handle_deleted_object + # calls objectchange.user = ... unconditionally (no None guard), so returning + # None for deletes would crash on deletion. Deletes are also rare and worth + # preserving for audit purposes. The high-frequency use case this flag targets + # is create/update (e.g. nightly fleet scans), not deletion. + if not self.changelog_enabled: + def _no_changelog_to_objectchange(_self, _action): + if _action == ObjectChangeActionChoices.ACTION_DELETE: + return ChangeLoggingMixin.to_objectchange(_self, _action) + return None + attrs["to_objectchange"] = _no_changelog_to_objectchange + # Pass the generating models set to field generation fields = [] field_attrs = self._fetch_and_generate_field_attrs( @@ -1776,7 +1803,10 @@ def create_model(self): self.object_type.public = True self.object_type.save() - with _get_schema_connection().schema_editor() as schema_editor: + # Non-changelog COTs bypass branch isolation; their table must live in main + # so that queries (which always route to main for these models) can find it. + schema_conn = connection if not self.changelog_enabled else _get_schema_connection() + with schema_conn.schema_editor() as schema_editor: schema_editor.create_model(model) self._store_base_column_snapshot(model) @@ -1845,6 +1875,12 @@ def clean_fields(self, exclude=None): def save(self, *args, **kwargs): needs_db_create = self._state.adding + # Non-changelog COTs are paired with CO tables in main (COs bypass branch + # isolation). Route the COT record to main as well so the two are consistent. + if not self.changelog_enabled and 'using' not in kwargs: + if _get_schema_connection() is not connection: + kwargs['using'] = 'default' + super().save(*args, **kwargs) if needs_db_create: diff --git a/netbox_custom_objects/tables.py b/netbox_custom_objects/tables.py index 7f01b93b..cd5902ed 100644 --- a/netbox_custom_objects/tables.py +++ b/netbox_custom_objects/tables.py @@ -73,6 +73,7 @@ class Meta(NetBoxTable.Meta): "verbose_name_plural", "slug", 'description', + "changelog_enabled", 'comments', 'tags', "created", diff --git a/netbox_custom_objects/templates/netbox_custom_objects/customobject.html b/netbox_custom_objects/templates/netbox_custom_objects/customobject.html index c7315a4f..e6c93869 100644 --- a/netbox_custom_objects/templates/netbox_custom_objects/customobject.html +++ b/netbox_custom_objects/templates/netbox_custom_objects/customobject.html @@ -77,9 +77,11 @@ + {% if object.custom_object_type.changelog_enabled %} + {% endif %} {% endblock tabs %} diff --git a/netbox_custom_objects/templates/netbox_custom_objects/customobjecttype.html b/netbox_custom_objects/templates/netbox_custom_objects/customobjecttype.html index a6724902..7b9325e6 100644 --- a/netbox_custom_objects/templates/netbox_custom_objects/customobjecttype.html +++ b/netbox_custom_objects/templates/netbox_custom_objects/customobjecttype.html @@ -37,6 +37,10 @@
{% trans "Custom Object Type" %}
{% trans "Description" %} {{ object.description|placeholder }} + + {% trans "Changelog" %} + {% checkmark object.changelog_enabled %} + {% trans "Last activity" %} diff --git a/netbox_custom_objects/tests/test_api.py b/netbox_custom_objects/tests/test_api.py index f96bb179..93401c6b 100644 --- a/netbox_custom_objects/tests/test_api.py +++ b/netbox_custom_objects/tests/test_api.py @@ -4,7 +4,7 @@ from django.test import TestCase, RequestFactory from django.urls import reverse -from utilities.testing import create_test_user +from utilities.testing import TestCase as NetBoxTestCase, create_test_user from rest_framework import status from rest_framework.test import APIClient @@ -13,6 +13,7 @@ from .base import CustomObjectsTestCase, create_token from core.models import ObjectType from dcim.models import Device, DeviceRole, DeviceType, Manufacturer, Rack, Site +from extras.models import Tag from users.models import ObjectPermission from virtualization.models import Cluster, ClusterType @@ -98,7 +99,7 @@ def test_delete_object_without_permission(self): self.assertHttpStatus(response, 403) -class CustomObjectTest(CustomObjectsTestCase, CustomObjectAPITestCaseMixin, TestCase): +class CustomObjectTest(CustomObjectsTestCase, CustomObjectAPITestCaseMixin, NetBoxTestCase): model = None # Will be set in setUpTestData bulk_update_data = { 'test_field': 'Updated test field', @@ -371,6 +372,7 @@ def test_create_with_nested_serializers(self): obj_perm.save() obj_perm.users.add(self.user) obj_perm.object_types.add(ObjectType.objects.get_for_model(self.model)) + self.add_permissions('dcim.view_device') devices = Device.objects.all() @@ -398,6 +400,80 @@ def test_create_with_nested_serializers(self): set(data['devices']), ) + def test_create_with_tags_persists_to_db(self): + """Regression #371: tags submitted on POST must be saved to the DB, not just echoed.""" + self._add_permission('add', 'Create with tags perm') + self.add_permissions('extras.view_tag') + tag = Tag.objects.get_or_create(name='api-create-tag', slug='api-create-tag')[0] + + data = { + 'test_field': 'Tagged Object', + 'tags': [{'id': tag.id, 'name': tag.name, 'slug': tag.slug, 'color': tag.color}], + } + response = self.client.post(self._get_list_url(), data, format='json', **self.header) + self.assertHttpStatus(response, status.HTTP_201_CREATED) + self.assertIn('tags', response.data) + self.assertTrue(len(response.data['tags']) > 0, 'Response should include the submitted tag') + + # Fetch fresh from the DB — the critical assertion that caught #371 + instance = self._get_queryset().get(pk=response.data['id']) + self.assertIn(tag.name, list(instance.tags.names()), 'Tag must be persisted to the DB') + + def test_patch_with_tags_persists_to_db(self): + """Regression #371: tags submitted on PATCH must be saved to the DB, not just echoed.""" + self._add_permission('view', 'View perm') + self._add_permission('change', 'Patch with tags perm') + self.add_permissions('extras.view_tag') + tag = Tag.objects.get_or_create(name='api-patch-tag', slug='api-patch-tag')[0] + + instance = self._get_queryset().first() + self.assertEqual(list(instance.tags.names()), [], 'Instance should start with no tags') + + data = {'tags': [{'id': tag.id, 'name': tag.name, 'slug': tag.slug, 'color': tag.color}]} + response = self.client.patch(self._get_detail_url(instance), data, format='json', **self.header) + self.assertHttpStatus(response, status.HTTP_200_OK) + + instance.refresh_from_db() + self.assertIn(tag.name, list(instance.tags.names()), 'Tag must be persisted to the DB after PATCH') + + def test_patch_with_empty_tags_clears_existing(self): + """PATCH with tags=[] must remove all existing tags from the DB.""" + self._add_permission('view', 'View perm') + self._add_permission('change', 'Patch clear tags perm') + tag = Tag.objects.get_or_create(name='api-clear-tag', slug='api-clear-tag')[0] + + instance = self._get_queryset().first() + instance.tags.add(tag.name) + self.assertIn(tag.name, list(instance.tags.names()), 'Pre-condition: tag should be set') + + response = self.client.patch(self._get_detail_url(instance), {'tags': []}, format='json', **self.header) + self.assertHttpStatus(response, status.HTTP_200_OK) + + instance.refresh_from_db() + self.assertEqual(list(instance.tags.names()), [], 'All tags must be cleared after PATCH with tags=[]') + + def test_patch_without_tags_preserves_existing(self): + """PATCH that omits the tags key entirely must leave existing tags unchanged.""" + self._add_permission('view', 'View perm') + self._add_permission('change', 'Patch preserve tags perm') + tag = Tag.objects.get_or_create(name='api-preserve-tag', slug='api-preserve-tag')[0] + + instance = self._get_queryset().first() + instance.tags.add(tag.name) + self.assertIn(tag.name, list(instance.tags.names()), 'Pre-condition: tag should be set') + + response = self.client.patch( + self._get_detail_url(instance), {'test_field': 'updated'}, format='json', **self.header + ) + self.assertHttpStatus(response, status.HTTP_200_OK) + + instance.refresh_from_db() + self.assertIn( + tag.name, + list(instance.tags.names()), + 'Existing tags must be preserved when tags not in PATCH payload', + ) + class LinkedObjectsAPITest(CustomObjectsTestCase, TestCase): """ @@ -1503,6 +1579,7 @@ def _add_perm(self, action, model): def test_patch_updates_cross_cot_m2m_field(self): """#443 – PATCH with a list of target PKs must update the M2M field.""" self._add_perm('change', self.model_source) + self._add_perm('view', self.model_target) # Confirm initial state. self.assertSetEqual( diff --git a/netbox_custom_objects/tests/test_branching.py b/netbox_custom_objects/tests/test_branching.py index f8c8efc3..6a784496 100644 --- a/netbox_custom_objects/tests/test_branching.py +++ b/netbox_custom_objects/tests/test_branching.py @@ -3036,3 +3036,261 @@ def test_main_and_branch_isolated_schema_and_data(self): self.assertEqual( [r['name'] for r in main_again['custom_objects_server_list']], ['main-server'], ) + + +@unittest.skipUnless(HAS_BRANCHING, 'netbox-branching is not installed') +class ChangelogEnabledBranchingTestCase(BranchingTestBase, TransactionTestCase): + """ + Verify the branching behaviour of changelog_enabled=False COTs. + + Key invariants: + - Objects of a non-changelog COT produce no ObjectChange records, so they + are invisible to branching (can't be captured in a branch diff). + - Cross-COT object-field references between changelog and non-changelog + COTs don't cause errors during branch sync/merge/revert. + - Covered: single-object, multi-object, and polymorphic field variants. + """ + + def _make_changelog_cot(self, name, slug): + cot = CustomObjectType.objects.create(name=name, slug=slug, changelog_enabled=True) + CustomObjectTypeField.objects.create( + custom_object_type=cot, name='label', label='Label', + type='text', primary=True, required=True, + ) + return cot + + def _make_nolog_cot(self, name, slug): + cot = CustomObjectType.objects.create(name=name, slug=slug, changelog_enabled=False) + CustomObjectTypeField.objects.create( + custom_object_type=cot, name='label', label='Label', + type='text', primary=True, required=True, + ) + return cot + + def _object_type_for(self, cot): + return ObjectType.objects.get( + app_label='netbox_custom_objects', + model=cot.get_table_model_name(cot.id).lower(), + ) + + def test_nolog_cot_objects_visible_in_main_when_created_in_branch(self): + """ + Regression for Arthur's reported scenario: objects of a non-changelog COT + created while a branch is active must be visible in main, because the + supports_branching_resolver returns False for these models and the DB + router writes them directly to the default schema. + + Steps: + 1. Create 'aaa' in main (no branch active) + 2. Activate a branch + 3. Create 'bbb' while the branch is active + 4. Switch back to main — both 'aaa' and 'bbb' must be visible + """ + nolog = self._make_nolog_cot('VisMain', 'vismain') + model = nolog.get_model() + + with event_tracking(self.request): + model.objects.create(label='aaa') + + branch = _provision_branch('VisMain branch', user=self.user) + branch_request = _make_request(self.user) + with activate_branch(branch), event_tracking(branch_request): + model.objects.create(label='bbb') + + # Back in main: both objects must be visible. + labels = set(model.objects.values_list('label', flat=True)) + self.assertIn('aaa', labels, "Object created in main must be visible in main") + self.assertIn( + 'bbb', labels, + "Object created in branch context for non-changelog COT must be visible " + "in main (branch router exempts it; writes go directly to default DB)", + ) + + def test_nolog_cot_objects_not_captured_in_branch(self): + """ + Objects created inside a branch for a changelog-disabled COT must not + appear in the branch's change diff (no ObjectChange rows). + """ + from core.models import ObjectChange + nolog = self._make_nolog_cot('NoBranch', 'nobranch') + model = nolog.get_model() + + branch = _provision_branch('NoBranch test', user=self.user) + branch_request = _make_request(self.user) + with activate_branch(branch), event_tracking(branch_request): + instance = model.objects.create(label='x') + ct = ContentType.objects.get_for_model(model) + changes = ObjectChange.objects.filter( + changed_object_type=ct, + changed_object_id=instance.pk, + ) + self.assertEqual( + changes.count(), 0, + "Non-changelog COT must not produce ObjectChange rows inside a branch", + ) + + def test_regular_cot_with_fk_to_nolog_cot_merges_cleanly(self): + """ + Regular COT (A) with an object field pointing at non-changelog COT (B). + Creating/editing A objects in a branch (with valid B FK) then merging + must not raise. + """ + cot_b = self._make_nolog_cot('NL_B', 'nl-b') + model_b = cot_b.get_model() + b_obj = model_b.objects.create(label='b-target') + + cot_a = self._make_changelog_cot('CL_A', 'cl-a') + cot_a.refresh_from_db() + CustomObjectTypeField.objects.create( + custom_object_type=cot_a, name='ref_b', label='Ref B', + type='object', related_object_type=self._object_type_for(cot_b), + ) + cot_a.refresh_from_db() + model_a = cot_a.get_model() + + branch = _provision_branch('FKtoNolog', merge_strategy='iterative', user=self.user) + branch_request = _make_request(self.user) + with activate_branch(branch), event_tracking(branch_request): + model_a.objects.create(label='a-obj', ref_b=b_obj) + + branch.refresh_from_db() + branch.merge(user=self.user, commit=True) + # Merge succeeded — assert the record exists on main. + self.assertTrue(model_a.objects.filter(label='a-obj').exists()) + + def test_nolog_cot_with_fk_to_regular_cot_does_not_break_branch(self): + """ + Non-changelog COT (B) with an object field pointing at regular COT (A). + Modifications to A objects (tracked by branch) must not be disrupted by + untracked B objects referencing them. + """ + cot_a = self._make_changelog_cot('CL_A2', 'cl-a2') + model_a = cot_a.get_model() + + cot_b = self._make_nolog_cot('NL_B2', 'nl-b2') + cot_b.refresh_from_db() + CustomObjectTypeField.objects.create( + custom_object_type=cot_b, name='ref_a', label='Ref A', + type='object', related_object_type=self._object_type_for(cot_a), + ) + cot_b.refresh_from_db() + model_b = cot_b.get_model() + + a_obj = model_a.objects.create(label='a-main') + with event_tracking(self.request): + model_b.objects.create(label='b-untracked', ref_a=a_obj) + + branch = _provision_branch('NologFKtoRegular', merge_strategy='iterative', user=self.user) + branch_request = _make_request(self.user) + with activate_branch(branch), event_tracking(branch_request): + # Edit the changelog A object inside the branch. + branch_a = model_a.objects.get(pk=a_obj.pk) + branch_a.label = 'a-edited' + branch_a.save() + + branch.refresh_from_db() + branch.merge(user=self.user, commit=True) + a_obj.refresh_from_db() + self.assertEqual(a_obj.label, 'a-edited') + + def test_multiobject_field_regular_cot_to_nolog_cot_merges_cleanly(self): + """ + Regular COT (A) with a multi-object field → non-changelog COT (B). + """ + cot_b = self._make_nolog_cot('NL_BM', 'nl-bm') + model_b = cot_b.get_model() + b1 = model_b.objects.create(label='b1') + b2 = model_b.objects.create(label='b2') + + cot_a = self._make_changelog_cot('CL_AM', 'cl-am') + cot_a.refresh_from_db() + CustomObjectTypeField.objects.create( + custom_object_type=cot_a, name='refs_b', label='Refs B', + type='multiobject', related_object_type=self._object_type_for(cot_b), + ) + cot_a.refresh_from_db() + model_a = cot_a.get_model() + + branch = _provision_branch('M2MtoNolog', merge_strategy='iterative', user=self.user) + branch_request = _make_request(self.user) + with activate_branch(branch), event_tracking(branch_request): + a_obj = model_a.objects.create(label='a-m2m') + a_obj.refs_b.set([b1, b2]) + + branch.refresh_from_db() + branch.merge(user=self.user, commit=True) + self.assertTrue(model_a.objects.filter(label='a-m2m').exists()) + + def test_polymorphic_object_field_regular_cot_to_nolog_cot_merges_cleanly(self): + """ + Regular COT (A) with a polymorphic object field that includes a + non-changelog COT (B) as one of the allowed types. + """ + cot_b = self._make_nolog_cot('NL_BP', 'nl-bp') + model_b = cot_b.get_model() + b_obj = model_b.objects.create(label='bp-target') + + cot_a = self._make_changelog_cot('CL_AP', 'cl-ap') + cot_a.refresh_from_db() + poly_field = CustomObjectTypeField.objects.create( + custom_object_type=cot_a, name='poly_ref', label='Poly Ref', + type='object', is_polymorphic=True, + ) + poly_field.related_object_types.add( + self._object_type_for(cot_b), + ObjectType.objects.get(app_label='dcim', model='site'), + ) + cot_a.refresh_from_db() + model_a = cot_a.get_model() + + branch = _provision_branch('PolytoNolog', merge_strategy='iterative', user=self.user) + branch_request = _make_request(self.user) + with activate_branch(branch), event_tracking(branch_request): + a_obj = model_a.objects.create(label='a-poly') + # Set the polymorphic GFK to point at the non-changelog target. + ct = ContentType.objects.get_for_model(model_b) + a_obj.__class__.objects.filter(pk=a_obj.pk).update( + poly_ref_content_type_id=ct.pk, + poly_ref_object_id=b_obj.pk, + ) + + branch.refresh_from_db() + branch.merge(user=self.user, commit=True) + self.assertTrue(model_a.objects.filter(label='a-poly').exists()) + + def test_nolog_cot_created_in_branch_lands_in_main(self): + """ + Regression for PR #574 review: creating a COT with changelog_enabled=False + while a branch is active must not crash. + + Before the fix: + - CustomObjectType.save() stored the COT record in the branch schema. + - create_model() used _get_schema_connection() → CO table created in + the branch schema. + - The detail view then issued model.objects.all(), which + supports_branching_resolver routes to main; the table didn't exist + there, causing ProgrammingError: relation "custom_objects_N" does + not exist. + + After the fix, both the COT record and the CO table land in main so + queries always resolve correctly regardless of the active branch. + """ + branch = _provision_branch('NologInBranch', user=self.user) + branch_request = _make_request(self.user) + with activate_branch(branch), event_tracking(branch_request): + # Creating the COT while in the branch must not crash. + cot = CustomObjectType.objects.create( + name='NologInBranch', slug='nolog-in-branch', + changelog_enabled=False, + ) + + # The CO table is in main — querying it must not raise ProgrammingError. + model = cot.get_model() + count = model.objects.count() + self.assertEqual(count, 0, "Freshly created non-changelog COT must have no objects") + + # The COT record itself is accessible from main (not hidden in the branch). + self.assertTrue( + CustomObjectType.objects.filter(pk=cot.pk).exists(), + "COT with changelog_enabled=False created in a branch must be visible in main", + ) diff --git a/netbox_custom_objects/tests/test_models.py b/netbox_custom_objects/tests/test_models.py index 9e1df4ed..e885cc8d 100644 --- a/netbox_custom_objects/tests/test_models.py +++ b/netbox_custom_objects/tests/test_models.py @@ -2262,3 +2262,181 @@ def test_child_serializer_does_not_clobber_parent_serializer(self): "building child serializer must not clobber parent's full serializer") self.assertIn("title", registered_parent.Meta.fields, "parent's full field set must be intact after child serializer is built") + + +class ChangelogEnabledTestCase(CustomObjectsTestCase, TestCase): + """ + Tests for the changelog_enabled flag on CustomObjectType. + + Verifies that to_objectchange() is suppressed when changelog_enabled=False, + and that the generated model still works normally when it is True. + """ + + def _make_request(self): + import uuid + from django.test import RequestFactory + from django.urls import reverse + request = RequestFactory().get(reverse('home')) + request.id = uuid.uuid4() + request.user = self.user + return request + + def test_changelog_disabled_suppresses_objectchange(self): + """ + Saving a custom object instance must produce zero ObjectChange rows when + the parent COT has changelog_enabled=False. + """ + from core.models import ObjectChange + from netbox.context_managers import event_tracking + + cot = self.create_simple_custom_object_type( + name="NoLog", slug="nolog", changelog_enabled=False, + ) + model = cot.get_model() + request = self._make_request() + + with event_tracking(request): + instance = model.objects.create(name="nolog-obj") + + ct = ContentType.objects.get_for_model(model) + self.assertEqual( + ObjectChange.objects.filter( + changed_object_type=ct, changed_object_id=instance.pk, + ).count(), + 0, + "ObjectChange must not be written when changelog_enabled=False", + ) + + def test_changelog_enabled_writes_objectchange(self): + """ + Sanity check: ObjectChange rows are still written when changelog_enabled=True + (the default), so the disabled test is not trivially passing due to a broken + signal setup. + """ + from core.models import ObjectChange + from netbox.context_managers import event_tracking + + cot = self.create_simple_custom_object_type( + name="WithLog", slug="withlog", changelog_enabled=True, + ) + model = cot.get_model() + request = self._make_request() + + with event_tracking(request): + instance = model.objects.create(name="withlog-obj") + + ct = ContentType.objects.get_for_model(model) + self.assertGreater( + ObjectChange.objects.filter( + changed_object_type=ct, changed_object_id=instance.pk, + ).count(), + 0, + "ObjectChange must be written when changelog_enabled=True", + ) + + def test_to_objectchange_returns_none_for_create_update_when_disabled(self): + """ + The injected to_objectchange() override must return None for create/update + actions but not for delete (the delete signal handler has no None guard). + """ + from core.choices import ObjectChangeActionChoices + cot = self.create_simple_custom_object_type( + name="DirectCheck", slug="directcheck", changelog_enabled=False, + ) + model = cot.get_model() + instance = model(name="x") + self.assertIsNone(instance.to_objectchange(ObjectChangeActionChoices.ACTION_CREATE)) + self.assertIsNone(instance.to_objectchange(ObjectChangeActionChoices.ACTION_UPDATE)) + # DELETE must not return None — the delete signal handler calls + # objectchange.user = ... unconditionally and would crash. + self.assertIsNotNone(instance.to_objectchange(ObjectChangeActionChoices.ACTION_DELETE)) + + def test_mid_lifecycle_toggle_disables_changelog(self): + """ + Toggling changelog_enabled from True → False on an existing COT must + suppress ObjectChange rows for subsequent saves. Verifies that the + cache_timestamp bump on save() causes get_model() to regenerate the + model with the updated override injected. + """ + from core.models import ObjectChange + from netbox.context_managers import event_tracking + + cot = self.create_simple_custom_object_type( + name="Toggle", slug="toggle", changelog_enabled=True, + ) + model = cot.get_model() + request = self._make_request() + ct = ContentType.objects.get_for_model(model) + + # Phase 1: changelog enabled — ObjectChange rows must be written. + with event_tracking(request): + obj_before = model.objects.create(name="before-toggle") + self.assertGreater( + ObjectChange.objects.filter(changed_object_type=ct, changed_object_id=obj_before.pk).count(), + 0, + "ObjectChange must be written before the toggle", + ) + + # Toggle off — saving the COT bumps cache_timestamp, which forces + # the next get_model() call to regenerate with the disabled override. + cot.changelog_enabled = False + cot.save() + + # Regenerate the model (as production code does on the next request). + model = cot.get_model() + + # Phase 2: changelog disabled — no ObjectChange rows for new saves. + with event_tracking(request): + obj_after = model.objects.create(name="after-toggle") + self.assertEqual( + ObjectChange.objects.filter(changed_object_type=ct, changed_object_id=obj_after.pk).count(), + 0, + "ObjectChange must not be written after toggling changelog_enabled=False", + ) + + +class ChangelogEnabledImmutabilityTestCase(CustomObjectsTestCase, TestCase): + """ + changelog_enabled is locked after creation. Tests verify that the API + serializer rejects changes and the form disables the field on edit. + """ + + def test_api_serializer_rejects_change_after_creation(self): + from netbox_custom_objects.api.serializers import CustomObjectTypeSerializer + from rest_framework.exceptions import ValidationError as DRFValidationError + + cot = self.create_simple_custom_object_type(name="Locked", slug="locked", changelog_enabled=True) + serializer = CustomObjectTypeSerializer(instance=cot, data={'changelog_enabled': False}, partial=True) + with self.assertRaises(DRFValidationError): + serializer.is_valid(raise_exception=True) + + def test_api_serializer_allows_same_value_on_update(self): + from netbox_custom_objects.api.serializers import CustomObjectTypeSerializer + + cot = self.create_simple_custom_object_type(name="sameval", slug="sameval", changelog_enabled=True) + serializer = CustomObjectTypeSerializer(instance=cot, data={'changelog_enabled': True}, partial=True) + self.assertTrue(serializer.is_valid(), serializer.errors) + + def test_api_serializer_allows_setting_on_creation(self): + from netbox_custom_objects.api.serializers import CustomObjectTypeSerializer + + data = { + 'name': 'newcot', + 'slug': 'newcot', + 'changelog_enabled': False, + } + serializer = CustomObjectTypeSerializer(data=data) + self.assertTrue(serializer.is_valid(), serializer.errors) + + def test_form_disables_field_on_edit(self): + from netbox_custom_objects.forms import CustomObjectTypeForm + + cot = self.create_simple_custom_object_type(name="FormLock", slug="formlock", changelog_enabled=True) + form = CustomObjectTypeForm(instance=cot) + self.assertTrue(form.fields['changelog_enabled'].disabled) + + def test_form_field_enabled_on_create(self): + from netbox_custom_objects.forms import CustomObjectTypeForm + + form = CustomObjectTypeForm() + self.assertFalse(form.fields['changelog_enabled'].disabled) diff --git a/netbox_custom_objects/tests/test_views.py b/netbox_custom_objects/tests/test_views.py index 6a798f48..bfe6f41a 100644 --- a/netbox_custom_objects/tests/test_views.py +++ b/netbox_custom_objects/tests/test_views.py @@ -983,3 +983,46 @@ def test_quick_add_post_validation_failure_rerenders(self): # No new object created. model = self.target_cot.get_model() self.assertFalse(model.objects.exists()) + + +class ChangelogEnabledViewTestCase(CustomObjectsTestCase, TestCase): + """Tests for changelog_enabled UI behaviour on custom object instances.""" + + def setUp(self): + super().setUp() + # Superuser so permission checks don't interfere with template-content assertions. + self.user.is_superuser = True + self.user.save() + self.cot_disabled = self.create_simple_custom_object_type( + name="NoLog", slug="nolog-view", changelog_enabled=False, + ) + self.instance = self.cot_disabled.get_model().objects.create(name="obj1") + + def test_changelog_view_returns_404_when_disabled(self): + """ + Hitting the changelog URL for a COT with changelog_enabled=False must + return 404 rather than an empty page. + """ + url = reverse( + 'plugins:netbox_custom_objects:customobject_changelog', + kwargs={'custom_object_type': self.cot_disabled.slug, 'pk': self.instance.pk}, + ) + response = self.client.get(url) + self.assertEqual(response.status_code, 404) + + def test_changelog_tab_absent_in_template_when_disabled(self): + """ + The detail view for a custom object whose COT has changelog_enabled=False + must not render a Changelog tab link. + """ + url = reverse( + 'plugins:netbox_custom_objects:customobject', + kwargs={'custom_object_type': self.cot_disabled.slug, 'pk': self.instance.pk}, + ) + response = self.client.get(url) + self.assertEqual(response.status_code, 200) + self.assertNotContains( + response, + 'customobject_changelog', + msg_prefix="Changelog tab link must be absent when changelog is disabled", + ) diff --git a/netbox_custom_objects/views.py b/netbox_custom_objects/views.py index bfb09101..80629b08 100644 --- a/netbox_custom_objects/views.py +++ b/netbox_custom_objects/views.py @@ -8,6 +8,7 @@ from django.contrib.contenttypes.models import ContentType from django.db import router, transaction from django.db.models import ProtectedError, Q, RestrictedError +from django.http import Http404 from django.shortcuts import get_object_or_404, redirect, render from django.urls import reverse from django.utils.html import escape @@ -1453,6 +1454,11 @@ def get(self, request, custom_object_type, **kwargs): object_type = get_object_or_404( CustomObjectType, slug=custom_object_type ) + + # Changelog is disabled for this COT — return 404 rather than an empty page. + if not object_type.changelog_enabled: + raise Http404 + model = object_type.get_model_with_serializer() # Get the specific object