-
Notifications
You must be signed in to change notification settings - Fork 39
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
Send push notifications to users nearby unrated locations #688
base: master
Are you sure you want to change the base?
Conversation
…of an unrated location. New notifications will replace old ones to avoid spam. Mobile users will get a vibration and noise when receiving it
app/public/service-worker.js
Outdated
}) | ||
|
||
self.addEventListener('fetch',function(e) { | ||
//console.log('fetching '+e); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove this code that doesn't do anything. The fetch here doesn't do anything. I guess you used it for troubleshooting but it doesn't need to get deployed.
@@ -0,0 +1,50 @@ | |||
var url= window.location.href; | |||
var pos = url.search('using_pwa'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move this check to the server. HomeController.php has a method that would be called for route "/". Store the true/false value of whether the request was using_pwa into the server's session so it is available as the user navigates around the site.
This value on the server can be used to prevent including these service worker scripts when not in a PWA.
default_location = new google.maps.LatLng(default_location.latitude, default_location.longitude); | ||
|
||
try{ | ||
default_location = new google.maps.LatLng(default_location.latitude, default_location.longitude); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When was this needed? What error did you run into? Was default_location.latitude undefined at some point?
app/public/service-worker.js
Outdated
reviewRedirect='location/rating/'+id+'/5'; | ||
options.body='Would you like to add a review to '+name+'?'; | ||
} | ||
//self.registration.showNotification(title, options); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does helpNotification do anything after the showNotification was commented out?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, it will be used to update the data based on the new user locations. I'll rename the methods to clear up the confusion since they've changed from what I originally intended when making them
app/public/service-worker.js
Outdated
options = { | ||
body: '', | ||
icon: '/images/logo-192x192.png', | ||
sound: '/sounds/alert.wav', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you forget to commit this public/sounds/alert.wav file?
Also, is it big? mp3 may be better compressed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I must've lost it when I was fixing rebase issues, I'll add it back and change it to an mp3
Will ask users to rate locations if they are within 20 meters of them hourly.
Line 88 in "service-worker.js" sends a sample push notification after 5 seconds for testing purposes. Tester must be near an unrated location for this to work. This line can be removed after testing has succeeded.
Addresses #638