[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
There are no open issues
smillernl
  1. 
      
  2. src/Lunr/Vortex/FCM/FCMDispatcher.php (Diff revision 1)
     
     
    The issue has been resolved. Show all issues

    not google per se, firebase

  3. src/Lunr/Vortex/FCM/FCMDispatcher.php (Diff revision 1)
     
     
    The issue has been resolved. Show all issues

    FCMS?

  4. 
      
p.valk
p.valk
pprkut
  1. 
      
  2. src/Lunr/Vortex/FCM/FCMDispatcher.php (Diff revision 2)
     
     
     
     
     
    The issue has been resolved. Show all issues

    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)
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
    The issue has been resolved. Show all issues

    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)
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
    The issue has been resolved. Show all issues

    I'd rather have this in a separate review

  3. The issue has been resolved. Show all issues

    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)
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
    The issue has been resolved. Show all issues

    these need unit tests

  3. src/Lunr/Vortex/FCM/Tests/FCMDispatcherPushTest.php (Diff revision 7)
     
     
     
     
     
     
     
    The issue has been resolved. Show all issues

    we don't need to retest inheritet functions

  4. src/Lunr/Vortex/GCM/GCMDispatcher.php (Diff revision 7)
     
     
     
     
     
     
     
    The issue has been resolved. Show all issues

    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)
     
     
    The issue has been resolved. Show all issues

    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...