Skip to content

[Notifier] Add from in SmsMessage and support it in bridge transports #16701

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

Merged

Conversation

alamirault
Copy link
Contributor

If PR #45987 looks good. I will change docs

@alamirault alamirault requested a review from xabbuh as a code owner April 10, 2022 19:05
@carsonbot carsonbot added this to the 6.1 milestone Apr 10, 2022
@carsonbot carsonbot changed the title [WIP] [Notifier] Add from in SmsMessage and support it in bridge transports [Notifier] [WIP] Add from in SmsMessage and support it in bridge transports Apr 10, 2022
@alamirault alamirault marked this pull request as draft April 10, 2022 19:06
@OskarStark
Copy link
Contributor

These changes look totally unrelated 🙄

@wouterj wouterj modified the milestones: 6.1, next May 1, 2022
@wouterj wouterj added the Waiting Code Merge Docs for features pending to be merged label May 1, 2022
fabpot added a commit to symfony/symfony that referenced this pull request Jul 26, 2022
This PR was merged into the 6.2 branch.

Discussion
----------

[Notifier] Add `from` in `SmsMessage`

| Q             | A
| ------------- | ---
| Branch?       | 6.1
| Bug fix?      | no
| New feature?  | yes
| Deprecations? | no
| Tickets       | Fix #45435
| License       | MIT
| Doc PR        | symfony/symfony-docs#16701 [WIP]

This PR allow to define `From` in `SmsMessage`. When it defined, it override default `from` in transports.

```php
$sms = new SmsMessage(
  // the phone number to send the SMS message to
  '+1411111111',
  // the message
  'A new login was detected!',
  // [Optional] the From SMS message, it can also be a name
  '+1422222222',
);

// Can also be defined with setter
$sms->setFrom('+1422222222');

$sentMessage = $texter->send($sms);
```

Commits
-------

21b5051 [Notifier] Add from in SmsMessage and support it in bridge transports
@javiereguiluz javiereguiluz removed the Waiting Code Merge Docs for features pending to be merged label Oct 11, 2022
@javiereguiluz
Copy link
Member

The related code PR was merged: symfony/symfony#45987 🎉

@alamirault would you have time to finish this PR? If you can't fix the git issues with the unrelated commits, it's OK to delete this PR and create a new one. Thanks.

@alamirault
Copy link
Contributor Author

alamirault commented Oct 11, 2022

I'm not sure where I have to do changes.

Is edit SmsMessage example is enough ? https://github.com/symfony/symfony-docs/blame/6.2/notifier/texters.rst#L22 (Like in PR description) ?

@alamirault alamirault force-pushed the feature-45435-notifier-sms-message-add-from branch from 44aaa9e to b911480 Compare October 13, 2022 19:35
@alamirault alamirault changed the base branch from 6.1 to 6.2 October 13, 2022 19:36
@alamirault alamirault changed the title [Notifier] [WIP] Add from in SmsMessage and support it in bridge transports [Notifier] Add from in SmsMessage and support it in bridge transports Oct 13, 2022
@alamirault alamirault marked this pull request as ready for review October 13, 2022 19:36
@carsonbot carsonbot modified the milestones: next, 6.2 Oct 13, 2022
@alamirault
Copy link
Contributor Author

@javiereguiluz I rebased and changed documentation

@wouterj wouterj force-pushed the feature-45435-notifier-sms-message-add-from branch from 48313bc to d627e1c Compare October 23, 2022 13:43
@wouterj
Copy link
Member

wouterj commented Oct 23, 2022

Thanks @alamirault!

@wouterj wouterj merged commit 91edb50 into symfony:6.2 Oct 23, 2022
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.

5 participants