Fixes: #558 - Avoid "<type> None" in CustomObject.__str__ post-delete#528
Open
Kani999 wants to merge 1 commit into
Open
Fixes: #558 - Avoid "<type> None" in CustomObject.__str__ post-delete#528Kani999 wants to merge 1 commit into
Kani999 wants to merge 1 commit into
Conversation
Django's Model.delete() sets self.pk to None after the DB row is gone.
NetBox's delete-toast and changelog renderers call str() on the
post-delete instance, so when the COT has no primary=True field the
previous output included a literal "None":
Deleted SmokeTabTarget SmokeTabTarget None
Branch on self.pk: when None, return just the COT's display name
("SmokeTabTarget"); when set, behave as before ("SmokeTabTarget 5").
Observed during the all-in-one smoke run on krupa.vm.cesnet.cz on
2026-05-26 while validating PR netboxlabs#482's related-tabs work.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Contributor
|
Hi @Kani999 , as with the other PR just now, please don't open a PR unless there is an accepted bug to link it to (this is for later bookkeeping/forensics). Please open a bug issue with repro steps and any relevant details according to the template, and then once it's triaged and accepted we can reopen this PR. Thanks! |
Contributor
|
Two small suggestions: 1. Trim the comment (project convention is one short line max): if self.pk is None: # post-delete: pk is cleared by Django's Model.delete()
return str(self.custom_object_type.display_name)2. Regression test in def test_str_post_delete_no_primary_field_does_not_contain_none(self):
"""Regression #558: str() on a post-delete instance (pk=None, no primary
field value) must return the COT display name, not '<type> None'."""
cot = self.create_custom_object_type(
name="DeleteStrTest", slug="delete-str-test",
verbose_name_plural="Delete Str Tests",
)
# No primary field — falls through to the pk fallback path in __str__.
model = cot.get_model()
instance = model.objects.create()
instance.delete() # sets instance.pk = None
result = str(instance)
self.assertNotIn("None", result)
self.assertEqual(result, cot.display_name)Both verified locally — test passes with the fix and fails on the unpatched code. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes: #558
Summary
CustomObject.__str__includesself.idin its fallback output when no field on the COT is markedprimary=True. But Django'sModel.delete()setsself.pktoNoneafter the database row is removed — and NetBox's delete-toast and changelog renderers callstr()on the post-delete instance. The result is a literal "None" in the user-facing message:Fix
Branch on
self.pkinside the no-primary-field path:self.pk is None(post-delete) → return just the COT's display name ("SmokeTabTarget").self.pkis set (normal rendering) → behave as before ("SmokeTabTarget 5").8 lines, no behavioural change for any code path with a live
pk.Verification
Observed while smoke-testing PR #482's related-tabs work.
Context
Surfaced during the related-object-tabs smoke testing (PR #482); independent of that work and easy enough to land standalone.