Skip to content

Closes #673: Support conditional unique constraints on Custom Object Types - #830

Merged
jnovinger merged 3 commits into
featurefrom
673-conditional-unique-constraints
Oct 16, 2026
Merged

jnovinger merged 3 commits into
featurefrom
673-conditional-unique-constraints

Conversation

@bctiemann

@bctiemann bctiemann commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

Closes: #673

Adds an optional condition to Custom Object Type unique constraints. A constraint with a condition becomes a partial unique index in PostgreSQL, so uniqueness applies only to the rows that match the condition.

Condition format

The condition is a list of predicates that are all ANDed together:

[{"field": <field>, "lookup": "exact" | "n" | "in" | "isnull", "value": ...}]
  • Storage / portable schema: each field is referenced by its schema ID, so renaming a field doesn't break the condition.
  • REST API: writes take field PKs; reads return brief field objects.
  • UI form: fields are entered by name and converted to schema IDs.
  • Lookups:
    • exact means equals.
    • n means "does not equal". It also matches empty values, because a plain NOT (col = x) would treat NULL as not matching.
    • in means "is one of" and needs a non-empty list.
    • isnull means "is empty" and needs a boolean.
  • Object / multi-object fields only support isnull.
  • Values are run through the field's to_python(). Select values must be valid choices.
  • Single-field constraints are allowed when the constraint has a condition. Example: "serial is unique among active rows".

Examples

The examples use the UI form's syntax, with fields named. Through the REST API, field is the field's ID instead, e.g. {"field": 43, ...}.

Serial numbers must be unique among non-retired devices. Fields: serial.

[{"field": "status", "lookup": "n", "value": "retired"}]

The constraint also covers objects whose status is empty, and the table shows it as "Status does not equal retired (or is empty)".

Only one primary contact per site. Fields: site.

[{"field": "is_primary", "lookup": "exact", "value": true}]

Hostnames must be unique among live environments. Fields: hostname.

[{"field": "environment", "lookup": "in", "value": ["production", "staging"]}]

Top-level names must be unique; nested ones only within their parent. This takes two constraints.

  • Fields name, with condition [{"field": "parent", "lookup": "isnull", "value": true}]
  • Fields parent, name, with no condition

Combined predicates (ANDed): the asset tag must be unique among assigned, active assets. Fields: asset_tag.

[
  {"field": "assigned_to", "lookup": "isnull", "value": false},
  {"field": "status", "lookup": "exact", "value": "active"}
]

Things that are rejected:

  • {"field": "parent", "lookup": "exact", "value": 5}: object fields can only be tested with isnull.
  • {"field": "status", "lookup": "in", "value": []}: in needs a non-empty list.
  • {"field": "status", "lookup": "exact", "value": "bogus"}: a select field's value must be one of its choices.
  • {"field": "status", "lookup": "isnull"}: isnull needs true or false, and every other lookup needs a value too.
  • {"field": "tags", "lookup": "isnull", "value": true}, where tags is a multi-object field: conditions can only test the field types that constraints can use, and multi-object fields aren't one of them.

Integration

  • A field used in a condition counts as part of the constraint (CustomObjectTypeField.constraints). The existing protections therefore cover condition fields too:
    • the guard against changing the field's type
    • cascading delete when the field is deleted
    • the schema executor's guard
  • Branching tracks field references made inside conditions.
  • A condition change counts as a definition change, so the DB constraint is rebuilt. The usual check for existing rows that violate the new constraint also runs.
  • The portable schema format, its JSON Schema, the exporter, the comparator and the executor all support condition.
  • The constraint table has a new Condition column, e.g. "Status does not equal retired".
  • Migration 0020 adds the condition JSONField (default []).

Tests

New cases cover:

  • model validation of conditions
  • enforcement for each lookup
  • type-change and delete protection for condition fields
  • API create, plus the check that condition fields belong to the type
  • schema format and executor round trip and alter
  • branching merge and revert
  • view-level conversion between field names and schema IDs

Full suites pass:

  • without branching: 1465 tests
  • with branching: 1538 tests

🤖 Generated with Claude Code

…Types

A constraint can now have a condition: a list of {field, lookup, value}
predicates that must all hold for the constraint to apply, enforced as a
partial unique index. Lookups are exact, n, in and isnull; object fields
can only be tested for emptiness. A conditional constraint may cover a
single field.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@bctiemann

Copy link
Copy Markdown
Contributor Author

@claude /review and report only on actionable issues.

@github-actions

github-actions Bot commented Oct 12, 2026 •

Copy link
Copy Markdown

Claude finished @bctiemann's task in 3m 38s —— View job


PR Review: Conditional Unique Constraints

  • Read changed files
  • Analyze models and validation logic
  • Analyze API serializers
  • Analyze tests
  • Post review findings

Overall this is a solid, well-tested implementation. Below are the actionable issues found, ordered by severity.


1. ConstraintConditionField.to_internal_value only validates "field" presence, not "lookup"

netbox_custom_objects/api/serializers.py line 442

if not all(isinstance(p, dict) and "field" in p for p in data):
    raise ValidationError(_("Each predicate needs a field ID."))

A predicate like {"field": 1} (missing "lookup") passes this check, resolves the field FK, and returns {"_condition": [{"field": <FieldObject>}]}. The validate() method then writes {"field": field.schema_id} — no "lookup" key — to the model, which fails model clean() with a generic internal error rather than a clean serializer-level message. Fix: also require "lookup" in the initial predicate check, similar to how the model does it.

Fix this →


2. _cot_fields() called twice per form validation

netbox_custom_objects/forms.py lines 501 and 518

