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

[2.x] Fix shortcode messaging with the latest version of nexmo-client #37

Merged
merged 1 commit into from
Aug 7, 2020
Merged

[2.x] Fix shortcode messaging with the latest version of nexmo-client #37

merged 1 commit into from
Aug 7, 2020

Conversation

dwightwatson
Copy link
Contributor

With the latest release(s) of nexmo-client (v2.2.0+) they've made some changes internally, which affect how Nexmo\Message\Client uses the Nexmo\Client that is passed into it. This breaks the ability to send shortcodes when you have the latest versions of nexmo-client.

Previously I used the container to create a new instance of Nexmo\Message\Client and Laravel was being smart and giving it an instance of an optional dependency. However, this optional dependency now overrides the custom Nexmo\Client we're setting here.

The easy fix is to just not provide the optional dependency and new up the instance as-is. Alternatively I could register it directly with the service provider so it knows not to pass any dependency but that felt like overkill in this instance.

@driesvints driesvints changed the title Fix shortcode messaging with the latest version of nexmo-client [2.x] Fix shortcode messaging with the latest version of nexmo-client Aug 7, 2020
@driesvints
Copy link
Member

It seems https://github.com/Nexmo/nexmo-php-complete doesn't have a 2.0 tag yet. Since nexmo/laravel uses this package we can't merge this until it does.

I've sent in Nexmo/nexmo-laravel#45 so nexmo-laravel might get rid of the unnecessary extra dependency in between.

@driesvints driesvints marked this pull request as draft August 7, 2020 10:32
@driesvints
Copy link
Member

Converted this to draft until the other PR has been handled.

@driesvints driesvints marked this pull request as ready for review August 7, 2020 13:27
@driesvints
Copy link
Member

As Chris explained in Nexmo/nexmo-laravel#45 it seems it does already have a 2.0.0 tag. So this PR is gtg for me.

@taylorotwell taylorotwell merged commit 76a7e25 into laravel:2.0 Aug 7, 2020
@dwightwatson dwightwatson deleted the message-client branch August 11, 2020 03:55
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.

3 participants