Commit Graph

5 Commits

Author SHA1 Message Date
bctiemann d2024a1edc
Closes #22770: Allow plugins to register Event Rule action handlers (#22793)
* Closes #22770: Allow plugins to register Event Rule action handlers

Introduces an EventRuleAction registration API (netbox.event_rules /
netbox.extras.event_rules) so plugins can add new EventRule action types
the same way they already register search indexes and event types,
replacing the hardcoded webhook/script/notification elif-chain. Core's
own three action types are refactored onto this mechanism.

An EventRule referencing an unregistered action (e.g. its providing
plugin is uninstalled) remains stored, is skipped during processing
without affecting other rules, is visibly marked unavailable in the
UI/API, and triggers a new extras.W001 system check, resuming
automatically once the plugin is reinstalled, with no need to re-save.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* Fix CI failure: extras.W001 check must tolerate a not-yet-migrated database

check_event_rule_actions() queried EventRule unconditionally, which broke
`manage.py makemigrations --check` (and a fresh `manage.py migrate`) on a
database with no tables yet, since Django runs system checks before
verifying/applying migrations. Wrap the query and swallow DatabaseError,
matching the existing check_postgresql_version precedent for a database
that may not be ready. Verified against a fresh, unmigrated database that
makemigrations --check, migrate, and manage.py check all behave correctly,
and that the warning still fires once a qualifying EventRule exists.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* Fix EventRuleForm action_type widget: HTMXSelect was silently ignored

Meta.widgets only applies to fields the ModelForm auto-generates; action_type
is an explicit class-level field, so its HTMXSelect assignment in Meta.widgets
never took effect, and switching Action type in the browser never refreshed
the action_choice field's label/queryset. Move the widget onto the field
declaration itself.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* Address review feedback from Jeremy Stretch on PR #22793

- Revert action_object_type on_delete to CASCADE (was changed to SET_NULL)
- Make action_type choices dynamic via the model field's own callable
  choices=, simplifying EventRule.clean() and making any unavailable
  action_type invalid on save, whether new or unchanged
- Rename is_action_available to action_is_available
- Fold new dispatch tests into the existing RQQueueTestMixin test class to
  fix a flaky --parallel run (two such classes cross-flush each other's
  Redis queues)
- Only catch broad exceptions around plugin-provided actions in
  process_event_rules(); let a core action's own bugs propagate
- Add value_action_type() so table exports don't leak the "unavailable"
  badge's HTML markup
- Drop the frozen CSVChoiceField on action_type and make action_object
  optional at the field level, so bulk import of an object-less action
  works
- Map ValidationErrors on unexposed model fields to NON_FIELD_ERRORS in
  bulk import instead of letting them surface as a raw ValueError
- Restore EventRuleActionEnum/the enum-based GraphQL filter, built from
  the live action registry instead of the static EventRuleActionChoices
- Default EventRuleAction.object_required to False, matching
  object_model's default of None; set it explicitly on the three core
  actions
- Drop the unused request parameter on get_object_queryset()
- Fix action_object_type's serializer queryset, which incorrectly used
  the triggering object_types' feature flag
- Use .format() instead of % in get_action_type_display()
- Remove the extras.W001 system check (a DB query on every management
  command) in favor of an action_is_available field on the REST API
- Raise ValidationError instead of a bare Exception on duplicate action
  slug registration
- Shorten a couple of overly verbose inline comments
- Split EventRuleAction.validate() into an internal _validate() and a
  public no-op validate(), so a subclass's custom validation doesn't
  need to remember to call super()

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* Trim verbose comments/docstrings added while addressing review feedback

Shortened a number of overly long inline comments and test docstrings
introduced across the previous commit's review-feedback fixes (the
EventRuleActionEnum comment, the _validate()/validate() docstrings, and
several test docstrings that restated context already given elsewhere).
Also drops the auto-generated "Generated by Django" header comment from
migration 0143, matching the rest of this app's hand-touched migrations.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* Address findings from automated follow-up review of #22793

- Clear stale action_object_type/action_object_id when an action declares
  object_model but is left with no object selected (object_required=False);
  previously neither branch of the if/elif fired and the old value from
  before the edit silently persisted. Fixed in both EventRuleForm and
  EventRuleImportForm (the latter matters for CSV updates of an existing
  row). Also resolve the content type from the actual selected object
  rather than the action's declared object_model, correctly handling
  subclass/proxy instances.
- Validate action slugs at registration time (format, and collision via
  enum_key() with an already-registered slug) so a bad third-party slug
  is rejected immediately instead of crashing GraphQL schema assembly at
  startup.
- Strip the dead-code label/description duplication out of
  EventRuleActionChoices.CHOICES -- nothing reads it, and it risked
  drifting from WebhookAction/ScriptAction/NotificationAction's own.
- Record whether an action is plugin-provided at registration time
  instead of introspecting its module on every dispatch; core's three
  actions now register with is_plugin_provided=False explicitly.
- Add an action_is_available filter (API + UI) so event rules with a
  now-unavailable action can still be found in bulk, now that the
  extras.W001 system check is gone.
- Update the plugin dev docs: fix the OpenTicketAction example (it was
  missing object_required=True, the exact gap the action_object fix
  above addresses), note that an unavailable rule can't be saved at all
  (not just skipped), and move an internal-only note out of the
  published class docstring.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* Address findings from second automated follow-up review of #22793

- Drop the ChoiceSet base from EventRuleActionChoices. With CHOICES=(),
  the previous version made ChoiceField(choices=EventRuleActionChoices)
  -- the idiomatic pattern used for every other ChoiceSet in this
  codebase, and reachable via `from extras.choices import *` -- silently
  reject every value instead of failing at first use.
- Reject slugs containing hyphens or a leading underscore at
  registration time: a hyphenated slug (plausible, since plugin
  distribution names are conventionally hyphenated) or a slug starting
  with an underscore both currently pass validation but produce a
  GraphQL-invalid or GraphQL-reserved enum member name once sanitized,
  crashing schema assembly at startup. Document the constraint in the
  plugin docs and the published slug docstring.
- Raise ImproperlyConfigured instead of ValidationError for all three
  registration-failure cases in register_event_rule_action() -- these
  are packaging/configuration mistakes surfaced from AppConfig.ready(),
  not user input, matching the convention ChoiceSetMeta already uses for
  the same class of error.
- Remove is_plugin_provided's class-level default; nothing reads it
  before an action is registered in any real code path, and the default
  masked a class-vs-instance inconsistency. Move its documentation out
  of the published Attributes docstring into a plain comment.
- Simplify EventRuleImportForm.clean()'s action_object_type/id
  assignment to match EventRuleForm.clean()'s approach (set both fields
  once, unconditionally, from the resolved object) rather than assigning
  via the GFK setter and then conditionally overwriting the content
  type.
- Split a dense doc sentence in eventrule.md onto its own line.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* Address findings from third automated follow-up review of #22793

- Restore is_plugin_provided's class-level default of True. Its only
  read is inside process_event_rules()'s exception handler; without a
  default, an action reaching dispatch without going through
  register_event_rule_action() (e.g. inserted into the registry dict
  directly) raised AttributeError while already handling the real
  exception, masking it entirely instead of degrading gracefully.
- Move the slug/label presence checks out of __init_subclass__ (which
  fired at class-definition time, raising TypeError) and into
  register_event_rule_action() as ImproperlyConfigured, unifying them
  with the other three registration-time checks. This also resolves a
  still-open item from the very first automated review: an intermediate
  base class shared by several concrete plugin actions couldn't
  previously be defined without a placeholder slug/label of its own.
- Restore the GFK assignment (self.instance.action_object = obj) in
  EventRuleImportForm.clean() alongside the explicit content-type
  assignment, so EventRule.clean()'s later access to action_object hits
  the descriptor cache instead of an extra SELECT per imported row.
- Clarify the slug docstring/docs wording (leading underscore
  specifically, not underscores in general; tell authors to use an
  underscore instead of a hyphen) and document that intermediate base
  classes are now supported. Add a test for an uppercase slug.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* Document the frozen-at-import tradeoff on EventRuleSerializer.action_type

The EventRuleActionAPITestCase docstring in test_api.py pointed here for
an explanation of why the choices are materialized once at module-import
time rather than dynamically, but the field itself had no such comment.

* Address findings from fourth automated follow-up review of #22793

- Restore the "must start with a letter" slug constraint dropped from
  the docs page and class docstring by the previous round's rewording;
  reword to "must begin with a lowercase letter", which covers the
  leading-digit case SLUG_RE actually rejects and matches the
  ImproperlyConfigured message an author will hit.
- Scope the GFK-cache-priming comment in EventRuleImportForm.clean() to
  the non-proxy case it actually holds for, rather than claiming it
  unconditionally.
- Trim comments and docstrings that had regrown into reviewer-facing
  rationale (why a prior finding was reverted, why a check isn't in
  __init_subclass__ anymore rather than API documentation, in
  event_rules.py and test_event_rules.py.
EOF
)

* Misc cleanup

* Misc cleanup

---------

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Co-authored-by: Jeremy Stretch <jstretch@netboxlabs.com>
2026-07-31 16:28:01 -04:00
Jeremy Stretch 10748edc3a Fixes #18222: Include action data from event rule in webhook and custom script data 2025-01-03 09:39:05 -05:00
Jeremy Stretch 6e59db7310 #16886: Updated documentation for event types 2024-07-31 15:54:31 -04:00
Jeremy Stretch 115111df9e #14132: Fix documentation link 2023-12-04 11:15:13 -05:00
Arthur Hanson a38a38218b
14132 Add EventRule - change webhook and add in script processing to events (#14267)
---------

Co-authored-by: Jeremy Stretch <jstretch@netboxlabs.com>
2023-11-30 16:36:33 -05:00