__init__ calls self._cot_fields() to translate schema IDs → names for the initial display, and clean_condition calls it again to translate names → schema IDs. Both are CustomObjectTypeField.objects.filter(...) queries against the same data. A simple memoization (or combining the two passes) would eliminate the duplicate query.

def _cot_fields(self):
    # Called separately in __init__ and clean_condition — two DB hits per form load+submit.
    return CustomObjectTypeField.objects.filter(custom_object_type_id=self.instance.custom_object_type_id)

Fix this →


3. form.clean_condition passes extra predicate keys through unchecked

netbox_custom_objects/forms.py line 524

return [{**p, "field": schema_ids[p["field"]]} for p in condition]

The dict spread {**p, ...} carries any extra keys the user submits (e.g. {"field": "status", "lookup": "exact", "value": "active", "not": true}). The model _clean_condition catches this — predicate.keys() - {"field", "lookup", "value"} — but the resulting error surfaces as a model-level ValidationError rather than a fieldset-level form error. For consistency with how the API handles this, the form's clean_condition could strip or reject unknown keys explicitly.


4. NOT_EQUAL ("n") matching NULL rows is non-obvious — consider UI note

netbox_custom_objects/models.py lines 4363–4365

if lookup == ConstraintLookupChoices.NOT_EQUAL:
    # Spell out that empty values match; full_clean() evaluates NOT (NULL = x) as false.
    q &= ~Q(**{column: value}) | Q(**{f"{column}__isnull": True})

This is correct for portability — a plain NOT (status = 'retired') in a partial index excludes NULL rows, making them unconstrained. Including OR status IS NULL ensures NULL rows are also covered. The PR description explains this, but the get_condition_display() output ("Status does not equal retired") doesn't communicate that NULL rows are also constrained. Users who expect NULL rows to be outside the constraint will be confused. Consider displaying it as "Status does not equal retired (including empty)" or adding a note in the UI help text on the form field.


5. _condition_column for polymorphic fields uses only _object_id

netbox_custom_objects/models.py lines 4217–4218

if field.is_polymorphic:
    return f"{field.name}_object_id"

The partial index condition for a polymorphic field becomes WHERE {field}_object_id IS NULL/NOT NULL, omitting the paired _content_type_id column. Django's GFK always sets both columns together, so this is practically safe. However, it's asymmetric with _constraint_columns which returns both content_type and object_id for coverage. If _condition_column is used for anything beyond isnull checks in the future this will need revisiting.


No blocking issues. Issues 1 and 3 are the most straightforward to fix before merge.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@bctiemann

Copy link
Copy Markdown
Contributor Author

Thanks for the review. I changed item 4 in 08ef703; the other four don't need changes:

  1. Missing lookup in the API. The model's clean() already catches this and returns a 400 keyed on condition: "Each predicate needs a field and a lookup, and may have only a value besides." It runs before any code reads lookup, so adding the same check to the serializer would only duplicate it.
  2. _cot_fields() querying twice. __init__ calls it only for an unbound form (GET) and clean_condition only for a bound one (POST), so each request makes one query. Caching wouldn't save any.
  3. Extra predicate keys in the form. The model rejects them with a ValidationError keyed on condition, and Django attaches that to the form's condition field. The error shows where the form's own errors do.
  4. "Does not equal" and empty values. Changed. The display now reads "Status does not equal retired (or is empty)". The lookup label and the docs now say so too.
  5. Polymorphic columns. Object fields can only use isnull, and the model enforces that. The content-type and object-ID columns are always set or cleared together, so testing _object_id is enough.

@bctiemann
bctiemann requested review from a team and jnovinger and removed request for a team October 12, 2026 19:43

@jnovinger jnovinger left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

A couple of items that I think should be addressed:

  1. Text fields saved from the UI store a blank value as '' rather than NULL, so a constraint displayed as "Status is empty" doesn't apply to them: two objects added through the form with the same serial and a blank status both saved. The storage side predates this PR and I'll open a separate issue for it, so should this wait on that fix, or label isnull as "is null" until then?

  2. Posting a condition with a non-string field (e.g. [{"field": ["status"], "lookup": "isnull", "value": true}]) from the constraint form returns a 500 from forms.py:519 rather than a validation error.

Comment thread netbox_custom_objects/forms.py Outdated
Text, long text, URL and select fields store a blank value as NULL or ''
depending on how it was saved, so "is empty" matches both. Also reject
a non-string field name in the form's condition instead of raising a
TypeError, and fix the form's description of the "n" lookup.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@bctiemann

Copy link
Copy Markdown
Contributor Author

@jnovinger Thanks. All three are fixed in c866d61:

  1. Blank text and "is empty": rather than waiting on the storage fix or relabeling, "is empty" now matches both NULL and '' for text, long text, URL and select fields: col IS NULL OR col = '', and col IS NOT NULL AND col <> '' for "is not empty". That works in the partial index and in full_clean()'s check, and stays correct once blank values are stored consistently, at which point the '' branch just stops matching anything. "Does not equal" already counted '' as not equal. Other field types can't store '', so they keep the plain NULL test. test_condition_lookups now covers both blank forms for "is empty", "is not empty" and "does not equal".
  2. Non-string field in the form: any non-string field is now reported as an unknown field ("Unknown field(s): ['status']") instead of raising a TypeError. Added to test_condition_uses_field_names. The API was already safe, since it converts field with int().
  3. Help text: fixed, see the inline reply.

Both full suites pass: 1465 tests without netbox-branching and 1538 with it, 8 skipped in each.

@jnovinger
jnovinger merged commit 7709181 into feature Oct 16, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants