AndroidNotification

[kiasaty]

Hi Jérôme Gamez,

Firstly, thanks for creating this awesome package.

There is a specific kind of notification in firebase named AndroidNotification:
https://firebase.google.com/docs/reference/fcm/rest/v1/projects.messages#androidnotification

I checked the package and figured out that it doesn't have such a notification class.
So I decided to extend the main Notification class and make a PR.
But when I checked the Notifications class, I noticed that it is "final".
What is the reason behind making this class final?

Regards

[jeromegamez]

You can add platform-specific configuration (Android, iOS, Web) with

use Kreait\Firebase\Messaging\AndroidConfig;

// Example from https://firebase.google.com/docs/cloud-messaging/admin/send-messages#android_specific_fields
$config = AndroidConfig::fromArray([
    'ttl' => '3600s',
    'priority' => 'normal',
    'notification' => [
        'title' => '$GOOG up 1.43% on the day',
        'body' => '$GOOG gained 11.80 points to close at 835.67, up 1.43% on the day.',
        'icon' => 'stock_ticker_update',
        'color' => '#f45342',
        'sound' => 'default',
    ],
]);

$message = $message->withAndroidConfig($config);

or directly with

$message = CloudMessage::fromArray([
    'notification' => [
        'title' => 'Generic title',
        'body' => 'Generic body',
    ],
    'android' => [
        'ttl' => '3600s',
        'priority' => 'normal',
        'notification' => [
            'title' => 'Android-specific title',
            'body' => 'Android-specific body',
        ],
    ],
]);

https://firebase-php.readthedocs.io/en/5.14.1/cloud-messaging.html#adding-target-platform-specific-configuration

With both ways, you can add any configuration that is supported by Firebase (current and future fields). (In fact, I would say today that using an array instead of the builder classes should be the recommended way, and I'm probably going to make this the recommended default and remove the other ways in the next major release, but that's just a side note)

Concerning the decision to make most of the classes final, others have explained the principle behind it better than me, for example at https://matthiasnoback.nl/2018/09/final-classes-by-default-why/ and https://ocramius.github.io/blog/when-to-declare-classes-final/. I'm usually taking it one step further an often make classes final that are not value ojects or implement an interface: in the past, people extended the classes from my libraries in their own projects and filed bug-reports when I changed the internals of the parent classes. Making everything final "protects me" from that 😅.

I know this is a controversial topic and many wars have been fought about it on Twitter and Reddit, but here, this is how I roll 😅.

I hope this helps, sorry if it was too much bla bla!

PS: You don't need to notify me in an additional email that you asked a question, I get notified about anything that happens in my open-source projects and I usually react in a timely manner - 😅

[kiasaty]

You can add platform-specific configuration (Android, iOS, Web)

Thanks a looot.

in the past, people extended the classes from my libraries in their own projects and filed bug-reports when I changed the internals of the parent classes. Making everything final "protects me" from that .

So it makes life easier for maintainers and makes life harder for developers. :)) it's a challenging decision.
But there might be a win-win solution in some scenarios using interfaces and composition.

this is how I roll

fair enough :))

I hope this helps, sorry if it was too much bla bla!

It was great! Thank you for taking the time and explaining it in this level of detail. I really appreciate it.

PS: You don't need to notify me in an additional email that you asked a question, I get notified about anything that happens in my open-source projects and I usually react in a timely manner

Sorry for that :)
Actually, my colleague told me that I'm not going to get an answer soon. because he had submitted an issue and get a response late.
So I decided to send you an email to have a higher chance to get an answer soon.
But you answered super fast. Thanks a lot :)

[jeromegamez]

I just remembered: CloudMessage is "just" an implementation of the Message interface (which the Messaging component accepts). RawMessageFromArray is another one, so you should be able to roll your own implementation, if you want to do it differently.

As for the Notification class, some thoughts from the top of my head - I wouldn't extend the Android notification from it but create a different, "self sufficient" class for that. It's true that all Notifications (general, iOS, Android, web) share a title and body field, but they are used in different contexts and could have different requirements. If a Web notification would allow HTML, and an Android notification perhaps some control chars for coloring (just fantasizing here), we would have to override the methods anyway, obsoleting the benefits of inheritance.

In the end the whole Message and its components are just a value object and the only requirement is that it can be converted to an array/JSON to send it to Firebase.

When I started the component, my vision was to protect developers from setting wrong fields to avoid being rejected by Firebase, but when I realized that there are so many different fields and requirements that I probably shouldn't do this it was too late: I had already released a version and needed to keep it to not break backward compatibility 😅. That's why the Notification class exists and is pretty strict, but the AndroidConfig class accepts pretty much anything 🙈