feat: notify on change - #696
Conversation
Notifications could be attached for start, success and failure, so a playbook that runs nightly in check mode to report drift had nothing to notify on: it succeeds whether or not it found anything. Job templates and organizations gain a changed trigger. A run that reported a change on any host notifies those templates next to the ones for how it ended, so a job that changed something and succeeded notifies both. Ad hoc commands are covered by the organization trigger the same way jobs are, and the job types that do not record per host results never fire it. The message for the trigger is customizable like the others, and defaults to one that says the run reported changes.
9a803ad to
08009df
Compare
There was a problem hiding this comment.
Pull request overview
Adds a new Changed notification trigger for job templates and organizations, enabling notifications when a run reports changes (including check mode), alongside the existing start/success/failure triggers.
Changes:
- Backend: introduce
notification_templates_changed, ahas_changes()contract, and dispatch logic to emit Changed in addition to the terminal outcome trigger. - API/UI: add new association endpoints and UI toggles + message customization fields for the Changed trigger.
- Tests/docs: add functional tests for the new trigger and update user documentation.
Reviewed changes
Copilot reviewed 29 out of 29 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/docsite/rst/userguide/notifications.rst | Documents the new Changed notification trigger and endpoints. |
| awx/ui/src/screens/Template/Template.js | Enables Changed toggle for job template notifications UI. |
| awx/ui/src/screens/Organization/Organization.js | Enables Changed toggle for organization notifications UI. |
| awx/ui/src/screens/NotificationTemplate/shared/NotificationTemplateForm.js | Adds Changed message fields to the notification template form normalization/defaulting. |
| awx/ui/src/screens/NotificationTemplate/shared/notification-template-default-messages.json | Defines default Changed messages/bodies per notification backend type. |
| awx/ui/src/screens/NotificationTemplate/shared/hasCustomMessages.js | Treats Changed message customizations as “custom messages”. |
| awx/ui/src/screens/NotificationTemplate/shared/CustomMessagesSubForm.js | Adds Changed message/body editors to the custom messages subform. |
| awx/ui/src/components/NotificationList/NotificationListItem.test.js | Adds tests for rendering/clicking the Changed toggle. |
| awx/ui/src/components/NotificationList/NotificationListItem.js | Renders a Changed toggle in notification list rows when enabled. |
| awx/ui/src/components/NotificationList/NotificationList.test.js | Adds tests for fetching/disassociating Changed templates in lists. |
| awx/ui/src/components/NotificationList/NotificationList.js | Fetches Changed-associated templates and wires toggle state. |
| awx/ui/src/api/mixins/Notifications.mixin.js | Adds client methods for Changed notification template association endpoints. |
| awx/main/tests/functional/models/test_notifications.py | Adds model-level tests for has_changes() and changed-trigger dispatch behavior. |
| awx/main/tests/functional/api/test_notifications.py | Adds API endpoint tests for job template/org Changed notification associations. |
| awx/main/notifications/webhook_backend.py | Adds Changed defaults for webhook notifications. |
| awx/main/notifications/pagerduty_backend.py | Adds Changed defaults for PagerDuty notifications. |
| awx/main/notifications/grafana_backend.py | Adds Changed defaults for Grafana notifications. |
| awx/main/notifications/email_backend.py | Adds Changed defaults for email notifications. |
| awx/main/notifications/custom_notification_base.py | Introduces default Changed message/body templates. |
| awx/main/models/organization.py | Adds notification_templates_changed M2M on Organization. |
| awx/main/models/notifications.py | Adds has_changes() hook and dispatch logic for Changed notifications. |
| awx/main/models/jobs.py | Adds notification_templates_changed on JobTemplate and aggregates org/jobtemplate changed templates. |
| awx/main/models/ad_hoc_commands.py | Adds org-level Changed templates aggregation and has_changes() for ad hoc commands. |
| awx/main/migrations/0209_notification_templates_changed.py | Migration adding Changed notification template relations. |
| awx/api/views/organization.py | Adds Organization changed-notification templates sublist view. |
| awx/api/views/init.py | Adds JobTemplate changed-notification templates sublist view. |
| awx/api/urls/organization.py | Adds organization /notification_templates_changed/ route. |
| awx/api/urls/job_template.py | Adds job template /notification_templates_changed/ route. |
| awx/api/serializers.py | Adds related links and message validation support for the Changed event. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
Thanks, this review earned its keep. Both findings were right, and chasing the first one turned up a real bug in the code rather than in the test. Fixed in 0e90343.
That last one is not a test problem. I had written - return self.job_host_summaries.filter(changed__gt=0).exists()
+ return self.ad_hoc_command_events.filter(changed=True).exists()I also added the negative case, The parametrized test. Correct on both counts: - @pytest.mark.django_db
@pytest.mark.parametrize('JobClass', [InventoryUpdate, ProjectUpdate, SystemJob, WorkflowJob])
def test_job_types_without_host_results_never_report_changes(self, JobClass):
- assert JobClass.objects.create(name='fake-job').has_changes() is False
+ assert JobClass().has_changes() is FalseThese four inherit The two docstrings. Updated both to Tests after the change:
The whole suite, on a freshly created database, in a checkout linked with That failure is unrelated: |
|
After this PR, it was wanting to create a new 210 migration (and ui tests are failing), so I ran this through Fable internally, and it came up with these. Issues found
|
Three things came out of the review:
- 0209 hard-coded the resolved related_name values, jobtemplate_... and
organization_..., while the models declare the %(class)s form. Django's
autodetector compares them literally, so makemigrations wanted a corrective
0210 AlterField. related_name is ORM-only metadata, so the migration as
already applied stays valid and the schema does not change.
- NotificationTemplateForm reads defs.changed the way it reads the other
message keys, and the fixture in NotificationTemplateForm.test.js had no
changed key, so the form crashed on render and took 10 pre-existing tests
with it. The real prop comes from server OPTIONS, which now always includes
it, so the fixture is what was stale.
- NotificationTemplate.default_messages() still returned only started, success,
error and workflow_approval, so a template created without explicit messages
exposed a messages dict with no changed key. The send path falls back through
.get('changed', {}), so nothing broke, but the API response was inconsistent.
|
Thanks, all three were right. Applied in 5dbfcda, and the branch now carries current Migration state mismatch. Confirmed and fixed. I reproduced it before and after: with the hard-coded names, With the The 10 UI tests. Same call, and for the same reason:
Verification after the merge:
|
|
Main has a new migration file, so will need to bump this one up to 210 now, then it should be good to merge. |
Closes #206.
Problem
Notifications can be attached to a job template for start, success and failure. A hardening playbook that runs nightly in check mode succeeds whether or not it found drift, so there is nothing to notify on: the interesting outcome is that something reported as changed.
Change
Job templates and organizations gain a fourth trigger, Changed.
A run that reported a change on any host notifies the templates attached to that trigger, next to the ones for how the run ended. A job that changed something and succeeded notifies both Success and Changed; one that changed something and failed notifies Failure and Changed. Check mode counts, which is what makes the trigger useful for a compliance playbook. Start never fires it, since nothing has run yet.
notification_templates_changedon the job template and on the organization, with the endpoints and the related links that the other triggers have.Job.has_changes()is true when any of the job's host summaries recorded a change. Ad hoc commands answer the same way, so an organization's changed templates cover them as they cover its jobs. Project updates, inventory updates, system jobs and workflow jobs record no per host results and never fire it.{{ job_friendly_name }} #{{ job.id }} '{{ job.name }}' reported changes: {{ url }}.In the UI the Notifications tab of a job template and of an organization gains a Changed toggle, and the notification template form gains the Changed message fields.
Workflow job templates are left out: a workflow does not run against hosts itself, and the job templates inside it each notify on their own.
Testing
flake8 awxandblack --check awxpass, and prettier passes on every changed file underawx/ui.The suites themselves were not run locally, since that needs the development image; CI runs them on this pull request.