Show configured email notification templates instead of defaults when editing form#24657
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes a bug where the email notification edit page displayed default email body templates instead of user-configured templates when editing existing notifications. The fix ensures that persisted notifications retain their custom templates while new notifications still receive the defaults.
Key Changes:
- Added
isPersistedflag to prevent template override when editing existing notifications - Propagated
notificationIdthrough component hierarchy to identify persisted notifications - Added comprehensive test coverage for both new and existing notification scenarios
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| EmailNotificationForm.tsx | Added notificationId prop and isPersisted flag to preserve existing templates |
| EmailNotificationForm.test.tsx | New test file validating template behavior for new vs. existing notifications |
| EventNotificationFormContainer.tsx | Added lifecycle method to handle notification prop updates |
| EventNotificationForm.tsx | Passed notificationId to child component |
| changelog/unreleased/issue-24515.toml | Added changelog entry documenting the bug fix |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
danotorrey
left a comment
danotorrey
left a comment
There was a problem hiding this comment.
Great job on this fix. This is testing successfully for me locally. I just launched a server/open test instance, and I will run through it there too here in a few.
|
Oops, I launched an Enterprise instance instead. Firing up an open instance now... I'll open a small PR to enable the email transport as well to allow saving the notifications there. |
|
I enabled the email transport in https://github.com/Graylog2/infrastructure/commit/5566082c2f1e3031a63f2e5f182bdb228da87edd and ran Ansible: https://github.com/Graylog2/infrastructure/actions/runs/20821819768 |
danotorrey
left a comment
danotorrey
left a comment
There was a problem hiding this comment.
LGTM and tests successfully:
Open instance:
New email notifications correctly default to the static text and email templates. When a template is edited, the updated value is displayed during subsequent edits and is also used when sending the email.
Enterprise instance:
When no custom template is defined, new email notifications correctly default to the static text and email templates. Any edits are visible in subsequent edit attempts and on the details page. The same behavior works correctly when custom templates are defined.
|
Sweet thanks for the testing @danotorrey ! |
ryan-carroll-graylog
deleted the
24515-when-editing-an-existing-e-mail-notification-the-global-templates-are-shown-not-the-changed-templates
branch
… editing form (#24657) * Show configured email notification templates instead of defaults when editing * add cl entry * add tests * Update changelog/unreleased/issue-24515.toml Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * fix ts errors * fix test * fix test * fix linter complaint * fix test * refactor * cleanup * cleanup * update test * Fix linter complaint --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> (cherry picked from commit a492336)
… editing form (#24657) (#24676) * Show configured email notification templates instead of defaults when editing * add cl entry * add tests * Update changelog/unreleased/issue-24515.toml * fix ts errors * fix test * fix test * fix linter complaint * fix test * refactor * cleanup * cleanup * update test * Fix linter complaint --------- (cherry picked from commit a492336) Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Description
Fixes issue where the email notification edit page always displayed the default email body templates instead of what was saved on the actual notification, even if the user had updated the template.
Looks like this was a bug in the initial implementation of the ability to configure the defaults for these, which was backported to 7.0, so we should backport this fix there too: #24008
Motivation and Context
Closes: #24515
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Checklist: