Implement Realm.User.push() #2995
Conversation
| @@ -6,6 +6,7 @@ NOTE: This version bumps the Realm file format to version 11. It is not possible | |||
|
|
|||
| ### Enhancements | |||
| * Added RemoteMongoClient functionality to `Realm.User` | |||
| * Added Push functionality to `Realm.User` | |||
There was a problem hiding this comment.
Perhaps we can elaborate a bit more what this is (in end user terms). (I can see we also missed to do that with RemoteMongoClient.
There was a problem hiding this comment.
Unfortunately, I don't actually know what "Push" does in end user terms. This sounds like a good thing to have our Docs team work on so that they can ensure that release notes use consistent terminology with the rest of the documentation.
| } | ||
|
|
||
| template<typename T> | ||
| void UserClass<T>::push_deregister(ContextType ctx, ObjectType this_object, Arguments& args, ReturnValue &return_value) { |
There was a problem hiding this comment.
What's the difference to the "register" function above? They look identical.
There was a problem hiding this comment.
😬 Yikes! That would be a bug. It is supposed to be calling deregister_device() on like 343.
There was a problem hiding this comment.
So seems like the tests needs an update as well then ;-)
Isn't it possible to reuse some of that code so it's clear where there is supposed to be a difference :-)
There was a problem hiding this comment.
The API for deregisterDevice changed upsteam in object-store, so I'll add an update to the submodule to this PR so we use the new API realm/realm-object-store@1f3290a#diff-b215f5f5ce1cec59d075f06fbab4510e
Isn't it possible to reuse some of that code so it's clear where there is supposed to be a difference
Probably. I have some ideas on refactors that can clean these repetitive methods up a bit, but so far I've only done the promisify() part on the JS side. I'll probably make a separate PR to clean up the CPP side at some point.
|
I don't have a good suggestion on how to resolve this, but since Realm Web does not currently have an implementation of the push service we would have an issue if Realm Web gets released with these types. In principle we should strive to have feature parity between platforms, but I remember having a discussion with @jsflax about the (lack of) need for this service in Realm Web (since it runs in the more ephemeral browser, than an app installed on the users phone):
|
|
|
||
| auto credentials = realm::app::AppCredentials::function(payload_json); | ||
| auto credentials = realm::app::AppCredentials::function(payload_bson.operator const bson::BsonDocument&()); |
There was a problem hiding this comment.
Upstream object-store changed the parameter type.
| promisify(func) { | ||
| return new Promise((resolve, reject) => { | ||
| func((...cbargs) => { | ||
| if (cbargs.length < 1 || cbargs.length > 2) |
There was a problem hiding this comment.
So only 1 or 2 arguments? Maybe [1, 2].includes(cbargs.length)?
| * return promisify(cb => this._floop(how, why, cb)); | ||
| * } | ||
| */ | ||
| promisify(func) { |
| @@ -216,7 +216,7 @@ struct JavaScriptNetworkTransport : public JavaScriptNetworkTransportWrapper<T> | |||
| case app::HttpMethod::get: return "GET"; | |||
| case app::HttpMethod::put: return "PUT"; | |||
| case app::HttpMethod::post: return "POST"; | |||
| case app::HttpMethod::del: return "DEL"; | |||
| case app::HttpMethod::del: return "DELETE"; | |||
| @@ -371,6 +371,10 @@ declare namespace Realm { | |||
| linkCredentials(credentials: Credentials): Promise<void>; | |||
| callFunction(name: string, args: any[]): Promise<any>; | |||
| refreshCustomData(): Promise<Object>; | |||
| push(serviceName: string): { | |||
There was a problem hiding this comment.
@RedBeard0531 Going over this code when merging it with my changes (kh/v10-merged-types) I realize that we might want to have this serviceName be optional and provide a default name.
What, How & Why?
This closes # ???
☑️ ToDos
[ ] 📝Compatibilitylabel is updated or copied from previous entry[ ] 📝 Public documentation PR created or is not necessary[ ] 💥Breakinglabel has been applied or is not necessaryIf this PR adds or changes public API's: