Skip to content
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

Proposal for tracking Pushes #2917

Closed
wants to merge 7 commits into from

Conversation

alexblack
Copy link

@alexblack alexblack commented Oct 24, 2016

This PR is totally incomplete. Its an early proposal

Goal: add new push tracking capabilities to parse-server, specifically: track pushes and their status at the device level.

Objectives:

  1. Enable queries through the existing Parse API
  • Which installations was a given PushStatus sent to
  • What was the result of the push sending for each installation (support may vary by adapter and route)
  • Find failed Pushes
  • Find installations with failed pushes
  1. Enable visibility in the Dashboard
  • See recent Push objects
  • Filter Push objects to ones that failed
  • Filter Push objects by PushStatus
  • Filter Push objects by installation
  1. Work with all push adapters and destinations

Proposed implementation:

  • New public object Push
  • A Push object is created per installation before sending to the PushAdapter
  • After PushAdapter returns, any results can be updated on the Push object

Possible future extensions:

  • Add a Rest API method to track push opens, will update openedDt on the Push object
  • Push adapters can add more extensive tracking, eg hook into GCM delivery reports and the Push object can be updated

@facebook-github-bot
Copy link

@alexblack updated the pull request - view changes

@flovilmart
Copy link
Contributor

The code is not building.
The two calls you just added should be in the StatusHandler module, not the push controller.

pushStatus.setRunning(response.results);
return this.sendToAdapter(body, response.results, pushStatus, config);
}).then((results) => {
return pushStatus.complete(results);
return pushStatus.complete(results).then(() => {
return updateTracking(response.results, results, pushStatus);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Response don't exist here...

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. It'd need to be refactored.

@alexblack
Copy link
Author

alexblack commented Oct 24, 2016

The code is not building.

Correct. It also doesn't do anything yet.

The two calls you just added should be in the StatusHandler module, not the push controller.

Could you use this as an opportunity for me to learn a bit about parse-server? I'm not familiar with the push controller or StatusHandler, or how to know why the code belongs in once place or another.

Could you point out where these two calls might go then? That'd help me a lot, thx.

@flovilmart
Copy link
Contributor

Like I said, those two call should go into that module: https://github.com/ParsePlatform/parse-server/blob/master/src/StatusHandler.js

Inside the setRunning and complete methods

@facebook-github-bot
Copy link

@alexblack updated the pull request - view changes

@facebook-github-bot
Copy link

@alexblack updated the pull request - view changes

@alexblack
Copy link
Author

alexblack commented Oct 27, 2016

Pushed some changes. Still incomplete, not anywhere close to being ready to be considered for merging.

I'm very new to everything here - the codebase, the dependencies, the style, etc. So I imagine I'm doing everything wrong :)

The code runs. Push objects get inserted. I had to add fields installation and pushStatus to the class Push manually.

screenshot 2016-10-26 22 56 59

@facebook-github-bot
Copy link

@alexblack updated the pull request - view changes

@facebook-github-bot
Copy link

@alexblack updated the pull request - view changes

@facebook-github-bot
Copy link

@alexblack updated the pull request - view changes

@flovilmart
Copy link
Contributor

As you mentioned is nowhere to be close to merge. I'm closing the PR. Reopen when ready.

Also, make sure the test pass before reopening.

@flovilmart flovilmart closed this Oct 27, 2016
@alexblack
Copy link
Author

Ok sounds good. Do you have any feedback on the direction or code so far?

On Oct 27, 2016 4:56 AM, "Florent Vilmart" [email protected] wrote:

As you mentioned is nowhere to be close to merge. I'm closing the PR.
Reopen when ready.

Also, make sure the test pass before reopening.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#2917 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAJFwnLUca5gknMBmOZtYZAF-2z5We1Mks5q4JFwgaJpZM4KeUJk
.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants