Push features

[alexblack]

Hi, there are some areas within push that I'd love to see some new features, and I'm interested in contributing.

Would you be open to accepting PRs in these areas:

1. Modifying parse push send to return an ID that can be used to find the push status in the parse dashboard
2. An api to check push status, that takes the above ID as input, and returns which installations the push targeted, and if available any delivery info
3. Make available summary push info on the Installation record:

• most recent push sent I'd
• most recent push sent timestamp
• most recent push sent status (sent, failed, received, opened)

[flovilmart]

return an ID that can be used

The _PushStatus objectId is return as X-Parse-Push-Status-ID response header

which installations were targeted

The full query is stored in _PushStatus

On modifying the installation objects,

I'd rather not introduce side effects on public facing objects, I understand you want that part some analytics.

As for the status of the push, we report a global status, because we don't force the adapters to return a status per push to a certain device. That seems complex to enforce.

[flovilmart]

On another note, even if he delivery through the push adapter is successful, it is not guaranteed that the notification will be delivered. We usually encourage tracking opening on the local device for that purpose.

[alexblack]

I should have started with my goals, sorry :) Here is what I'm looking for out of a push provider (Parse or otherwise):

When sending a push, being able to both query for, and receive notification (via code/api) of:

• which devices pushes were attempted delivery to
• for each device, status of the delivery: sent, failed, received, opened

If you are interested in having these features in Parse server, do you have suggestions on what form of implementation would be acceptable?

Objectives here are:

• know when pushes fail to send
• easily debug push failures and issues
• find devices that have uninstalled the app (or that need action taken to restore push functionality)

Perhaps similar to #2889

[alexblack]

The _PushStatus objectId is return as X-Parse-Push-Status-ID response header

That sounds fantastic. Last I checked (ages ago) this wasn't possible. Do you know if the JS SDK exposes this? It doesn't appear to: https://parseplatform.github.io/docs/js/guide/#sending-pushes

The full query is stored in _PushStatus

Ah I was looking for the installations sent to, not the query. Eg the results of the query at the time of the push.

[flovilmart]

installations sent to, not the query.

The best for that would be a relation, like queryResults that would point to the targeted installations. Do you want to tackle That one?

[alexblack]

I'm interested in adding the capabilities I described above: #2910 (comment)

Is that something you'd be interested in having in Parse Server?

[flovilmart]

Let's start with tracking through a relation the installations. Like I explained above, the main issue with tracking status per installation is that most likely to be an analytics thing, and we try not to store them in as it would make the DB explode very fast.

[alexblack]

Hmm. If you're interested in those features, then I'd propose we discuss (at a high level) what they might look like, and if after that it looks reasonable then I'd be happy to start on one of them.

If on the other hand these are not features you're interested in, then we'd look to use another push solution instead.

[flovilmart]

I didn't say I'm not interested in those features, What I'm saying, is get a knock at it, open a pull request and we'play discuss the implementation details with the base code in, instead of over engineer a specification.

[alexblack]

I think I understood you clearly. I'm just saying I'd like to hear if you are interested in those, and then if you are - spend a few minutes discussing what they'd look like, and then if that looks good I'd love to help out.

[flovilmart]

For:

• which devices pushes were attempted delivery to

Add a key to the _PushStatus Object, which is a relation to the _Installation objects that were intended to be delivered to.

• for each device, status of the delivery: sent, failed, received, opened

The push adapters don't guarantee that the list of status will be sent back per installations, some do some don't, the default push adapter should forward back that information AFAIK.
Look there: https://github.com/ParsePlatform/parse-server/blob/master/src/StatusHandler.js#L145

Note that I don't think it's a good idea to add a new set of reserved columns on the _Installation object to track the status of the last push, first because it's ephemeral, second because you can very well do it yourself. This is an analytics problem an event that should be stored outside of Parse.
Also APNS don't report the push failures synchronously but through a separate listener that is not implemented. GCM, should provide the response back.

spend a few minutes discussing what they'd look like, and then if that looks good I'd love to help out.

Again, you should go forward with what you think is best, then we'll discuss with the code at hand. That's how it should work. For the simple reason that many other people can contribute to the code review/discussions about the implementation. Anything that I can think is good can be challenged by another maintainer, that's why I encourage you to move forward with an initial implementation of the feature(s) you'd like to see.

[flovilmart]

Also, important to note that with medium sized audiences (1000 push to send in 1 query) you will start noticing a bump in memory/CPU usage, above 10k audiences, you may notice complete meltdown of the system.

[alexblack]

This is an analytics problem an event that should be stored outside of Parse.
Also APNS don't report the push failures synchronously but through a separate listener that is not implemented. GCM, should provide the response back.

It sounds like we think about this quite differently. I would like to send pushes, and use a push solution that delivers pushes, and tracks delivery, receipt, and opening. If you're not interested in having these capabilities added to Parse, then we'll likely switch to another push solution.

Again, you should go forward with what you think is best, then we'll discuss with the code at hand. That's how it should work. For the simple reason that many other people can contribute to the code review/discussions about the implementation. Anything that I can think is good can be challenged by another maintainer, that's why I encourage you to move forward with an initial implementation of the feature(s) you'd like to see.

I think its worth taking a small amount of time to discuss before building the entire feature.

[flovilmart]

If you're only using parse-server as a push solution, I believe you'll be disappointed as it has not been designed to compete with OneSignal, SNS, Urban airship and other providers of push as a service.

Once again, I believe I'm clear enough on my concerns.

You should probably explore the code and make a proposal around it, also understand all the constraints set by the adapter structure etc...

draft a proposal, then we'll discuss it in the open. I'm not against what you're proposing, but I terms of general philosophy I prefer small incremental pull requests than major refactors. And that for historical reasons (notably support for multiple apps)

[alexblack]

Here's what I was thinking:

1. store a list of installations the push was sent to, say _push_installation

Fields on _push_installation

• sent_dt
• sent_result
• opened_dt
• installation_id
• push_status_id

1. An API to query that data (or maybe none needed if its just a parse object)
2. A webhook to receive notifications of updates of status to the object, eg on_delivered, on_opened
3. You pointed out that some pushes are sent to many installations, and there might be performance implications to this. This could be mitigated perhaps by one of these methods:
4. enable or disable the above tracking per push, default to OFF
5. only support the above tracking for pushes with audiences < PUSH_TRACKING_LIMIT, say 100

[flovilmart]

Again, go ahead and make a pull request with the according tests.

[alexblack]

If you're only using parse-server as a push solution, I believe you'll be disappointed as it has not been designed to compete with OneSignal, SNS, Urban airship and other providers of push as a service.

Agreed, I'm just asking if you're interested in having Parse Server improve in this regard so that it could get compete (or get closer to competing) with those. If you're not, then thats fine, I'll move on.

draft a proposal, then we'll discuss it in the open. I'm not against what you're proposing, but I terms of general philosophy I prefer small incremental pull requests than major refactors. And that for historical reasons (notably support for multiple apps

I am trying to discuss it :) I am not and never have proposed a major refactor.

[alexblack]

Again, go ahead and make a pull request with the according tests.

Great, does that mean you're open to accepting PRs in this direction?

[flovilmart]

 Great, does that mean you're open to accepting PRs in this direction?

YES and I said that a few hours ago!

get compete (or get closer to competing) with those

The issue is not tracking but the time/space complexity of the problem of sending and reporting multiple push status.

go ahead with the PR, we'll discuss the implementation details once it's opened.

[alexblack]

Great. I have been interested in discussing it, just to minimize chance I write some code and then find out you don't want it.

1. Do you like the idea I proposed above of a webhook to report updates on push status updates?
2. One of the ideas above is to do push open tracking too, this would require a call back from the app to the server with an ID representing the push. I think today there is push open tracking, but it just tracks analytics, it doesn't track which push was opened (if I understand correctly). Are you open to this idea?

[flovilmart]

