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

feat: return api response to be consumed by event listener #9

Merged
merged 1 commit into from
Feb 2, 2023
Merged

feat: return api response to be consumed by event listener #9

merged 1 commit into from
Feb 2, 2023

Conversation

ankurk91
Copy link
Contributor

@ankurk91 ankurk91 commented Feb 2, 2023

Hi,

The channel should return the api response so that it can be consumed by the Event Listeners.

Here is an example:

// EventServiceProvider.php


protected $listen = [
    NotificationSent::class => [
        VoiceCallSentNotificationListener::class,
    ],
];
// VoiceCallSentNotificationListener.php

public function handle(NotificationSent $event)
{
    // $event->channel // voice
    // $event->notifiable
    // $event->notification
    // $event->response // api response from vonage
}

These changes should not break anything.

You can read about it here

https://laravel.com/docs/9.x/notifications#notification-events

@dwightwatson dwightwatson merged commit 4caf593 into roomies-com:master Feb 2, 2023
@dwightwatson
Copy link
Member

This is cool, thank you.

I guess it is technically a breaking change by having the return type change.

I might pop it in a new major release to be safe.

@ankurk91
Copy link
Contributor Author

ankurk91 commented Feb 2, 2023

I don't think someone is consuming the events. It should NOT affect majority of people.

I don't want this feature urgently in my project. feel free to release a new major version later according to your schedule.

May be after this PR
laravel/vonage-notification-channel#72

@dwightwatson
Copy link
Member

Good call, I've subscribed to that PR and will make sure we're up to date with vonage-core too.

@ankurk91
Copy link
Contributor Author

ankurk91 commented Feb 8, 2023

The PR is merged and new version has been tagged.

@dwightwatson
Copy link
Member

dwightwatson commented Feb 8, 2023

Cool, I've updated the underlying dependency and dropped PHP 8.0 support in #10

Tagged a new release 6.0.0

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

Successfully merging this pull request may close these issues.

2 participants