Conversation
sinch/domains/conversation/webhooks/v1/events/conversation_webhooks_event.py
Outdated
Show resolved
Hide resolved
sinch/domains/conversation/webhooks/v1/conversation_webhooks.py
Outdated
Show resolved
Hide resolved
tests/unit/domains/conversation/v1/webhooks/test_conversation_webhooks.py
Outdated
Show resolved
Hide resolved
|
|
||
| def conversation_event(self): | ||
| headers = dict(request.headers) | ||
| body_str = request.raw_body.decode("utf-8") if request.raw_body else "" |
There was a problem hiding this comment.
Here assuming that content-type header is not present or do not contains charset value.
(current deployed backend do not specify any charset today, so fallback to UTF8 is fine)
Ideally body should be decoded according to content-type + charset and fallback to utf-8 only if not set
There was a problem hiding this comment.
What conduce the choice to have models for webhooks not under models directory like other models related to V1 ? (e.g. https://github.com/sinch/sinch-sdk-java/tree/feat/V2.0-next/openapi-contracts/src/main/com/sinch/sdk/domains/conversation/models/v1/webhooks/events and https://github.com/sinch/sinch-sdk-node/tree/main/packages/conversation/src/models/v1/mod-callback-events)
There was a problem hiding this comment.
As discussed, moved under models/
| channel_metadata: Optional[Any] = Field( | ||
| default=None, | ||
| description="Additional metadata from the channel.", |
There was a problem hiding this comment.
Specification are not defining it as a common field all webhooks events
| default=None, | ||
| description="The ID of the conversation.", | ||
| ) | ||
| status: Optional[StrictStr] = Field( |
There was a problem hiding this comment.
Specification are containing a list of supported values.
They should be used to create an enum
There was a problem hiding this comment.
Just created MessageDeliveryStatus
| default=None, | ||
| description="The ID of the contact. Empty if processing_mode is DISPATCH.", | ||
| ) | ||
| submitted_message: Optional[Any] = Field( |
There was a problem hiding this comment.
According to specs, should be an AppMessage no ?
| if not secret: | ||
| return False | ||
| if isinstance(payload, bytes): | ||
| payload = payload.decode("utf-8") |
There was a problem hiding this comment.
Same point about not processing content-type to get charset to decode payload
| if not _has_fetched_event(context): | ||
| return |
There was a problem hiding this comment.
Could this conduce to a false positive during test ? (same at multiple location)
I mean: if there is never _has_fetched_event returning true, then test will be ok right ?
There was a problem hiding this comment.
Yes, it's returning FPs.
I will have to refactor this logic for now, and will parametrize again when other statuses are implemented.
| def process_event(context, response): | ||
| context.formatted_headers = dict(response.headers) | ||
| context.raw_event = response.text | ||
| context.event = context.conversation_webhooks.parse_event(context.raw_event) | ||
|
|
||
|
|
||
| def _fetch_and_process(context, path_suffix): | ||
| base_url = context.sinch.configuration.conversation_origin | ||
| url = f"{base_url}/webhooks/conversation/{path_suffix}" | ||
| response = requests.get(url) | ||
| process_event(context, response) | ||
|
|
||
|
|
||
| def _has_fetched_event(context): | ||
| return getattr(context, "raw_event", None) is not None | ||
|
|
||
|
|
||
| def _submitted_has(submitted, key): | ||
| if submitted is None: | ||
| return False | ||
| if isinstance(submitted, dict): | ||
| return key in submitted and submitted[key] is not None | ||
| return getattr(submitted, key, None) is not None |
There was a problem hiding this comment.
This pattern is different from the ones with other domains but there is crossover between them.
This should be part of a common "utility" helpers for e2e
There was a problem hiding this comment.
Moved to helpers.py
3 participants
No description provided.