There is one thing that seems clear, is that all you're proposing could be implemented in a custom adapter with analytics points...

Have a look at the code here (which is the default push adapter)

https://github.com/parse-server-modules/parse-server-push-adapter

Again, go ahead with you implementation. You'll realize if it makes sense or not, if you have a better solution, then you'll have something different to propose.

I am not for implementing a full analytics solution for the push opens as we forward all analytics to a custom analytics adapter of your choice. You may want to keep that in mind.

Again, I said that I'm open for your feature, the choice of implementation are your own, and without seing the code changes, it's likely to be impossible to wrap my head around it.

See that PR for info #263

[alexblack]

I've given this a bit more thought, here is what I'd propose for a first PR:

1. New public object _Push, fields:

• object_id
• installation_id
• sent_dt
• sent_result
• received_dt
• _PushStatus object_id

2. New flag on Parse.Push.Send, track, boolean, default: false

3. When track is true, a _Push object will be created for each Installation a push is sent to

• sent_dt will be populated
• sent_result will be populated if available

4. A new REST api method: trackPushReceived(_Push.object_id)

• called when a push is received by a client device
• the field received_dt is then populated on the appropriate _Push object

[flovilmart]

You should use camelCase for your field names

[flovilmart]

again, go ahead, and we'll discuss it once your PR is open. This is likely to change when you'll be implementing the feature and the tests.

[alexblack]

hey @flovilmart great, camelCase sounds good. Could you please confirm you like the general idea before I invest the time? Thx.

Eg, that you are ok with new public object _Push, and new rest api method to track pushes being received.

(Definitely things could change, but no point starting if the intended direction is not something you agree with)

[flovilmart]

I'm not sure that object should be public, I'm curious how you will be able to plug that in. Again, I prefer looking at the PR, and base my decision upon that. Maybe you don't need a _Push object but can track it all on the _PushStatus object.

It is difficult to say yay or nay without seeing the implementation.

Also note that it's not because I would say yes that make sense now, that the Pr would automatically be accepted.
Merging a PR means that we take the responsibility to maintain your piece of code. So consider that also when building it.

[alexblack]

Also note that it's not because I would say yes that make sense now, that the Pr would automatically be accepted.
Merging a PR means that we take the responsibility to maintain your piece of code. So consider that also when building it.

Of course. I'm not looking for a greenlight, just making sure I'm going in the right direction.

I'm not sure that object should be public, I'm curious how you will be able to plug that in. Again, I prefer looking at the PR, and base my decision upon that. Maybe you don't need a _Push object but can track it all on the _PushStatus object.

This is why I wanted to discuss. If I put some time in, and add a new public object, its then too late to receive feedback that you don't think the object should be public.

Having a public object to represent each push sent makes a lot of sense to me.

1. Clients can then query the _Push object table to see which installations their push was sent to, and for each installation can check when push was sent, what the result was, and if/when received
2. Implementing the API to track push receives is straightforward, merely find the _Push object and update it.

If you object to that, how would you propose #1 and #2 be done with the PushStatus object?

[flovilmart]

What I'm saying is that it doesn't matter, that since the beginning of this thread I suggested you knock a simple PR out of it and then that we try to figure out he implementation details. Public or not public is a matter of putting an ACL on the object, and discussing it is irrelevant at that time.

I also know by experience that either you spend time writing a full and detailed specification, including the points of code that need to be overriding, the effect on the codebase complexity etc... or you just get started with the most minimal implementation. I suggest the latter.

You should also notice that some users are already complaining about the number of _PushStatus objects generated. Also you should consider the involvement of the dashboard.

I don't believe your implementation should be opinionated at the point where changing the way we store the status imply a complete overhaul of the feature.

Again, all those things that I am mentioning can only be raised when reviewing the code.

If it scares you to spend time refactoring your PR because the implementation you propose has architectural issues, then you should probably implement it part of an adapter. (Which is 100% possible too).

Maybe thé implementation can be split between the core codebase and a push adapter.

[alexblack]

