-
Couldn't load subscription status.
- Fork 2
fix(bulk-import): lookup custom object via slug #252
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(bulk-import): lookup custom object via slug #252
Conversation
custom objects are indexed via the view's url, which is derived from the custom object's slug. the existing bulk import form looked up the custom object by name though, rather than slug. this commit fixes that bad lookup field. the bulk import and bulk delete forms already do this. additionally, we add a slug uniqueness constraint on the custom object type for additional safeguarding
|
Thanks @anshulkamath . Sanity check, does this need a migration? |
|
@bctiemann great catch. I've updated the PR with the migration |
netbox_custom_objects/models.py
Outdated
| "A Custom Object Type with this name already exists." | ||
| ), | ||
| ), | ||
| models.UniqueConstraint( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this necessary if we already have unique=True on the slug field? I believe it's implicit unless you really want to override the error message.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I missed that one. Overriding is not necessary then.
| @@ -0,0 +1,20 @@ | |||
| # Generated by Django 5.2.7 on 2025-10-20 13:24 | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this UniqueConstraint is necessary (I don't think it is), could you please rename this migration to something a bit more informative/less unwieldy (following the usual pattern in NetBox core migrations)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed the unique constraint
Fixes: #251
custom objects are indexed via the view's url, which is derived from the custom object's slug. the existing bulk import form looked up the custom object by name though, rather than slug. this commit fixes that bad lookup field. the bulk import and bulk delete forms already do this.
additionally, we add a slug uniqueness constraint on the custom object type for additional safeguarding