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

Unifonic #113

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open

Conversation

darbaoui
Copy link

Notification Channel for Unifonic
Related to #112

@darbaoui
Copy link
Author

@atymic thanks for your support and fabulous effort in this project, please any update about this channel.

.gitignore Outdated
@@ -0,0 +1,6 @@

Copy link
Member

Choose a reason for hiding this comment

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

remove blank

.gitignore Outdated
build
composer.phar
composer.lock
.phpunit.result.cache
Copy link
Member

Choose a reason for hiding this comment

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

newline for styleci :P


Take a look at our [FAQ](http://laravel-notification-channels.com/) to see our small list of rules, to provide top-notch notification channels.

## Contents
Copy link
Member

Choose a reason for hiding this comment

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

Could you highlight the main markets for this SMS provider please?

README.md Outdated
``` bash
composer require laravel-notification-channels/unifonic
```
This package will register itself automatically if your Laravel 5.5+, trough Package auto-discovery.
Copy link
Member

Choose a reason for hiding this comment

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

Can remove this since we dont support below this

README.md Outdated

In order to let your Notification know which phone numer you are targeting, add the `routeNotificationForUnifonic` method to your Notifiable model.

**Important note**: Unifonic requires the recipients phone number to be without `+` like this format. 21267064497
Copy link
Member

Choose a reason for hiding this comment

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

Could say e164, and just strip the leading plus if it's set
https://developers.omnisend.com/guides/e164-phone-number-formatting

return '21267064497';
}
```

Copy link
Member

Choose a reason for hiding this comment

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

Could we have docs on the avb. methods?

composer.json Outdated
"require": {
"php": ">=7.2",
"guzzlehttp/guzzle": "^6.2|^7.0",
"illuminate/notifications": "~5.8 || ~6.0 || ^7.0 || ^8.0",
Copy link
Member

Choose a reason for hiding this comment

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

drop 5.x

$response = $this->client->request('POST', static::GATEWAY_URL, [
'form_params' => $this->buildMessageParameters($message, $recipient),
'headers' => [
'Content-Type' => 'application/x-www-form-urlencoded',
Copy link
Member

Choose a reason for hiding this comment

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

Don't think you need to set this (set by default)

@darbaoui darbaoui requested a review from atymic July 5, 2021 11:06
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.

2 participants