It seems like a pretty straightforward suggestion to store results in a public object _Push, and have an API update that.

If you're not onboard with the idea, then it doesn't make sense to start a PR. I'm open to other suggestions for the PR.

The purpose for making _Push public are:

1. Enable clients to query the push status with no new APIs or changes, simply use existing parse methods like query for objects
2. Enable users to check push status in the dashboard easily with no UI changes, simply use existing Dashboard features to search the object _Push

[alexblack]

You should also notice that some users are already complaining about the number of _PushStatus objects generated. Also you should consider the involvement of the dashboard.

Thanks for making me aware of that. Yes this would generate many objects, by default track is off so its optional to use.

Since its a public object _Push, it just needs to show up in the dashboard like any other object eg _Installation

[flovilmart]

SO after giving it more thoughts, I believe the _Push object should be in the adapter and not the core server.

The reason is that only the default push adapter is reporting the status / push. All other adapters don't do that. It doesn't make sense to have this feature as part of parse-server. This feature will be better off into the parse-server-push-adapter (here: https://github.com/parse-server-modules/parse-server-push-adapter)

You'll also have more insights on what's going on into that adapter than on the push server itself.

It can also be implemented as a separate npm module that would do push tracking at adapter level. The user could also choose the target object class etc...

Again, all those things don't require the implementation to be on parse-server.

What would be required on parse-server is just to make sure the _PushStatus id is properly sent to the adapter, and that is by the way already done.

So to summarize, for what you want to achieve, there is no need to put it in parse-server itself, and can readily be implemented with a custom push adapter (or made as a PR on parse-server-push-adapter)

[flovilmart]

Since its a public object _Push

You'll notice that working with _ prefixed classes is problematic, so there is no need to prefix is. just the the default CLP only allow master update.

As for tracking the push open, that can be done easily with a cloud function, an express endpoint etc.. The implementation could provide a vanilla callback like:

implementation.trackPushOpen(req.params.pushObjectId);

[alexblack]

Tracking the opening is independent of the PushAdapter, and would work for any pushes sent through any datapter, so its implementation doesn't belong in a push adapter.

I'm not seeing how the api to track the opened pushes would record the status. Are you saying it would put the result in PushStatus (rather than a Push object as I suggested)

[flovilmart]

Tracking the opening is independent of the PushAdapter

YES definitely, we call that analytics, and it's already taken care of by the analytics adapter, and this is an independent issue. from 90% of what you want to implement.

If you're using OneSignal or UrbanAirship to send the pushes, there is a big chance you want to track the conversion rates on those platforms, not on parse-server.

And risking to repeat myself, this implementation don't belong to parse-server, but to the push adapter, wether you want to do a PR on the repo I mentioned earlier or implement your own adapter is up to you at that point.

[alexblack]

From what I can tell the analytics does not let you query it to find out which installations the push was sent to, and if/when each was opened.

I'm proposing we add a Push object for this, and an API method. If you don't like this proposal, I'm open to other suggestions.

[flovilmart]

Man this is complicated:

1. Add relation on _PushStatus to put he list of all targeted _Installations
2. In the push adapter, do whatever tracking you want, in whatever object you want
3. In the _PushStatus, add a relation to _Installation for opened pushs

Is that clear and good enough?

[alexblack]

1 and 3 sound ok, 2 I don't want to do anything there right away.

My concern with your proposal to store this data in PushStatus instead of a new public object Push is:

1. How does someone query for the list of installations and their open dates using the API? This might be trivial, I am not too familiar with the PushStatus object, as I have only seen it in the dashboard. Can it be queried like any other parse object?
2. This sounds like it requires UI work in the Dashboard to get the new data to show up in the PushStatus. Personally I'd prefer to avoid that and just put the results in a regular object that shows up in the dashboard.
3. The Pushes sent UI in the dashboard doesn't support sorting or filtering.

Also, the intention would be for the Android and iOS SDKS to eventually call the API method to track that the push had been opened.

Queries that should be supported by API:

• Retrieve list of installations by pushstatus, including open dates
• Retrieve open date for an installation, pushstatus pair

UI functionality:

• View pushes recently sent, with fields: pushStatus, installation, sentDt, adapter etc
• View pushes to installations that have not been opened
• View pushes sent for a given PushStatus

[flovilmart]

Man, I really don't know why you're asking for feedback if you're settled on you own solution, or not considering what I'm even saying. I believe I expressed enough of my concerns and directions regarding that potential feature.

Like I said this morning, I'm waiting to see an implementation before giving anymore thoughts to it. Ive clearly expressed how I would go about building that feature with the minimal impact on the existing codebase.

I truly believe this can be implemented externally, inside the adapter and still achieve everything you want to achieve. And I believe it will be easier to integrate and build externally too, as mentioned, only the parse-server-push-adapter reports it's delivery status.

[alexblack]

I'm really finding this frustrating. I proposed what I thought was a simple elegant solution: adding a push object which gets nice UI in the dashboard.

You counter proposed using PushStatus, so I asked some questions about how it'd work.

At this point I think I'm kinda down on this whole thing :(

[flovilmart]

And I suggested you add this object, but that object would be created and updated FROM the adapter, not from parse-server.

Still i believe the audience and opens should be a relation on the _PushStatus object and not separate objects. The relations are fully browsable from the dashboard, without entering a query/filters, which is actually better.

Then, if you don't feel implementing this feature anymore, i can't force you into it.

[alexblack]

We'd discussed that this doesn't belong in adapter since open tracking would work for all adapters, did I misunderstand?

[alexblack]

he relations are fully browsable from the dashboard, without entering a query/filters, which is actually better.

I'm not sure I understand, can you elaborate? Background: from what I can see today PushStatus cannot be filtered or sorted in the Dashboard, but other objects like Installation can.

I proposed it'd make sense for pushes to be sorted and filtered too, same as any other parse object.

[flovilmart]

Man... That's painful. You seem to have an answer to everything so.

GO ahead, implement it as you wish, and we'll discuss the implementation details when your PR lands.

I believe I said that 5 times today already, and that by that time, i should be reviewing your first implementation instead of trying explain myself again.

Everything that I needed to say has been said, my patience is running thin and this thread has turned into a debate of opinions more than anything meaningful.

You probably wasted your time here as well, so I encourage you, for the last time to start building something instead of throwing ideas in the air.

I proposed what I thought was a simple elegant solution: adding a push object which gets nice UI in the dashboard.

You should go ahead with the simple and elegant, because obviously I have no clue what I'm talking about nor havent put any LOC in neither the PushController, push adapters.

[alexblack]

Hey, I seem to be really annoying you, thats not my intention. I'm sorry.

You should go ahead with the simple and elegant, because obviously I have no clue what I'm talking about nor havent put any LOC in neither the PushController, push adapters.

My whole purpose for opening this issue was to get your input on this topic. You have suggested that using PushStatus instead of a new object is a better way to go, so I was trying to ask some questions to make sure it would achieve the objectives I'd set out earlier in this discussion.

I didn't mean to claim my solution was simple and elegant, it seems that way to me, as an outsider to the codebase.

[alexblack]

I submitted a draft PR, its 100% incomplete, but, it might be something concrete to discuss, or reject.

#2917

[alexblack]

Push status id - oh wow, that's great. Is that exposed by the JS sdk?

That sounds like a no overall?

On Oct 22, 2016 7:40 PM, "Florent Vilmart" [email protected] wrote:

return an ID that can be used

The _PushStatus objectId is return as X-Parse-Push-Status-ID response
header

which installations were targeted

The full query is stored in _PushStatus

On modifying the installation objects,

I'd rather not introduce side effects on public facing objects, I
understand you want that part some analytics.

As for the status of the push, we report a global status, because we don't
force the adapters to return a status per push to a certain device. That
seems complex to enforce.

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#2910 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAJFwjwKQRGfqWtSEULuYzyAB9cSI4JZks5q2skAgaJpZM4KeBxu
.