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.

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

Jump to bottom

FEATURE: Add notifications ressource in wrapper #9

Closed
ryshu opened this issue Feb 18, 2023 · 22 comments
Closed

FEATURE: Add notifications ressource in wrapper #9

ryshu opened this issue Feb 18, 2023 · 22 comments
Assignees
@VeldaKiara
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@ryshu
Copy link
Collaborator

ryshu commented Feb 18, 2023

See https://api.novu.co/api/#/Notification in Novu API endpoints
@unicodeveloper
Copy link
Contributor

@ryshu When can we get this in?

@ryshu
Copy link
Collaborator Author

ryshu commented May 24, 2023

This will take time to do this one, I think we should manage the other tickets before this one so that everyone can benefit from it.

@ryshu ryshu added enhancement New feature or request help wanted Extra attention is needed labels May 24, 2023
@unicodeveloper
Copy link
Contributor

unicodeveloper commented May 24, 2023 via email

@unicodeveloper
Copy link
Contributor

unicodeveloper commented Jun 29, 2023 •
edited
Loading

@ryshu Thanks for getting the other issues fixed. Do you think it's hightime this gets some attention? 😄

We have a few folks asking for this already.

@VeldaKiara
Copy link
Contributor

I want to work on this.

@unicodeveloper
Copy link
Contributor

unicodeveloper commented Jul 6, 2023 via email

@unicodeveloper
Copy link
Contributor

@VeldaKiara How's your research going?

@VeldaKiara
Copy link
Contributor

@unicodeveloper So far, so good.

A couple of things:

I found a couple of broken links:
https://docs.novu.co/overview/quick-start
https://docs.novu.co/overview/quick-start#create-a-notification-template

Also, this is how I am thinking about it:

  • Add the NotificationAPI on the docs that will handle all API methods around notifications in Novu. It will also have the following : stats, graph stats and specific IDs

  • Add the DTO for notification say call it class novu.dto.Notifications, then have parameters like channels, templates, emails, search as required fields and others like page and transaction ID

  • Replicate the DTO to also accommodate graphs stats, stats and filtering by transaction_id.

  • Get to the actual implementation and test it out.

Let me know if I missed something and any more clarification

@unicodeveloper
Copy link
Contributor

@VeldaKiara Where did you find these broken links so it can be updated?

  1. First link is now https://docs.novu.co/overview/quickstart/general-quickstart
  2. Second link is now https://docs.novu.co/overview/quickstart/general-quickstart#create-a-workflow

Yes, I think that works.

@ryshu Have any other opinion?

@VeldaKiara
Copy link
Contributor

Screenshot 2023-07-07 at 21 10 29

Screenshot 2023-07-07 at 21 11 14

I found the link on quickstart on the get started page after you set up an account, then create a notification template link, I found it on the readme file

@unicodeveloper
Copy link
Contributor

Thanks for the catch. We will update it as soon as possible! @VeldaKiara

@VeldaKiara
Copy link
Contributor

VeldaKiara commented Jul 7, 2023 •
edited
Loading

Welcome, I am moving forward with the task.

@unicodeveloper
Copy link
Contributor

Fixed

Screenshot 2023-07-07 at 19 42 23

@VeldaKiara
Copy link
Contributor

Awesome 🔥🔥🔥

@VeldaKiara
Copy link
Contributor

I am still working on this will give an update before end of day today

@unicodeveloper
Copy link
Contributor

No problem

VeldaKiara added a commit to VeldaKiara/novu-python that referenced this issue Jul 11, 2023
@VeldaKiara
FEATURE: Add notifications ressource in wrapper novuhq#9(update the a…
ef56cc6 
…pi and dto)
@VeldaKiara
Copy link
Contributor

I completed updating the API and DTO, but I still have ongoing issues with some failing tests.
I realized that I had issues with URLs on the testing part that is instead of calling GET
[/v1/notifications] I was calling it twice while testing.
I am still making changes.

@unicodeveloper
Copy link
Contributor

Is there a PR to this effect? @VeldaKiara

@VeldaKiara
Copy link
Contributor

I have not submitted a PR since some of the tests have yet to pass. I committed to the changes to the other files pending this.

Here: VeldaKiara@ef56cc6

@unicodeveloper
Copy link
Contributor

Alright, looking forward to it.

@VeldaKiara
Copy link
Contributor

My tests finally passed. My major issues were caused by not checking the base.py file, which also implemented some of the params. I kept getting the "multiple values for keyword argument 'params'" error, which I resolved by assigning payload to the payload on the response handling requests.
I am formating the code and will be sending in a PR in a few.

@VeldaKiara
Copy link
Contributor

The PR is here: https://github.com/novuhq/novu-python/pull/44

unicodeveloper pushed a commit that referenced this issue Sep 13, 2023
@VeldaKiara @unicodeveloper
FEATURE: Add notifications ressource in wrapper #9(update the api and…
061e5cc 
… dto)
ryshu pushed a commit that referenced this issue Sep 13, 2023
@VeldaKiara @ryshu
feat(#9): add notifications ressource in wrappers
9502293
ryshu pushed a commit that referenced this issue Sep 13, 2023
@VeldaKiara @ryshu
feat(#9): add notifications ressource in wrappers
f33a32f
unicodeveloper pushed a commit that referenced this issue Sep 13, 2023
@semantic-release-bot
chore(release): 1.4.0
be6817f 
# [1.4.0](https://github.com/novuhq/novu-python/compare/v1.3.1...v1.4.0) (2023-09-13)

### Bug Fixes

* **#54:** SubscriberApi.list() not returning credentials, field email is missing in SubscriberDto ([#55](https://github.com/novuhq/novu-python/issues/55)) ([5d67f52](https://github.com/novuhq/novu-python/commit/5d67f52c3b844821ad5191a1d840a0d1e464ee12)), closes [#54](https://github.com/novuhq/novu-python/issues/54)
* **feed:** delete a feed doesn't return any data ([84c3ccb](https://github.com/novuhq/novu-python/commit/84c3ccb2ae9848f2b35475147ef1f9353c901acb))

### Features

* **#9:** add notifications ressource in wrappers ([f33a32f](https://github.com/novuhq/novu-python/commit/f33a32fc788085516515ba7a3fa54181248e43bf)), closes [#9](https://github.com/novuhq/novu-python/issues/9)
* **tenant:** introduce Tenant API ([330cb19](https://github.com/novuhq/novu-python/commit/330cb19a679f1284487acd8f6beabd3edc681448))
@ryshu ryshu closed this as completed Sep 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@VeldaKiara VeldaKiara

Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@unicodeveloper @ryshu @VeldaKiara