Skip to content
This repository has been archived by the owner on Sep 20, 2023. It is now read-only.

Mobile notifications #2105

Merged
merged 93 commits into from Apr 30, 2019
Merged

Mobile notifications #2105

merged 93 commits into from Apr 30, 2019

Conversation

tomlinton
Copy link
Collaborator

@tomlinton tomlinton commented Apr 23, 2019

This is a draft of the mobile push integration with the new notifications server. Check it out @wanderingstan and we can sync up.

  • Adds an endpoint on the notifications server for saving device tokens
  • Basic push logic (tested and functions on Android)

Things I have to do:

  • Handle wallet deletes
  • Add more tests for token save endpoint to notifications server
  • Test on iOS
  • Figure out how to handle URLs in notifications (open in WebView rather than browser?)

Closes #2049 and closes #1879.

@micahalcorn
Copy link
Member

Deep linking to webview would be very helpful. 🙏

@tomlinton tomlinton marked this pull request as ready for review April 29, 2019 10:46
@tomlinton
Copy link
Collaborator Author

This is ready to go, some other changes:

  • Simplified integration with @origin/graphql
  • Add notifications server to @origin/graphql config
  • Created a separate component for handling push notifications
  • Made linking to pages in the WebView work. Doesn't work in development builds on iOS for the case where the app is closed and clicking the push notification opens the app. I think this might be because of the bundler and hopefully it works in test builds.

@micahalcorn
Copy link
Member

@wanderingstan did you want to take a look at this? If not, I'll probably just stamp it. 🤷‍♂

Copy link
Collaborator

@wanderingstan wanderingstan left a comment

Choose a reason for hiding this comment

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

It all LGTM, but I'm not set up to test it. @tomlinton can we schedule some time tomorrow? I'd also like to get up to speed for integrating user-to-user messaging. (Though I think I see how we'd do it)

@tomlinton
Copy link
Collaborator Author

Sure, whatever suits. Can do anything after 1pm your time.

This is a real hassle to test locally, you need to have a working docker-compose stack and an iOS/Android build running on a physical device (simulators don't support push notifications) and figure out how to connect to a DApp running on your machine over wifi. And you also need proper development APNS/FCM credentials. 😞

I think if we just push it to dev and use TestFlight builds to pick up any bugs I didn't find in my own testing it will be the easiest.

@tomlinton tomlinton merged commit adaea19 into master Apr 30, 2019
@tomlinton tomlinton deleted the tomlinton/mobile-notifications branch April 30, 2019 06:05
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add endpoints to notifications server for saving device tokens Log Wallet Creation
3 participants