laravel-notification-channels / twilio

Twilio notifications channel for Laravel
https://laravel-notification-channels.com
227 stars 36 forks source link

Updated config names to match TwilioService. Added ability to explici… #35

Closed ChristopherCarranza closed 6 years ago

ChristopherCarranza commented 6 years ago

I ran across this issue when trying to use a sub-account with an API key in Twilio. After a lot of confusion and back and forth with their support, i got it to work when using the standard PHP SDK that they provide.

When using an API key, you must provide the Account SID, API Key, and API Secret. All 3 can be passed into the RestClient as seen below.

$accountSid = 'ACf54ecc1c1becbe782a63f7ffXXXXXXXX';
$username = 'SKXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX'; // (API Key)
$password = 'your_api_secret'; // (API Secret)
$client = new Client($username, $password, $accountSid);

This library doesn't support this because of the explicit usage of the Account SID. When not using API Keys the username is just your Account SID.

Currently the naming conventions for the configuration items can lead to a lot of confusion when debugging as the Twilio library uses username, password, and accountSid. This PR unifies naming of parameters / config items to make things easier to understand.

'twilio' => [
    'username' => env('TWILIO_USERNAME'),
    'password' => env('TWILIO_PASSWORD'),
    'account_sid' => env('TWILIO_ACCOUNT_SID'), // optional
    'from' => env('TWILIO_FROM'), // optional
],

I know that this is a breaking change as the account_sid is being used as "username" currently. I'm open to other ideas, however logically it just seems to make sense to match Twilio in the long run?