JonasJoKuJonas / homeassistant-WebUntis

Custom component to access data from Web Untis in Home Assistant
https://community.home-assistant.io/t/webuntis-timetable-in-ha/568273
MIT License
52 stars 12 forks source link

Error in Notification with "Message" only configuration #156

Closed cofw2005 closed 2 weeks ago

cofw2005 commented 2 weeks ago

The problem

First, explain the background: I am using two notify service configured in the integration: Telegram and Whatsapp.

I configured in the "Notify Template" with "Message & Title" in both notify services. Then I realized in the Whatsapp message, there is no title in the message. It is because the notify.whatsapp has no "title" parameter. So the action only send the "message" content.

Based on the behavior, I changed configuration of Whatsapp notify service with "Message" only. Unfortuantely then I got a error message: Sending notification to notify.whatsapp failed - required key not provided @ data['message']

I tested with telegram notify service, I got same error message if I changed configuration of Whatsapp notify service with "Message" only.

Version of Home Assistant?

2024.8.3

What type of installation are you running?

Home Assistant OS

Version of WebUntis

v1.2.5

Last working WebUntis version

n.a.

Traceback/Error logs

Sending notification to notify.telegram failed - required key not provided @ data['message']

Additional information

-------------------- here is my investigation ----------------------

I looked in the notify.py, and found the problem in this function:

def get_notification_data(changes, service, entry_title):

    message = ""
    title = ""
    data = {}

    template = service.get("template", TEMPLATE_OPTIONS[0])

    if template == "message_title":
        title = f"WebUntis ({entry_title}) - {changes['title']}"
        message = f"""Subject: {changes["subject"]}
Date: {changes["date"]}
Time: {changes["time_start"]} - {changes["time_end"]}"""

        if changes["change"] not in ["cancelled", "test"]:
            message += f"""{changes["change"]}
Old: {changes["old"]}
New: {changes["new"]}"""

    elif template == "message":
        message = f"{title}\n{message}"

    elif template == "discord":
        ....

in the beginning of the fuction, all three parameters were set to empty. title and message get only values in the first if. And in the second if, this message = f"{title}\n{message}" actually only gives message an empty value. I think this is the reason of the error message.

Based on the investigation, I changed my local "notify.py" in this way:

def get_notification_data(changes, service, entry_title):

    data = {}
    title = f"WebUntis ({entry_title}) - {changes['title']}"
    message = f"""Subject: {changes["subject"]}
Date: {changes["date"]}
Time: {changes["time_start"]} - {changes["time_end"]}"""

    if changes["change"] not in ["cancelled", "test"]:
        message += f"""{changes["change"]}
Old: {changes["old"]}
New: {changes["new"]}"""

    template = service.get("template", TEMPLATE_OPTIONS[0])

    if template == "message_title":
        title = title
        message = message

    elif template == "message":
        message = f"{title}\n{message}"

    elif template == "discord":
        ...

With this modification, it can send correct message in both "Message & Title" and "Message" notify templates.

-------------------- additionally, I also found something else in this function ----------------------

In the return part, this function always return all three parameters no matter what notify template selected.

    return {
        "message": message,
        "title": title,
        "data": data,
    }

This caused that even though I select only "Message" in the notify.telegram template, I still got a title line, which make it showing the title lines two times in the message. So I also adapted the code to merge the return in the the if directly:

    if template == "message_title":
        return {
            "message": message,
            "title": title,
        }

    elif template == "message":
        return {
            "message": f"{title}\n{message}",
        }

    elif template == "discord":
        data = {
        ...
        }

        return {
            "data": data,
        }

Now I think it works perfect for me. I hope this post can help for the improvement.

JonasJoKuJonas commented 2 weeks ago

Thank you for the detailed issue description ♥️. I fixed the bug by adding a second case to the title & message if. And creating an result dict to prevent the need of aditional if cases.

Fixed in v1.3.0

cofw2005 commented 2 weeks ago

I had a look at the changed code, not sure if it is the expected behavior. When you add the or template == "message", it result that this elif below will be prevented.

    elif template == "message":
        message = f"{title}\n{message}"

From the previous code version, I interpreted that when we configure to "Message" only, the title is intended to be combined into the message content. With this update, this is skipped, right? In real case, the message will have only a lesson subject and time, but we cannot see if it is cancelled or changed, because {changes['title']} is only in "title" content, not in "message".

And by the way, is it really necessary always return all the three para (title, message, data) in dict?

JonasJoKuJonas commented 2 weeks ago

Yes. The message need to be returned all the time thanks for the elif info. I only testet it via a discord notification because There it will work but on telegram probably not

cofw2005 commented 2 weeks ago

A short hint after trial with new version. In the notify.py you implemeted new function get_homework_notification, the if ... elif structure should also be corrected in the same way.