Skip to content

[Notifier] Add from in SmsMessage #45987

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jul 26, 2022
Merged

[Notifier] Add from in SmsMessage #45987

merged 1 commit into from
Jul 26, 2022

Conversation

alamirault
Copy link
Contributor

@alamirault alamirault commented Apr 10, 2022 •
edited
Loading

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.

$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);

@carsonbot carsonbot added this to the 6.1 milestone Apr 10, 2022
@alamirault alamirault mentioned this pull request Apr 10, 2022
Merged
@alamirault alamirault force-pushed the feature-45435-notifier-sms-message-add-from branch from fc6ea52 to f2e2be8 Compare April 10, 2022 19:11
@alamirault alamirault requested a review from fabpot May 10, 2022 10:21
@alamirault
Copy link
Contributor Author

alamirault commented May 11, 2022 •
edited
Loading

Hope it's ok for you now

@fabpot fabpot modified the milestones: 6.1, 6.2 May 20, 2022
fabpot
fabpot requested changes Jul 21, 2022
View reviewed changes
Copy link
Member

@fabpot fabpot left a comment

Choose a reason for hiding this comment

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

Can you also rebase on current 6.2 to get rid of the merge commit?

@OskarStark OskarStark changed the title [Notifier] Add from in SmsMessage and support it in bridge transports [Notifier] Add from in SmsMessage Jul 22, 2022
@alamirault alamirault force-pushed the feature-45435-notifier-sms-message-add-from branch from 76144c9 to 21b5051 Compare July 25, 2022 20:26
@alamirault alamirault requested review from OskarStark and fabpot July 25, 2022 20:32
@alamirault
Copy link
Contributor Author

@fabpot @OskarStark I rebased on current 6.2 and removed nullability of "from"

@fabpot
Copy link
Member

fabpot commented Jul 26, 2022

Thank you @alamirault.

@fabpot fabpot merged commit 3633475 into symfony:6.2 Jul 26, 2022
wouterj added a commit to symfony/symfony-docs that referenced this pull request Oct 23, 2022
@wouterj
feature #16701 [Notifier] Add from in SmsMessage and support it in br…
91edb50 
…idge transports (alamirault)

This PR was squashed before being merged into the 6.2 branch.

Discussion
----------

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

If PR [#45987](symfony/symfony#45987) looks good. I will change docs

Commits
-------

d627e1c [Notifier] Add from in SmsMessage and support it in bridge transports
@fabpot fabpot mentioned this pull request Oct 24, 2022
Merged
@leblanc-simon leblanc-simon mentioned this pull request Nov 20, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fabpot fabpot fabpot approved these changes

@jderusse jderusse Awaiting requested review from jderusse

@OskarStark OskarStark Awaiting requested review from OskarStark OskarStark is a code owner

Assignees
No one assigned
Labels
Feature Notifier Status: Reviewed
Projects
None yet
Milestone
6.2
Development

Successfully merging this pull request may close these issues.

[Notifier/SmsMessage] Add setFrom method
4 participants
@alamirault @fabpot @OskarStark @carsonbot