Closes #18159: Expose snapshots to event rule condition evaluation

[bctiemann]

Closes: #18159

Summary

Event rule conditions previously evaluated only the current (post-change) REST API representation of the object. Snapshots — the pre- and post-change data captured at event time — were present in the event queue but never exposed to condition logic, making it impossible to express rules like "fire only when status changes to active."

This PR adds two complementary mechanisms:

1. changed / unchanged operators

New snapshot-comparison operators that check whether an attribute's value differs between the prechange and postchange snapshots. No value key is accepted.

{
  "and": [
    { "attr": "status.value", "value": "active" },
    { "attr": "status", "op": "changed" }
  ]
}

This is the canonical solution for the issue's use case: the webhook fires only on the transition to active, not on subsequent updates that leave status unchanged.

2. Direct snapshot path access

Snapshots are merged into the condition evaluation context, so any existing operator can reference pre/post values via snapshots.prechange.<attr> and snapshots.postchange.<attr> dot-paths.

{ "attr": "snapshots.prechange.status", "value": "planned" }

Note on serialization format: Snapshot data uses the model serializer format (raw field values), not the REST API format. Choice fields like status appear as "active" in snapshots, not {"value": "active", "label": "Active"}. Use attr: "status" — not "status.value" — when referencing snapshot attributes. The changed/unchanged operators compare the same format on both sides, so they are unaffected by this distinction.

Changes

File	Change
extras/conditions.py	Add CHANGED/UNCHANGED operators; _MISSING sentinel; make value optional; _resolve_snapshot_attr() helper; eval_changed()/eval_unchanged() methods
extras/events.py	Merge snapshots into condition context in process_event_rules()
extras/tests/test_conditions.py	16 new tests covering all new functionality and edge cases
extras/tests/test_event_rules.py	1 new integration test exercising the full process_event_rules() path
docs/reference/conditions.md	Document new operators, snapshot path syntax, format caveat, and availability note

Comparison with community PR

A community implementation exists at https://github.com/kenancakir/netbox/pull/1/changes. Key differences from this PR:

	Community PR	This PR
changed/unchanged operators	✓	✓
Snapshot access syntax	prechange.status (intercepts path prefix)	snapshots.prechange.status (transparent path merge)
ConditionSet.eval() signature	Modified (passes snapshots arg through chain)	Unchanged
Implementation complexity	Larger (routing logic, signature changes)	Minimal (one merge in process_event_rules, two eval methods)
Format inconsistency	Unclear	Documented; changed/unchanged are immune
Unit tests added	0	17

The main tradeoff: prechange.status is marginally more ergonomic than snapshots.prechange.status, but requires special-casing the path prefix throughout the eval chain. Our approach falls out naturally from the existing dot-path traversal with no routing logic needed.

🤖 Generated with Claude Code

[bctiemann]

@claude /review and limit response to areas of potential concern.

[github-actions]

Claude finished @bctiemann's task in 2m 16s —— View job

---

PR Review — Areas of Potential Concern

• Read changed files
• Analyze conditions.py changes
• Analyze events.py changes
• Analyze tests
• Post focused review

---

Overall the implementation is clean and well-tested. Below are the specific concerns worth addressing before merge.

---

1. 'snapshots' key injection can silently shadow a model field (events.py:184)

condition_data = {**event['data'], 'snapshots': event.get('snapshots')}

Because the injected key is placed after the model data in the spread, it wins if the serialized model data already contains a 'snapshots' key. No current NetBox core model has such a field, but plugin-provided models could. A user writing {"attr": "snapshots", "op": "eq", ...} would unexpectedly receive the event snapshots dict instead of the model field value.

Possible fix: Use a less collision-prone key name (e.g. '_snapshots') and update the snapshot operators to look for that key instead of 'snapshots'. This is a small change but prevents a hard-to-debug class of bug for plugin authors.

Fix this →

---

2. Delete-event behavior of changed/unchanged is undocumented (conditions.py:195–203)

For OBJECT_DELETED events get_snapshots() sets postchange = None, so _resolve_snapshot_attr(None) always returns _MISSING. For any attribute that exists in prechange, pre != _MISSING → changed returns True on every delete event. This is arguably correct, but it isn't mentioned in the docs. A user who writes a rule like "fire only when status changes" may be surprised that the rule also fires on every deletion of that object.

---

3. Mixing the dot-path snapshot syntax with changed/unchanged is a silent no-op (conditions.py:195–203)

The changed operator resolves self.attr inside the snapshot dicts. If a user writes:

{"attr": "snapshots.prechange.status", "op": "changed"}

_resolve_snapshot_attr looks for the key "snapshots" inside the prechange dict (not a nested path through the top-level context). Both pre and post resolve to _MISSING, so eval_changed returns False and the condition silently never fires. There is no validation that catches this at configuration time.

---

4. changed/unchanged on non-object events fails silently with only an error log (conditions.py:137–140, models/models.py:164)

When an event rule with a changed condition is evaluated against a job lifecycle event (JOB_STARTED, JOB_COMPLETED), event.get('snapshots') is None, InvalidCondition is raised, caught by eval_conditions, and the rule silently returns False (with only an ERROR log). There is no upfront validation that prevents a rule author from misconfiguring a changed condition on a job-type event rule. The error is invisible unless someone is watching logs.

---

5. 'or {}' fallback in _resolve_attr masks null prechange silently (conditions.py:103–104)

def _get(obj, key):
    if isinstance(obj, list):
        return [operator.getitem(item or {}, key) for item in obj]
    return operator.getitem(obj or {}, key)

When a user references snapshots.prechange.status via the dot-path syntax (not the changed operator) on a create event where prechange is None, the or {} fallback converts None to {}, then operator.getitem({}, 'status') raises KeyError → InvalidCondition. The InvalidCondition is caught and False is returned. This is the documented behavior, but the or {} makes the traversal silently attempt the key on an empty dict rather than short-circuiting on None. For deeply nested paths this could produce misleading InvalidCondition messages pointing to the wrong key segment.

---

Minor: Two tests in SnapshotConditionTestCase create Site(name='Site 1', slug='site-1') — they only work because each TestCase method is wrapped in its own transaction rollback. Worth using distinct names to make the isolation explicit and avoid confusion for future test authors.

---

[bctiemann]

Addressed items 2 and 3, and also added a test for the validation on 3. Fixed the duplicate site names in the fixtures as well.

[pheus]

Thanks for working on this! The overall approach looks solid to me.

I think there are a few edge cases worth tightening before merge, mostly around fail-closed behavior for invalid snapshot paths.

[bctiemann]

Addressed both suggestions. Thanks!

[pheus]

Thanks for the follow-up!

I think there is one closely related edge case left before merge: direct snapshot paths using standard operators still go through _resolve_attr(), not _resolve_snapshot_attr().

For example, this is an easy mistake because REST API choice fields normally use .value, while snapshots store raw strings:

{
  "attr": "snapshots.prechange.status.value",
  "value": "planned"
}

With snapshot data like:

{
    "snapshots": {
        "prechange": {"status": "planned"},
        "postchange": {"status": "active"},
    }
}

_resolve_attr() will traverse into "planned" and call operator.getitem("planned", "value"), which raises TypeError. Since _resolve_attr() only catches KeyError, and Condition.eval() calls _resolve_attr() before the TypeError guard around eval_func(), this can still bubble out of event rule evaluation instead of failing closed.

Could we also catch TypeError in _resolve_attr() and convert it to InvalidCondition, similar to the snapshot-operator helper?

[bctiemann]

Good call, implemented (including tests). Thanks!

[pheus]

Thanks for the follow-ups! This looks good to me!