[Vortex] FCM Dispatcher

Review Request #483 — Created Jan. 3, 2017 and submitted

p.valk
Lunr
vortex-4
517, 516, 482
10ac8ae...
lunr

Added Dispatcher based on GCM

unittests.

  • 0
  • 0
  • 11
  • 0
  • 11
Description From Last Updated
smillernl
  1. 
      
  2. src/Lunr/Vortex/FCM/FCMDispatcher.php (Diff revision 1)
     
     

    not google per se, firebase

  3. src/Lunr/Vortex/FCM/FCMDispatcher.php (Diff revision 1)
     
     

    https://firebase.google.com/docs/cloud-messaging/concept-options#setting-the-priority-of-a-message

  4. 
      
p.valk
p.valk
pprkut
  1. 
      
  2. src/Lunr/Vortex/FCM/FCMDispatcher.php (Diff revision 2)
     
     
     
     
     

    why is this not extending the GCM Dispatcher class?

    1. the string at line 72 const FIREBASE_SEND_URL = 'https://fcm.googleapis.com/fcm/send'; is different.
      cant override string value, so or change this upstream and make it settable trough some function or this.

    2. why wouldn't you be able to change the string value? Just redefine the constant

  3. 
      
p.valk
pprkut
  1. 
      
  2. src/Lunr/Vortex/FCM/FCMDispatcher.php (Diff revision 3)
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     

    It's probably best if you solve this differently, otherwise you have to fix the push_batch() method as well.

    Easiest would probably be to add object getter methods in the GCMDispatcher class for the response objects and have this class just override those object getters.

  3. 
      
p.valk
pprkut
  1. 
      
  2. src/Lunr/Vortex/GCM/GCMDispatcher.php (Diff revisions 3 - 4)
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     

    I'd rather have this in a separate review

  3. This is in the wrong review request

  4. 
      
p.valk
p.valk
p.valk
pprkut
  1. 
      
  2. src/Lunr/Vortex/FCM/FCMDispatcher.php (Diff revision 7)
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     

    these need unit tests

  3. src/Lunr/Vortex/FCM/Tests/FCMDispatcherPushTest.php (Diff revision 7)
     
     
     
     
     
     
     

    we don't need to retest inheritet functions

  4. src/Lunr/Vortex/GCM/GCMDispatcher.php (Diff revision 7)
     
     
     
     
     
     
     

    All changes to this file should go in a separate review

  5. 
      
p.valk
p.valk
p.valk
p.valk
p.valk
pprkut
  1. 
      
  2. src/Lunr/Vortex/FCM/FCMDispatcher.php (Diff revision 11)
     
     

    I don't think so

    1. ah copy paste bug cought me again >_<

  3. 
      
p.valk
p.valk
Review request changed

Status: Closed (submitted)

Change Summary:

Merged into master

Loading...