Add support for Apprise Tags #1123
Conversation
WalkthroughA new "TARGETTYPE" configuration option was added to the Apprise publisher plugin, allowing selection between "url" and "tag" as the target type. The Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~7 minutes Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
front/plugins/_publisher_apprise/apprise.py (2)
81-84: Simplify the return statement as suggested by static analysis.The condition can be simplified by returning the negated condition directly.
Apply this diff:
- if get_setting_value('APPRISE_HOST') == '' or get_setting_value('APPRISE_URL') == '': - return False - else: - return True + return not (get_setting_value('APPRISE_HOST') == '' or get_setting_value('APPRISE_URL') == '')
116-123: Consider refactoring to reduce code duplication and improve clarity.The new tag functionality works correctly, but there are opportunities for improvement:
- Code duplication: The payload structure is duplicated between the two branches
- Variable naming: Using
APPRISE_URLfor both URLs and tags could be confusingConsider this refactor to reduce duplication and improve clarity:
- # Define Apprise compatible payload (https://github.com/caronc/apprise-api#stateless-solution) - - _json_payload = { - "urls": get_setting_value('APPRISE_URL'), - "title": "NetAlertX Notifications", - "format": get_setting_value('APPRISE_PAYLOAD'), - "body": payloadData - } - - if get_setting_value('APPRISE_TARGETTYPE') == 'tag': - _json_payload = { - "tags": get_setting_value('APPRISE_URL'), - "title": "NetAlertX Notifications", - "format": get_setting_value('APPRISE_PAYLOAD'), - "body": payloadData - } + # Define Apprise compatible payload (https://github.com/caronc/apprise-api#stateless-solution) + target_key = "tags" if get_setting_value('APPRISE_TARGETTYPE') == 'tag' else "urls" + + _json_payload = { + target_key: get_setting_value('APPRISE_URL'), + "title": "NetAlertX Notifications", + "format": get_setting_value('APPRISE_PAYLOAD'), + "body": payloadData + }front/plugins/_publisher_apprise/config.json (1)
425-456: Well-structured configuration setting with a translation issue.The new TARGETTYPE configuration setting is properly implemented with appropriate structure, data types, and options. However, there's an issue with the Spanish translation.
Translation correction needed:
The Spanish translation uses "alvo" which appears to be Portuguese. Apply this diff to correct it:
"language_code": "es_es", - "string": "Tipo de alvo" + "string": "Tipo de destino" } ], "description": [ { "language_code": "en_us", "string": "Select the target type sent to Apprise." }, { "language_code": "es_es", - "string": "Seleccione el tipo de alvo enviada a Apprise." + "string": "Seleccione el tipo de destino enviado a Apprise." }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
front/php/templates/language/de_de.json(0 hunks)front/php/templates/language/en_us.json(1 hunks)front/plugins/_publisher_apprise/apprise.py(2 hunks)front/plugins/_publisher_apprise/config.json(1 hunks)
💤 Files with no reviewable changes (1)
- front/php/templates/language/de_de.json
🧰 Additional context used
🧬 Code Graph Analysis (1)
front/plugins/_publisher_apprise/apprise.py (1)
server/helper.py (1)
get_setting_value(228-247)
🪛 Ruff (0.12.2)
front/plugins/_publisher_apprise/apprise.py
81-84: Return the negated condition directly
Inline condition
(SIM103)
🔇 Additional comments (1)
front/php/templates/language/en_us.json (1)
8-17: LGTM! Well-structured localization entries for Apprise settings.The new English localization entries are comprehensive and well-formatted. Each entry includes both descriptive text and display names that will help users understand and configure the Apprise notification settings. The naming convention and structure are consistent with existing entries in the file.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
front/plugins/_publisher_apprise/apprise.py (1)
81-84: Simplify the condition logic.The static analysis tool correctly identifies that this can be simplified by returning the negated condition directly.
Apply this diff to improve readability:
- if get_setting_value('APPRISE_HOST') == '' or get_setting_value('APPRISE_URL') == '': - return False - else: - return True + return get_setting_value('APPRISE_HOST') != '' and get_setting_value('APPRISE_URL') != ''
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
front/plugins/_publisher_apprise/apprise.py(2 hunks)
🧰 Additional context used
🪛 Ruff (0.12.2)
front/plugins/_publisher_apprise/apprise.py
81-84: Return the negated condition directly
Inline condition
(SIM103)
| "body": payloadData | ||
| } | ||
|
|
||
| if get_setting_value('APPRISE_TARGETTYPE') == 'tag': |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify validation for APPRISE_TARGETTYPE setting.
The code assumes that APPRISE_TARGETTYPE contains valid values ('tag' or presumably 'url'). Ensure that proper validation exists for this setting to prevent unexpected behavior.
🏁 Script executed:
#!/bin/bash
# Description: Check if validation exists for APPRISE_TARGETTYPE setting
# Expected: Find validation logic or default value handling
# Search for APPRISE_TARGETTYPE validation or configuration
rg -A 5 -B 5 "APPRISE_TARGETTYPE"
# Look for configuration files that might define valid values
fd -e json -e py -e php --exec rg -l "TARGETTYPE" {}Length of output: 1076
🏁 Script executed:
#!/bin/bash
# Display the configuration file to inspect valid values or defaults for APPRISE_TARGETTYPE
sed -n '1,200p' front/plugins/_publisher_apprise/config.jsonLength of output: 4308
🏁 Script executed:
#!/bin/bash
# Search for get_setting_value function definition and its usage to inspect validation logic
rg -n "def get_setting_value"
rg -n "get_setting_value("Length of output: 196
🏁 Script executed:
#!/bin/bash
# Show the get_setting_value implementation to verify default handling and validation
sed -n '200,260p' server/helper.pyLength of output: 2220
🏁 Script executed:
#!/bin/bash
# Show the APPRISE_TARGETTYPE usage and default else branch in apprise.py
sed -n '80,140p' front/plugins/_publisher_apprise/apprise.pyLength of output: 2196
Add explicit validation for APPRISE_TARGETTYPE
Currently, get_setting_value('APPRISE_TARGETTYPE') defaults to an empty string and isn’t checked against any allowed values (e.g. 'url' or 'tag'), which could lead to silent mis-routing of payloads. We should enforce valid options at configuration load or right before use:
• front/plugins/_publisher_apprise/config.json
– Define a new entry under "params" for APPRISE_TARGETTYPE with allowed values ("url", "tag") and a default.
• front/plugins/_publisher_apprise/apprise.py
– In send(), after reading APPRISE_TARGETTYPE, verify it’s one of the expected values.
– Fallback to 'url' or raise/log an error if it’s invalid before constructing _json_payload.
• (Optional) server/helper.py
– Extend setting_value_to_python_type or get_setting_value to validate against declared option lists and warn on unknown values.
This will prevent unexpected behavior when the setting is missing or malformed.
🤖 Prompt for AI Agents
In front/plugins/_publisher_apprise/apprise.py at line 116, the code uses
get_setting_value('APPRISE_TARGETTYPE') without validating its value, which can
cause silent errors. To fix this, after retrieving APPRISE_TARGETTYPE in the
send() function, add a check to confirm it is either 'url' or 'tag'. If the
value is invalid or missing, either default it to 'url' or log/raise an error
before proceeding. Additionally, update
front/plugins/_publisher_apprise/config.json to define APPRISE_TARGETTYPE under
"params" with allowed values 'url' and 'tag' and a default. Optionally, enhance
server/helper.py's setting retrieval functions to validate against allowed
options and warn on invalid values.
There was a problem hiding this comment.
Thanks for the PR @dougmaitelli - you shouldn't modify en_us.json - strings are loaded from front/plugins/_publisher_apprise/config.json
There was a problem hiding this comment.
I see, so the translations en_us are auto-generated? Any idea why the german translations have the english values then?
There was a problem hiding this comment.
Reverted these
There was a problem hiding this comment.
I have to clean up the german/spanish json files, those are remainders from when there was no plugin system in the app
| #------------------------------------------------------------------------------- | ||
| def check_config(): | ||
| if get_setting_value('APPRISE_URL') == '' or get_setting_value('APPRISE_HOST') == '': | ||
| if get_setting_value('APPRISE_HOST') == '' or get_setting_value('APPRISE_URL') == '': |
There was a problem hiding this comment.
Unsure why this has to change?
There was a problem hiding this comment.
I doesn't, I was originally validating the new setting there but decided against it and I guess this was left-over, I can revert this line
|
Also, according to docs you might need one more setting on the plugin to specify the tags:
You could probably get rid of |
I was thinking about that, but we can either pass URLs or TAG to the API, and if we have both fields it might get confusing to the users |
In that case I would add still a separate setting for TAGS and keep the TARGETTYPE setting too as a switcher between the modes. Reusing the URL setting is probably too confusing too. I'm also unsure if you can pass URL and TAGS at teh same time, according to teh docs it seems possible - if so, then the TARGETTYPE setting might not be needed: {
"tags": ["devops", "admin", "me"],
"urls": [
{
"url": "slack://TokenA/TokenB/TokenC",
"tags": ["devops", "admin"]
},
{
"url": "discord://WebhookID/WebhookToken",
"tags": ["devops"]
},
{
"url": "mailto://user:pass@gmail.com",
"tags": ["me"]
}
]
}but maybe I misunderstand the docs |
You might be right, I did not try that either |
@jokob-sk I believe you misunderstood the docs. The data you pointed out is how you configure the tags on apprise side, this is not a API payload example |
Thanks for double checking! Let me know once this is tested and happy to merge if its working as you expect |
Added a separate setting for TAG as you suggested too to avoid confusion |
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
front/plugins/_publisher_apprise/apprise.py (1)
109-111: Clean implementation that eliminates code duplication.The dynamic assignment approach effectively addresses the previous code duplication while maintaining clear logic flow.
Note: As mentioned in previous reviews,
APPRISE_TARGETTYPElacks explicit validation. While the current approach gracefully defaults to "urls" behavior for invalid values, consider adding validation to ensure the setting contains only expected values ('url' or 'tag').
🧹 Nitpick comments (1)
front/plugins/_publisher_apprise/apprise.py (1)
81-84: LGTM! Consider simplifying the return condition.The updated validation logic correctly ensures either
APPRISE_URLorAPPRISE_TAGis provided, which aligns with the new target type functionality.Consider applying this simplification as suggested by static analysis:
- if get_setting_value('APPRISE_HOST') == '' or (get_setting_value('APPRISE_URL') == '' and get_setting_value('APPRISE_TAG') == ''): - return False - else: - return True + return not (get_setting_value('APPRISE_HOST') == '' or (get_setting_value('APPRISE_URL') == '' and get_setting_value('APPRISE_TAG') == ''))
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
front/plugins/_publisher_apprise/apprise.py(2 hunks)front/plugins/_publisher_apprise/config.json(2 hunks)
🧰 Additional context used
🪛 Ruff (0.12.2)
front/plugins/_publisher_apprise/apprise.py
81-84: Return the negated condition directly
Inline condition
(SIM103)
🔇 Additional comments (3)
front/plugins/_publisher_apprise/apprise.py (1)
113-113: Perfect usage of the dynamic target key.The payload construction cleanly uses the dynamically determined
target_key, completing the refactoring effectively.front/plugins/_publisher_apprise/config.json (2)
425-456: Well-configured target type setting.The TARGETTYPE setting is properly structured with correct options, appropriate default value for backward compatibility, and proper localization.
489-520: Appropriate configuration for tag input.The TAG setting is correctly configured as a string input with proper localization and an appropriate empty default value.
|
Looks good! Let me know once you test it and it's working :) |
Added a separate setting for TAG as you suggested too to avoid confusion |
|
I tested with both tag and url and it works. For URL host I used |
|
Cool, thanks, do we need to add anything to the setting description? E. G.
if they need to use a slash "/" or if multiple tags are supported? It's OK
if nothing needs to be changed
…On Mon, 28 July 2025, 14:10 Douglas Maitelli, ***@***.***> wrote:
*dougmaitelli* left a comment (netalertx/NetAlertX#1123)
<#1123 (comment)>
@jokob-sk <https://github.com/jokob-sk>
I tested with both tag and url and it works.
For URL host I used /notify/ and for tag I used /notify/apprise (apprise
is the key for my config that contains the tags)
—
Reply to this email directly, view it on GitHub
<#1123 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AW5URDFLATJMJXRQFIGKEED3KWPCHAVCNFSM6AAAAACCPLR3JCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTCMRVGMZTKMZUGY>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
I think only documentation mayb, the UI change is probably good enough |
|
Thanks, will merge and it will be available in 15 min in the netalertx-dev image if you want to double check :) |
2 participants
Add support for Apprise Tags so users can send notifications to tags / categories defined on the Apprise config
Summary by CodeRabbit
New Features
Bug Fixes