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

Added Pushy Messaging Adapter #41

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Diveshmahajan4
Copy link

What does this PR do?

Adds a new Push Adapter for sending push notification via Pushy.

Test Plan

  • SignIn and Create a new Application on Pushy
  • Add the following environment variables
    • PUSHY_SECRET_KEY: < APP_NAME > API Authentication Tab >
    • PUSHY_SERVER_TO: <userId e.g. 'test-user-id'>
  • Run test against tests/e2e/Push/PushyTest.php

Related PRs and Issues

Closes appwrite/appwrite#6400

Have you read the Contributing Guidelines on issues?

Yes

@gewenyu99
Copy link

Hey,

Due to time constraints, I'm going to mark this PR hacktoberfest-accepted for now so you get DO's Hacktoberfest rewards. We'll continue to work with you on this issue for review and merge.

When it is merged, we'll contact you for Appwrite-specific Hacktoberfest swag.

Thanks for helping us improve Appwrite!

*/
public function getMaxMessagesPerRequest(): int
{
return 1000;
Copy link
Member

Choose a reason for hiding this comment

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

Did you confirm from the documentation that this limit is 1000 for Pushy?

Copy link
Member

Choose a reason for hiding this comment

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

@Diveshmahajan4 The limit here should be 100000

@vermakhushboo
Copy link
Member

Hey @Diveshmahajan4! 👋🏼 Thank you for your contribution. The PR looks good overall. Please run composer format to fix the linter errors and post a screenshot of tests passing locally. 😄

@tessamero
Copy link

Hello @Diveshmahajan4

Thank you for your contribution to Hacktoberfest 2023! We've noticed that your PR is still pending and requires some updates based on our engineering team's feedback.

We would love to see your PR successfully merged and send you the Appwrite swag as a token of appreciation. To remain eligible for the swag, please address the pending suggestions and/or ensure the tests pass by Friday, November 17th. If the PR isn't updated by then, we will unfortunately have to close it due to the end of the Hacktoberfest event.

Looking forward to your updates and thank you!

@gewenyu99
Copy link

Hey there! There were a lot of big PRs during this Hacktoberfest, and we wanted to give everyone ample time to collaborate with our engineering team. If you were able to merge your PRs during October, amazing. If it’s still not merged, don’t worry about it either. Either way, we’ve got your Hacktoberfest swag minted and ready to ship.

Please comment with your Discord username here so we can contact you about your shipping information to deliver your Hacktoberfest swag.

@Diveshmahajan4
Copy link
Author

Diveshmahajan4 commented Apr 8, 2024 via email

@gewenyu99
Copy link

Will reach out soon. Appreciate patience. Adding you to a giant list of people to reach out to!

*/
public function process(Push $message): string
{

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change

body: \json_encode([
'to' => $message->getTo(),
'data' => $message->getData(),
'notifications' => [
Copy link
Member

Choose a reason for hiding this comment

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

I think this should be notification instead of notifications

*/
public function getMaxMessagesPerRequest(): int
{
return 1000;
Copy link
Member

Choose a reason for hiding this comment

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

@Diveshmahajan4 The limit here should be 100000

public function process(Push $message): string
{

return $this->request(
Copy link
Member

Choose a reason for hiding this comment

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

This should build a structured response consistent with other adapters, please check APNS/FCM for an idea on how to do so

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

💬 Improve Appwrite Messaging with Pushy Adapter
5 participants