-
Notifications
You must be signed in to change notification settings - Fork 38
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
[Feature request] please add support for admin.messaging #50
Comments
I second this feature request.. I've got my interop all built out in dart, and everything works up until I need to push a notification with FCM or PubSub, and that was the whole purpose for me. Without this ability, I need to abandon this package and rebuild the whole thing with the js version, which I really don't want. If I can help, I would since this is one of the last feature I'm missing before I can publish my app as usable. |
Of course, you're more than welcome to submit a PR for adding messaging support. It shouldn't be that hard to implement. Just need to follow similar patterns used in other modules (firestore, auth). In general, implementation consists of two pieces:
One thing that I'd mention here to not forget to copy-paste official docs for all methods and classes into dartdocs in the code. Here is how Auth service interface looks like in /// The Firebase Auth service interface.
@JS()
@anonymous
abstract class Auth {
/// The app associated with this Auth service instance.
external App get app;
/// Creates a new Firebase custom token (JWT) that can be sent back to a client
/// device to use to sign in with the client SDKs' signInWithCustomToken()
/// methods.
///
/// Returns a promise fulfilled with a custom token string for the provided uid
/// and payload.
external Promise createCustomToken(String uid, developerClaims);
/// Creates a new user.
///
/// Returns a promise fulfilled with [UserRecord] corresponding to the newly
/// created user.
external Promise createUser(CreateUserRequest properties);
/// Deletes an existing user.
///
/// Returns a promise containing `void`.
external Promise deleteUser(String uid);
// .. continues
}
Here is again Auth class which is exposed by this library: class Auth {
Auth(this.nativeInstance);
@protected
final js.Auth nativeInstance;
/// Creates a new Firebase custom token (JWT) that can be sent back to a client
/// device to use to sign in with the client SDKs' signInWithCustomToken()
/// methods.
///
/// Returns a [Future] containing a custom token string for the provided [uid]
/// and payload.
Future<String> createCustomToken(String uid,
[Map<String, String> developerClaims]) =>
promiseToFuture(
nativeInstance.createCustomToken(uid, jsify(developerClaims)));
/// Creates a new user.
Future<UserRecord> createUser(CreateUserRequest properties) =>
promiseToFuture(nativeInstance.createUser(properties));
// continues...
} Notice usage of Please ask any other questions if you need more help on understanding how this works. |
Alright, that was a fun learning experience.. Worked out what I could and think I got it fairly accurate but not tested and not sure about certain bits, so you're gonna have to verify. I wasn't sure which properties needed jsify() since I only saw it used for Map/Object and that wasn't in the methods. Also didn't know how to handle the parameters that took String | String[] to allow for an array. And not sure if I made the Message class correctly because it's listed as a type alias for TokenMessage | TopicMessage | ConditionMessage but is still used and not documented clearly.. Here's what I added into bindings.dart:
On top, in class FirebaseAdmin, added this:
In abstract class App, added this:
Now here's what I got for the new messaging.dart file:
Then finally in firebase_admin_interop.dart added this: Did the best I could to keep consistent with the pattern and adding the docs, should save you a lot of time getting it up and running for the next version of admin-interop, looking forward to putting it to use. Thanks... |
I tried to test it locally in my app, and discovered the class Message is already used in firebase_functions_interop so we're getting a conflict.. That's a tricky one to deal with (for me) since it's needed for send and sendAll functions. Should it be renamed to FcmMessage or something? I assume it can still be passed to the JS with a different name, but haven't tried yet.
That's better.. I was also thinking some of those messaging classes could use a factory constructor like you have on some others, just wasn't sure which ones needed it. The more I played with it, I realized that all the exposed classes needed the external factory constructor or they are abstracts that can't be used. Still working on it, but here's what I changed so far in bindings.dart:
I think I've got to do more of them, but that was enough for me to test further. I'm also thinking that the DataMessagePayload class is supposed to contain a Map<String, String> but the docs didn't tell me what to name it. Progress though. |
@Skquark how's it going? :) |
Well, I took what I got so far and put it in my Fork.. Check it out here https://github.com/Skquark/firebase-admin-interop
If you can test it out and see if it miraculously works the way it is, that'd be good. Was still questioning whether any of the functions needed jsify, but I believe everything else is done right. Once confirmed, will make a Pull Request with the update. |
@Skquark, I did a quick test and your branch is working for me. I was able to send a plain (title, sub-title) push notification. Thanks for building this out. |
@pulyaevskiy I know this repo hasn't been a priority, but any chance of merging in @Skquark's messaging work? Looks like he hustled pretty hard to implement, and these interop packages are integral to my project as well given some architectural constraints (firestore triggers + dart models). Also the 'Remove links and shortcuts and try again' error he mentions seems related to the symbolic link in build/node. Not familiar enough with the codegen to know if that can be fixed, but I have to delete that symlink to upload to Firestore. Many thanks for the work to both of you! :) |
@pulyaevskiy Any updates on this? |
I guess an update has been long overdue since it's a pretty practical feature I can imaging a lot of people's projects would need. Just officially submitted a PR to make it easier, I think it's been tested well enough and all checks out. I also took the liberty of updating the other dependancy versions in pubspec that I found pending in someone else's PR, hope you don't mind. You'll need to update you're firebase_functions_interop pubspec versions as well to roll out the update.. NotificationMessagePayload notification = NotificationMessagePayload(
title: title,
body: body,
clickAction: "FLUTTER_NOTIFICATION_CLICK",
);
MessagingPayload payload = new MessagingPayload(notification: notification, data: DataMessagePayload(data: {"doc" : event.reference.path}));
MessagingDevicesResponse result = await firestoreApp.messaging().sendToDevice(token, payload);
// or firestoreApp.messaging().sendToTopic(topic, payload); Looks good, I got mine functional within regularly scheduled cron jobs to send messages appropriately, felt nice getting that working the way I needed. Cheers. |
This was released in 2.1.0. Thanks @Skquark for the implementation!
Which versions exactly need to be updated? functions already depend on admin |
@Skquark, the example you gave doesn't work for me. Right now, DataMessagePayload({String key, dynamic value}) Which only allows a single value. The docs clearly make it sound like this was supposed to be a Thanks for all your work though! This is great timing for me, since I just finished using the Cloud FIrestore side of this library and am almost done porting my Python implementation to Dart. It's really nice to be able to keep all my project files in the same language. |
You're right, sorry, that was an object I was a bit confused on the implementation because of the way it was documented in Google's api doc at https://firebase.google.com/docs/reference/admin/node/admin.messaging.DataMessagePayload abstract class MessagingPayload {
/// The data message payload.
external List<DataMessagePayload> get data;
/// The notification message payload.
external NotificationMessagePayload get notification;
external factory MessagingPayload({
List<DataMessagePayload> data,
NotificationMessagePayload notification,
});
}
/// Interface representing an FCM legacy API data message payload. Data messages let developers send up to 4KB of custom key-value pairs. The keys and values must both be strings.
@JS()
@anonymous
abstract class DataMessagePayload {
/// Keys can be any custom string, except for the following reserved strings:
/// "from" and anything starting with "google."
external String get key;
external String get value;
external factory DataMessagePayload({
String key,
String value,
});
} That might be the wrong way though, I saw a Typescript implementation that looks like this: export interface DataMessagePayload {
[key: string]: string;
} But that syntax looks weird to me, don't know honestly, but it gives a clue. Hopefully you or someone else here has a better grasp than I. Regarding the other dependency updates I mentioned, I was referring to the functions_interop pubspec since that is our entry point to use admin_interop. I made a quick fork of it to update the versions in yaml myself, plus added example sendMessage.. Got it here: https://github.com/Skquark/firebase-functions-interop |
I just helped fix another typed Map issue in this repo -- check out From the Firebase docs for the Node.js admin SDK:
So it definitely sounds like we're talking about a |
I know things are crazy right now given current events, hope you guys are okay. Any updates? If this needs to take a backseat for a while I don't mind picking it up, or helping in any way I can. Stay safe everyone. |
@Levi-Lesches sorry, not sure which issue are referring to? Messaging was added in 2.1. |
3 similar comments
@Levi-Lesches sorry, not sure which issue are referring to? Messaging was added in 2.1. |
@Levi-Lesches sorry, not sure which issue are referring to? Messaging was added in 2.1. |
@Levi-Lesches sorry, not sure which issue are referring to? Messaging was added in 2.1. |
Right now, the
Which means that
I think the most straightforward solution is to replace EDIT: Additionally, Thanks again @Skquark for all your work, it takes care of a LOT that can be very difficult to implement. |
No description provided.
The text was updated successfully, but these errors were encountered: