[Notifier] Add notification property to SmsMessage class #37818
Conversation
| return $message; | ||
| } | ||
|
|
||
| public function getNotification(): ?Notification |
There was a problem hiding this comment.
As I said in the other PR, I don't like that you can get a Notification from the SMS message, it does not feel right. Instead, if many providers have a "quality" setting, we might add a quality property on the SMS message. Can you see if "big" providers like Nexmo or Twilio also have such a setting?
There was a problem hiding this comment.
I know, I put thechangehere to open the discussion.
It appears that nothing equivalent is present in thoses provider.
They offer an sms pricing without paid options. Or I couldn't find it.
There was a problem hiding this comment.
That's what I feared unfortunately. Using options on the message is still possible though. So, I think it could be enough for people really wanting to use this feature.
There was a problem hiding this comment.
ok thanks for yout feedback.
I might need to clean the mobyt bridge to delete code that won't be used (e.g.: fromNotification in MobytOptions)
This PR is a follow up from the discussion there: #36648 (comment)
The idea is to allow SmsTransports to get the notification level in order to use or set some advanced parameters in the transport.
Work case: MobytTransport, added in PR #36648, has a parameter to choose between 3 levels of delivery quality(and costs).
Depending on the notification importance we should be able to set the parameter.
I'm open to any suggestion to make it as generic as possible.