Skip to content
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

novu-python fails to parse response from setting push notification provider credentials #148

Closed
danbim opened this issue Oct 30, 2023 · 0 comments

Comments

@danbim
Copy link
Contributor

danbim commented Oct 30, 2023

Describe the bug
The current implementation has a bug that causes unexpected behavior under specific circumstances. This issue needs to be addressed to ensure the application functions reliably in all scenarios.
To Reproduce
Steps to reproduce the behavior:
Call SubscriberApi().credentials(user_id, provider_id, webhook_url, device_tokens) with empty webhook_url but with device_tokens and provider_id='apns' (for example) in order to set up push notifications. The request on server side will succeed but parsing the response will fail with:

TypeError: SubscriberChannelSettingsCredentialsDto.__init__() missing 2 required positional arguments: 'webhook_url' and 'channel'

Expected behavior
Call succeeds and successfully parses response of type SubscriberDto.

Screenshots
[Attach relevant screenshots, if available, to provide visual context for the issue.]

Versions (please complete the following information):

  • Novu version: SaaS
  • Python version: 3.11.4
  • Novu-python version: 1.9.1

Additional context

Note to Reviewers:
I believe the fix is to make webhook_url and channel optional which makes sense to me as they won't be set if I only set up a push provider.

@dataclasses.dataclass
class SubscriberChannelSettingsCredentialsDto(CamelCaseDto["SubscriberChannelSettingsCredentialsDto"]):
    """Credentials payload for the specified provider."""

    webhook_url: Optional[str] = None
    """Webhook url used by chat app integrations. The webhook should be obtained from the chat app provider"""

    channel: Optional[str] = None
    """Channel specification for Mattermost chat notifications"""

    device_tokens: Optional[List[str]] = None
    """Contains an array of the subscriber device tokens for a given provider. Used on Push integrations"""
@danbim danbim changed the title novu-python fails to parse response from settings push notification provider credentials novu-python fails to parse response from setting push notification provider credentials Oct 31, 2023
ryshu pushed a commit that referenced this issue Oct 31, 2023
ryshu pushed a commit that referenced this issue Oct 31, 2023
ryshu pushed a commit that referenced this issue Oct 31, 2023
unicodeveloper pushed a commit that referenced this issue Nov 1, 2023
# [1.10.0](v1.9.1...v1.10.0) (2023-11-01)

### Bug Fixes

* [#148](#148) make webhook_url and channel optional in SubscriberChannelSettingsCredentialsDto ([a334e48](a334e48))

### Features

* **api:** add transaction id param to  MessageApi list method ([4fc5dad](4fc5dad))
* **deps:** update dependency sentry-sdk to v1.33.0 ([939e10d](939e10d))
* **deps:** update dependency sentry-sdk to v1.33.1 ([7d0f327](7d0f327))
* **deps:** update dependency sphinx to v7.2.6 ([f430a40](f430a40))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant