-
-
Notifications
You must be signed in to change notification settings - Fork 14
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add support to send Teams messages as HTML content #224
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @pantherale0 - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟡 General issues: 3 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.
content_type: | ||
name: Content Type | ||
description: The type of content to send, html if you are sending a HTML message or text for plain text | ||
example: text | ||
required: false | ||
selector: | ||
select: | ||
mode: dropdown | ||
options: | ||
- label: HTML |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion (code_refinement): Consider marking 'content_type' as required if it's critical for message formatting.
If the content type is essential for the message to be formatted correctly, it might be better to enforce its specification by making it required.
@@ -168,15 +168,15 @@ def extra_state_attributes(self): | |||
attributes[ATTR_DATA] = self.coordinator.data[self.entity_key][ATTR_DATA] | |||
return attributes | |||
|
|||
def send_chat_message(self, chat_id, message): | |||
def send_chat_message(self, chat_id, message, content_type="text"): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion (code_refinement): Validate 'content_type' parameter to ensure it only accepts 'text' or 'html'.
Adding a validation step for 'content_type' can prevent errors from unsupported types and ensure that the function behaves as expected.
def send_chat_message(self, chat_id, message, content_type="text"): | |
def send_chat_message(self, chat_id, message, content_type="text"): | |
valid_content_types = ["text", "image", "video"] | |
if content_type not in valid_content_types: | |
raise ValueError("Unsupported content type") |
@@ -148,6 +148,7 @@ target: | |||
data: | |||
chat_id: xxxxxxxxxxxxxxxxxxxxxxxxx | |||
message: Hello world | |||
content_type: text |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion (code_clarification): Update documentation to explain the optional nature of 'content_type' and its defaults.
Expanding the documentation to clarify when and how to use the 'content_type' field can enhance user understanding and correct usage of the API.
content_type: text | |
content_type: text | |
# The `content_type` field specifies the type of content being sent. | |
# Use this field to define the format of the message content. | |
# For example, 'text' indicates that the message content is plain text. |
Thank you!! |
Hi,
Created this PR so that messages can be sent in HTML format alongside plain text. Text is set as the default, but can be overridden to html which will allow inserting html such as:
Also I've cleaned up
CHAT_SERVICE_SEND_MESSAGE_SCHEMA
as this had been defined three times which shouldn't be necessary.