Skip to content

Add missing 'device_id' attr in Webhook class #35

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.

Sign up for GitHub

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

Closed
vk496 wants to merge 4 commits into from
Closed

Add missing 'device_id' attr in Webhook class #35

Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a10c650
Add missing device_id attr in Webhook class
vk496 Jan 24, 2026
1ffaf38
match deviceId with API
vk496 Jan 24, 2026
a387b83
Set default value for device_id in webhook config
vk496 Jan 25, 2026
de3be64
Reorder device_id declaration in Webhook class
vk496 Jan 25, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions android_sms_gateway/domain.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,8 @@ class Webhook:

id: t.Optional[str]
"""The unique identifier of the webhook."""
device_id: t.Optional[str] = None
"""The unique identifier of the device the webhook is associated with."""
url: str
"""The URL the webhook will be sent to."""
event: WebhookEvent
Expand All @@ -215,6 +217,7 @@ def from_dict(cls, payload: t.Dict[str, t.Any]) -> "Webhook":
"""
return cls(
id=payload.get("id"),
device_id=payload.get("deviceId", None),
url=payload["url"],
event=WebhookEvent(payload["event"]),
)
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Expand All @@ -227,6 +230,7 @@ def asdict(self) -> t.Dict[str, t.Any]:
"""
return {
"id": self.id,
"deviceId": self.device_id,
"url": self.url,
"event": self.event.value,
}
Comment on lines 231 to 236
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Jan 25, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# First, let's find the domain.py file and examine its structure
find . -name "domain.py" -path "*/android_sms_gateway/*" -type f

Repository: android-sms-gateway/client-py

Length of output: 104

🏁 Script executed:

#!/bin/bash
# Look at the Webhook class and Message class asdict implementations
rg -A 15 "class Webhook" --type py
rg -A 15 "class Message" --type py

Repository: android-sms-gateway/client-py

Length of output: 4939

🏁 Script executed:

#!/bin/bash
# Search for Message.asdict() to see how it handles optional fields
rg -B 2 -A 10 "def asdict" --type py

Repository: android-sms-gateway/client-py

Length of output: 3877

🏁 Script executed:

#!/bin/bash
# Get the complete Message.asdict() implementation
rg -B 5 -A 40 "class Message:" --type py | head -80

Repository: android-sms-gateway/client-py

Length of output: 3181

🏁 Script executed:

#!/bin/bash
# Search for device_id within Message.asdict() context
rg -A 50 "def asdict.*Message" --type py

Repository: android-sms-gateway/client-py

Length of output: 55

🏁 Script executed:

#!/bin/bash
# More targeted search for Message's asdict and how it handles device_id
sed -n '/^class Message:/,/^class [A-Z]/p' android_sms_gateway/domain.py | grep -A 50 "def asdict"

Repository: android-sms-gateway/client-py

Length of output: 866

🏁 Script executed:

#!/bin/bash
# Search for webhook-related tests or API calls
rg -i "webhook" --type py -B 2 -A 5 | grep -i -A 5 "create\|update\|deviceid"

Repository: android-sms-gateway/client-py

Length of output: 4480

🏁 Script executed:

#!/bin/bash
# Check test files for webhook creation patterns
fd -e "test_*.py" -o -e "*_test.py" | head -20

Repository: android-sms-gateway/client-py

Length of output: 189

🏁 Script executed:

#!/bin/bash
# Look for any documented issues or comments about deviceId
rg "deviceId" --type py -B 2 -A 2

Repository: android-sms-gateway/client-py

Length of output: 1638

🏁 Script executed:

#!/bin/bash
# Get the full webhook create test to see what's actually sent
rg -A 20 "def test_webhook_create\(" tests/test_client.py

Repository: android-sms-gateway/client-py

Length of output: 753

🏁 Script executed:

#!/bin/bash
# Look for webhook asdict tests
rg -B 5 -A 15 "webhook.*asdict\|asdict.*webhook" --type py

Repository: android-sms-gateway/client-py

Length of output: 55

🏁 Script executed:

#!/bin/bash
# Check if there are any tests that explicitly test Webhook.asdict()
rg "Webhook.*asdict" --type py -A 5 -B 5

Repository: android-sms-gateway/client-py

Length of output: 55

🏁 Script executed:

#!/bin/bash
# Check how webhook.asdict() is actually used in the client
rg -B 3 -A 3 "webhook.*asdict\(\)" --type py

Repository: android-sms-gateway/client-py

Length of output: 1868

🏁 Script executed:

#!/bin/bash
# Look at the create_webhook implementation in detail
rg -A 20 "def create_webhook" android_sms_gateway/client.py

Repository: android-sms-gateway/client-py

Length of output: 1183

🏁 Script executed:

#!/bin/bash
# Check what http methods are being called with webhook data
rg "self.http.*webhook" -B 2 -A 2 --type py

Repository: android-sms-gateway/client-py

Length of output: 781

🏁 Script executed:

#!/bin/bash
# Get the complete webhook asdict test
rg -A 30 "def test_webhook_asdict" tests/test_domain.py

Repository: android-sms-gateway/client-py

Length of output: 929

🏁 Script executed:

#!/bin/bash
# Get the rest of the test to see the full expected_dict when device_id is None
rg -A 50 "def test_webhook_asdict" tests/test_domain.py | tail -20

Repository: android-sms-gateway/client-py

Length of output: 575

Webhook.asdict() should omit deviceId key when not set.

The current implementation always includes "deviceId": None in the returned dictionary, but the test test_webhook_asdict() expects the key to be absent when device_id is unset. This inconsistency can cause API validation errors and unintended side effects. The codebase already follows this pattern in Message.asdict(), which only includes non-None fields. Update Webhook.asdict() to conditionally include deviceId only when it's set:

Suggested fix 
def asdict(self) -> t.Dict[str, t.Any]:
    """Returns a dictionary representation of the webhook.

    Returns:
        A dictionary containing the webhook's data.
    """
    result = {
        "id": self.id,
        "url": self.url,
        "event": self.event.value,
    }
    if self.device_id is not None:
        result["deviceId"] = self.device_id
    return result
🤖 Prompt for AI Agents 
In `@android_sms_gateway/domain.py` around lines 231 - 236, Webhook.asdict()
currently always emits "deviceId" even when self.device_id is None; change
Webhook.asdict() to follow the Message.asdict() pattern and only add the
"deviceId" key when self.device_id is not None (i.e., construct the dict with
id, url, event and then conditionally set "deviceId": self.device_id if
self.device_id is truthy/ not None) so the returned mapping omits deviceId when
unset.

Expand Down