Add allow_multiple_targets to RESTful notification documentation #35675
Conversation
iamjackg
requested review from
bdraco,
thecode,
a team,
bachya and
allenporter
as code owners
home-assistant
bot
added
has-parent
✅ Deploy Preview for home-assistant-docs ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
📝 WalkthroughWalkthroughThe documentation for the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Notifier
participant Target
User->>Notifier: Send notification with allow_multiple_targets
Notifier->>Target: Send notification to all specified targets
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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
🧹 Outside diff range and nitpick comments (9)
source/_integrations/notify.rest.markdown (2)
72-76: LGTM: Well-documented new parameterThe
allow_multiple_targetsparameter is well-documented and follows the configuration block format. The description clearly explains the behavior change.Consider enhancing the description slightly for even more clarity:
- description: Send all targets to the resource. By default, the notifier only sends the first one. + description: When set to true, sends all targets to the resource. By default (false), the notifier only sends the first target.
Line range hint
77-85: Fix duplicatedataconfiguration entriesThere are two
dataconfiguration entries with different descriptions. This could cause confusion for users. The second entry appears to be the updated version supporting templates.Remove the duplicate entry and combine the information:
-data: - description: Dictionary of extra parameters to send to the resource. - required: false - type: string data: - description: Template dictionary of extra parameters to send to the resource. + description: Dictionary of extra parameters to send to the resource. Supports templates. required: false - type: template + type: template|stringsource/_integrations/simplisafe.markdown (1)
158-178: Documentation content looks good but needs minor improvements.The new section about secret alerts is well-structured and provides clear instructions. However, a few enhancements would make it even better:
- Add a link to SimpliSafe's documentation about secret alerts
- Clarify if this feature is available on both V2 and V3 systems
- Consider adding a troubleshooting section
Here's a suggested improvement to the note section:
{% note %} -Due to the way Simplisafe implements secret alerts, you can only determine when a sensor is triggered, not when it is cleared. +Due to SimpliSafe's implementation of secret alerts: +- You can only determine when a sensor is triggered, not when it is cleared +- This feature may increase battery usage on your sensors {% endnote %}source/_integrations/webostv.markdown (1)
79-83: Fix minor formatting issues.A few formatting improvements needed:
- Add a comma after "attribute" in line 79
- Remove trailing spaces in lines 80 and 83
-Look for the `source_list` attribute which contains all available sources. +Look for the `source_list` attribute, which contains all available sources.🧰 Tools
🪛 LanguageTool
[uncategorized] ~79-~79: Possible missing comma found.
Context: ...r entity. 3. Look for thesource_listattribute which contains all available sources. ...(AI_HYDRA_LEO_MISSING_COMMA)
🪛 Markdownlint
80-80: Expected: 0 or 2; Actual: 3
Trailing spaces(MD009, no-trailing-spaces)
83-83: Expected: 0 or 2; Actual: 1
Trailing spaces(MD009, no-trailing-spaces)
source/_posts/2024-11-06-release-202411.markdown (1)
432-455: Remove unused PR reference.The link reference
[#129970]appears to be unused in the document. Consider removing it to maintain clean documentation.-[#129970]: https://github.com/home-assistant/core/pull/129970🧰 Tools
🪛 Markdownlint
432-432: Unused link or image reference definition: "#129970"
Link and image reference definitions should be needed(MD053, link-image-reference-definitions)
source/_integrations/nest.markdown (1)
553-553: LGTM! Consider a minor enhancement to improve clarity.The new troubleshooting entry provides valuable guidance for users encountering OAuth credential and account issues. The content is accurate and well-structured.
Consider restructuring the entry slightly to make it more scannable:
-- *Something went wrong: Please contact the developer of this app if the issue persists*: This typically means you are using the wrong type of credential (e.g. *Desktop Auth*). Make sure the credential in the [Google Cloud Console](https://console.developers.google.com/apis/credentials) is a *Web Application* credential following the instructions above. Another case is if you have multiple Google accounts logged into the current browser session, Google may default to an unintended account while switching between pages. To avoid this, log out of other accounts or use a private/incognito browser window with only the desired Google account logged in. +- *Something went wrong: Please contact the developer of this app if the issue persists*: This error typically occurs in two scenarios: + 1. **Wrong credential type**: Ensure you are using a *Web Application* credential (not *Desktop Auth*) in the [Google Cloud Console](https://console.developers.google.com/apis/credentials). + 2. **Multiple Google accounts**: If you have multiple accounts logged into your browser, Google may use an unintended account. To resolve this: + - Log out of other Google accounts, or + - Use a private/incognito browser window with only the desired account logged in.source/_integrations/template.markdown (1)
446-448: Consider enhancing the note with an example.While the note about
thisvsvaluevariables is helpful, adding a small code example would make it even clearer for users.Consider adding an example like this:
{% note %} Self-referecing using `this` provides the current state for the entity. To access the new state, use the `value` variable. + +For example: +{% raw %} +template: + - number: + state: "{{ this.state }}" # Current state + set_value: + service: my_service + data: + new_value: "{{ value }}" # New state being set +{% endraw %} {% endnote %}source/_docs/configuration/templating.markdown (1)
109-111: Enhance the documentation of thethisvariable.The addition of the "This" section is valuable. However, consider enhancing it with:
- A brief example showing common usage patterns
- A note about limitations or scope (where
thisis not available)Consider expanding the section like this:
### This State-based and trigger-based template entities have the special template variable `this` available in their templates and actions. See more details and examples in the [Template integration documentation](/integrations/template). + +For example: +```yaml +template: + - sensor: + name: "Temperature Difference" + state: > + {{ this.state | float - states('sensor.outside_temp') | float }} +``` + +Note: The `this` variable is only available in template entities and is not accessible in other template contexts like automations or scripts.source/changelogs/core-2024.11.markdown (1)
1354-1354: Fix unused link reference.The link reference
[#129970]is defined but not used in the changelog.Remove the unused link reference to fix the markdownlint warning.
🧰 Tools
🪛 Markdownlint
1354-1354: Unused link or image reference definition: "#129970"
Link and image reference definitions should be needed(MD053, link-image-reference-definitions)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
Gemfile.lockis excluded by!**/*.lock
📒 Files selected for processing (15)
_config.yml(1 hunks)source/_docs/configuration/templating.markdown(1 hunks)source/_docs/scene.markdown(1 hunks)source/_integrations/habitica.markdown(2 hunks)source/_integrations/homekit.markdown(2 hunks)source/_integrations/lg_thinq.markdown(9 hunks)source/_integrations/nest.markdown(1 hunks)source/_integrations/notify.rest.markdown(2 hunks)source/_integrations/sensor.markdown(1 hunks)source/_integrations/simplisafe.markdown(1 hunks)source/_integrations/template.markdown(1 hunks)source/_integrations/webostv.markdown(2 hunks)source/_integrations/yale.markdown(1 hunks)source/_posts/2024-11-06-release-202411.markdown(2 hunks)source/changelogs/core-2024.11.markdown(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- _config.yml
- source/_integrations/habitica.markdown
- source/_integrations/lg_thinq.markdown
🧰 Additional context used
🪛 LanguageTool
source/_integrations/webostv.markdown
[uncategorized] ~79-~79: Possible missing comma found.
Context: ...r entity. 3. Look for the source_list attribute which contains all available sources. ...
(AI_HYDRA_LEO_MISSING_COMMA)
🪛 Markdownlint
source/_integrations/webostv.markdown
80-80: Expected: 0 or 2; Actual: 3
Trailing spaces
(MD009, no-trailing-spaces)
83-83: Expected: 0 or 2; Actual: 1
Trailing spaces
(MD009, no-trailing-spaces)
source/_posts/2024-11-06-release-202411.markdown
432-432: Unused link or image reference definition: "#129970"
Link and image reference definitions should be needed
(MD053, link-image-reference-definitions)
source/changelogs/core-2024.11.markdown
1354-1354: Unused link or image reference definition: "#129970"
Link and image reference definitions should be needed
(MD053, link-image-reference-definitions)
🔇 Additional comments (15)
source/_integrations/notify.rest.markdown (1)
43-43: LGTM: Formatting improvement
The authentication description formatting change improves consistency with the rest of the documentation.
source/_docs/scene.markdown (1)
41-41: LGTM! Verify core implementation alignment.
The change to make the description field optional is logical, as scene descriptions should be optional metadata. However, let's ensure this aligns with the core implementation.
Let's verify this change against the core implementation:
Question: Should we add a note about this change in the release notes or migration guide if this was previously required?
✅ Verification successful
Scene description field can be safely marked as optional
Based on the search results from the codebase:
- No schema definitions were found that enforce the description field as required
- Recent changelog entries only show schema-related refactoring but no functional changes
- Historical documentation changes don't show any evidence of description being a required field
Since we couldn't find any code or documentation enforcing the description field as required, marking it as optional in the documentation appears to be correct and aligned with the implementation.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if the scene implementation enforces description field
# Expected: No validation/requirement of description field in the core code
# Search for scene schema or validation code
ast-grep --pattern 'SCENE_SCHEMA = {
$$$
"description": $_,
$$$
}'
# Search for any description-related validation
rg -l 'description.*required' --type python
Length of output: 151
Script:
#!/bin/bash
# Let's try different approaches to find scene-related schema and validation
# Search for scene schema definitions
rg -i "scene.*schema" -A 5 -B 5
# Search for scene-related files
fd -e py scene
# Search for description field in scene-related code
rg "description" -A 3 -B 3 --type-add 'python:*.py' --type python
Length of output: 3871
Script:
#!/bin/bash
# Let's search in YAML files since the documentation is in YAML/Markdown format
# and look for historical changes to scene documentation
# Search for scene description requirements in YAML files
rg "description.*required" --type yaml
# Check git history for the specific file
git log -p source/_docs/scene.markdown | rg -A 5 -B 5 "description.*required"
# Look for scene schema in YAML files
rg "scene.*schema" --type yaml -A 10 -B 10
Length of output: 165
source/_integrations/sensor.markdown (2)
62-62: LGTM! Clear and well-formatted documentation update.
The addition of "ms" (milliseconds) to the duration device class units is clear and follows the existing documentation format.
62-62: Verify milliseconds support in Home Assistant core.
Let's verify that the Home Assistant core implementation supports milliseconds for duration sensors.
✅ Verification successful
Based on the search results, I can generate the final response as we have sufficient evidence about milliseconds support in Home Assistant core:
Milliseconds are fully supported for duration sensors
The search results confirm that milliseconds (ms) are officially supported:
- Added as a valid duration sensor unit in core 2023.4 (PR #90018)
- Explicitly documented in duration selectors with
enable_millisecondoption - Used across multiple core integrations for duration measurements
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if milliseconds support is implemented in the core codebase
# Expected: Find references to millisecond handling in sensor-related core files
# Search for duration device class definition and millisecond handling
rg -A 5 "DEVICE_CLASS_DURATION"
# Search for millisecond conversion or handling in sensor components
rg -i "millisecond|duration.*ms"
Length of output: 8686
source/_integrations/yale.markdown (1)
45-45: LGTM: Device entry follows the established format.
The addition of "Yale Linus L2" to the list of known working devices follows the table's format consistently. The entry correctly specifies that it doesn't require a Connect Bridge or Doorbell, which is valuable information for users.
source/_integrations/webostv.markdown (3)
35-36: LGTM! Clear improvement in documentation.
The rephrasing better explains the requirement for additional integrations to enable TV automation.
70-106: Excellent addition of source switching automation documentation!
The new section is well-structured and provides valuable information:
- Clear instructions for finding available sources
- Practical example with YouTube
- Complete YAML configuration
🧰 Tools
🪛 LanguageTool
[uncategorized] ~79-~79: Possible missing comma found.
Context: ...r entity. 3. Look for the source_list attribute which contains all available sources. ...
(AI_HYDRA_LEO_MISSING_COMMA)
🪛 Markdownlint
80-80: Expected: 0 or 2; Actual: 3
Trailing spaces
(MD009, no-trailing-spaces)
83-83: Expected: 0 or 2; Actual: 1
Trailing spaces
(MD009, no-trailing-spaces)
Line range hint 1-1: Verify PR scope and changes.
The PR objective mentions updating "RESTful notification documentation", but this file is about the webOS TV integration. Please verify if:
- The PR description is accurate
- The correct files are being modified
- Additional files need to be updated
🧰 Tools
🪛 LanguageTool
[typographical] ~38-~38: It seems that a comma is missing.
Context: ...e to use WakeOnLan feature. To use this feature your TV should be connected to your net...
(IN_ORDER_TO_VB_COMMA)
🪛 Markdownlint
37-37: Expected: 1; Actual: 2
Multiple consecutive blank lines
(MD012, no-multiple-blanks)
source/_posts/2024-11-06-release-202411.markdown (2)
401-406: LGTM! Clear and informative section about patch releases.
The new section effectively communicates the patch release schedule and purpose to users.
407-431: LGTM! Well-structured changelog entries.
The patch release entries follow a consistent format and provide clear attribution and tracking via PR numbers.
source/_integrations/template.markdown (1)
444-445: LGTM! Clear documentation of the this variable.
The addition clearly explains the purpose and availability of the special template variable this for state-based and trigger-based template entities.
source/_integrations/homekit.markdown (2)
Line range hint 361-378: LGTM! Documentation improvements enhance clarity.
The changes effectively:
- Clarify the default filtering behavior
- Provide a clear YAML example
- Reference common filter documentation
378-378: LGTM! Clear explanation of entity exclusion behavior.
The addition clearly explains which entities are excluded by default and how to override this behavior through explicit matching or UI selection.
source/changelogs/core-2024.11.markdown (2)
1329-1330: LGTM! Changelog section header follows the standard format.
The section header correctly indicates the version number and release date.
1331-1353: LGTM! Changes are well-documented and properly formatted.
The changelog entries:
- Follow a consistent format with PR numbers and author attribution
- Include important bug fixes, dependency updates, and improvements
- Provide clear descriptions of the changes
thecode
added
the
needs-rebase
|
Ah sorry, looks like this is a bit of a mess because of the change in base branch. Let me fix that. |
|
@thecode Should be all fixed now. |
home-assistant
bot
removed
the
needs-rebase
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
source/_integrations/notify.rest.markdown (1)
Line range hint
77-84: Remove duplicatedataconfiguration entry.There are two conflicting entries for the
dataparameter with different types and descriptions. Based on the PR context, we should keep only the template version.Apply this diff to remove the duplicate and outdated entry:
-data: - description: Dictionary of extra parameters to send to the resource. - required: false - type: string data: description: Template dictionary of extra parameters to send to the resource. required: false type: template
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
source/_integrations/notify.rest.markdown(1 hunks)
🔇 Additional comments (1)
source/_integrations/notify.rest.markdown (1)
72-76: LGTM! Well-documented parameter addition.
The new allow_multiple_targets parameter is properly documented with clear description, type, and default value. The documentation clearly explains both the default behavior and what changes when enabled.
Proposed change
This Home Assistant PR is adding functionality to the RESTful notification for sending multiple targets. This is the corresponding documentation change.
Type of change
currentbranch).currentbranch).nextbranch).nextbranch).Additional information
Checklist
currentbranch.nextbranch.Summary by CodeRabbit
New Features
allow_multiple_targetsfor therestnotification platform, allowing notifications to be sent to multiple targets.Documentation
dataandauthenticationparameters for clarity and consistency.