Skip to content

Add "serverName" and "environmentName" to the alert template variable list#47

Open
rogin wants to merge 1 commit intoOpenIntegrationEngine:mainfrom
rogin:alerts-add-envname
Open

Add "serverName" and "environmentName" to the alert template variable list#47
rogin wants to merge 1 commit intoOpenIntegrationEngine:mainfrom
rogin:alerts-add-envname

Conversation

@rogin
Copy link
Contributor

@rogin rogin commented Apr 19, 2025

Adds "serverName" and "environmentName" to the alert template variable list.
Requires Java 9+.

Fixes NextGen request #6261.

Previous PR is #6262.

Slight tangent: I deleted KeyEncryptorTest to get over / ignore its java 9+ error that I don't care to investigate as I believe it's unrelated to this code change: error: package com.sun.crypto.provider is not visible.

Assuming we have documentation, it should be updated as two additional variables will display in the Alert Template variable list.

Test procedure:

  1. Compile this patch with Java 9+ and start server
  2. Login and get past initial account setup
  3. Set Server Name and Environment Name under Settings tab
  4. Import and deploy the two attached channels
  5. Import the attached alert (remove its .txt extension after downloading)
  6. Open the imported alert and validate two additional options are present on the right-hand side: "serverName" and "environmentName"
  7. Validate the two additional options can be copied into the template text area
  8. Send any message to channel "throws error"
  9. Validate the resulting text in channel "Receiver" contains the Server Name and Environment Name as you defined in the Settings tab
  10. Clear the Server Name and Environment Name under Settings tab, then re-run a message to ensure the alert template's values are replaced with empty strings

throws.error.xml.txt
Receiver.xml.txt
Foo.alert.xml.txt

jonbartels
jonbartels previously approved these changes Apr 20, 2025
@rogin rogin force-pushed the alerts-add-envname branch 2 times, most recently from 0f91658 to fd5d84f Compare April 21, 2025 02:22
@rogin
Copy link
Contributor Author

rogin commented Apr 21, 2025

You can tell how often I use git for these situations. Need help getting just my commit signed and pushed.

@rogin rogin force-pushed the alerts-add-envname branch from fd5d84f to b0684db Compare April 21, 2025 16:46
@jonbartels jonbartels added this to the Technical Preview 2 milestone May 6, 2025
@rogin rogin force-pushed the alerts-add-envname branch from 7ebe149 to 4302644 Compare July 14, 2025 20:23
@ssrowe ssrowe changed the title add two vars to the alert template variable list Add "serverName" and "environmentName" to the alert template variable list Jul 28, 2025
@ssrowe ssrowe requested a review from Copilot March 11, 2026 18:34
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds two new Velocity template variables (serverName, environmentName) for alert templates, exposing values from server settings and listing them in the client alert editor UI.

Changes:

  • Server: Populate alert template context with serverName and environmentName from ServerSettings.
  • Client: Add serverName and environmentName to the alert template variable list in the alert editor UI.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
server/src/com/mirth/connect/server/alert/Alert.java Adds server settings values into the Velocity context for alert template rendering.
client/src/com/mirth/connect/client/ui/alert/DefaultAlertEditPanel.java Adds the new variables to the UI’s template variable list.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Issue: nextgenhealthcare/connect#6261
Original-pr: nextgenhealthcare/connect#6262
Signed-off-by: Richard Ogin <rogin@users.noreply.github.com>
@github-actions
Copy link

Test Results

  105 files  ±0    202 suites  ±0   6m 56s ⏱️ + 1m 10s
  633 tests ±0    633 ✅ ±0  0 💤 ±0  0 ❌ ±0 
1 266 runs  ±0  1 266 ✅ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit 4e038df. ± Comparison against base commit 9a36030.

Copy link
Member

@tonygermano tonygermano left a comment

Choose a reason for hiding this comment

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

I've rebased this and resolved the merge conflicts since #258 was merged. It works, but I'm reluctant to approve at this time.

The currently used ConfigurationController.getInstance().getServerName() executes quickly because the server name is cached by the DefaultConfigurationController.

ConfigurationController.getInstance().getServerSettings(), however, is a much more expensive call, as it makes multiple calls to the database each time.

The expensive call has the benefit of returning up-to-date information, even if the settings were changed by a different server instance connected to the same database. I wonder if we should add a separate getCachedServerSettings method on ConfigurationController that this would call instead?

I have not done any benchmarking. I'll defer to others if consensus is that these extra db calls should not be considered a problem.

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.

4 participants