Skip to content

Conversation

@bctiemann
Copy link
Contributor

@bctiemann bctiemann commented Apr 24, 2025

Fixes: #18717

The handle_deleted_object signal handler ensures that change records are saved for the object being deleted, but also manually triggers a m2m_changed signal to ensure that related objects through many-to-many relationships are also change-logged through their own signal handlers. However, we do not handle the case where the related object is linked to the deleted object through a many-to-one relationship.

This change handles ManyToOneRel fields in the same area of code, by resolving the related object in the same way as with M2M fields, but instead setting the related object's field value to None and saving, thus ensuring a change record is created for the related object.

Note the check on L164 which ensures the related field is null=True. This is primarily to ensure that unit tests pass, as without it one of the tests fails due to trying to set a non-nullable field to None. However I think in practice this will not happen as in the case of any FK relationship where null=False, trying to delete an object which is pointed to by a related object via a many-to-one relationship will be caught by object-level validation before the deleted object handler is ever called.

@bctiemann bctiemann requested review from a team and jnovinger and removed request for a team April 24, 2025 14:50
@bctiemann bctiemann marked this pull request as draft April 24, 2025 14:59
@bctiemann bctiemann marked this pull request as ready for review April 24, 2025 16:54
Copy link
Member

@jnovinger jnovinger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Spent some time thinking on this and trying to poke holes in it, but I got nothing.

I think in an ideal world, we'd have a test suite for handle_deleted_object and could add tests to make sure this behaves as expected. But, that seems out of scope for this fix.

@jnovinger jnovinger merged commit bee004f into main Apr 24, 2025
10 checks passed
@jnovinger jnovinger deleted the 18717-trigger-object-change-on-related-many-to-one-field branch April 24, 2025 19:24
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 25, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

DCIM Interface: ObjectChange missing for deleted untagged VLANs

3 participants