search

jazzband / django-push-notifications

Send push notifications to mobile devices through GCM or APNS in Django.
MIT License
2.24k stars 609 forks source link

Add Edge support to webpush #631

Closed simonkern closed 2 years ago

simonkern commented 2 years ago

This adds Edge support to webpush and showcases an easier way to extract browsername and version for Chrome, Edge, and Opera in the README.

Any feedback would be greatly appreciated.

codecov[bot] commented 2 years ago

Codecov Report

Merging #631 (42e6ac4) into master (879eee2) will decrease coverage by 0.02%. The diff coverage is n/a.

:exclamation: Current head 42e6ac4 differs from pull request most recent head a0149ce. Consider uploading reports for the commit a0149ce to get more accurate results Impacted file tree graph

@@            Coverage Diff             @@
##           master     #631      +/-   ##
==========================================
- Coverage   68.24%   68.21%   -0.03%     
==========================================
  Files          24       24              
  Lines        1099     1098       -1     
  Branches      173      173              
==========================================
- Hits          750      749       -1     
  Misses        312      312              
  Partials       37       37
Impacted Files Coverage Δ
push_notifications/conf/app.py 75.15% <ø> (ø)
push_notifications/models.py 78.04% <ø> (ø)
push_notifications/settings.py 91.66% <ø> (ø)
push_notifications/fields.py 60.93% <0.00%> (-0.61%) :arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 879eee2...a0149ce. Read the comment docs.

Andrew-Chen-Wang commented 2 years ago

@simonkern I don't know if this statement is correct, but the edge url seems to change all the time? Is it possible to make this an EDGE setting?

simonkern commented 2 years ago

@simonkern I don't know if this statement is correct, but the edge url seems to change all the time? Is it possible to make this an EDGE setting?

I was not able to produce an URL with another domain so far. However, I was also unable to find proper documentation on this matter. But if this URL would change all the time, it would be necessary to adapt the model to store the baseURL as well, imho. What do you think and suggest?

Andrew-Chen-Wang commented 2 years ago

I'm not sure what you mean by adapt the model to store the URL. I was suggesting in settings.py, instead of hard coding the url, allow for user customization like the chrome url. Additionally, make sure to create a migration!

simonkern commented 2 years ago

Doesn't WP_POST_URL already solve this?

Andrew-Chen-Wang commented 2 years ago

AHH but wait migration file!

simonkern commented 2 years ago

Oh sorry, I forgot to include the migration, it's now included.

Andrew-Chen-Wang commented 2 years ago

Thanks again!