[Vortex] Adapt payload priority handling for push notifications

Review Request #626 — Created Sept. 14, 2018 and submitted

p.valk
Lunr
vortex
48b12b7...
lunr

Vortex: Adapt payload priority handling for push notifications

unitests.

  • 0
  • 0
  • 14
  • 0
  • 14
Description From Last Updated
Mathijs
  1. 
      
  2. src/Lunr/Vortex/MPNS/MPNSPayload.php (Diff revision 1)
     
     

    Unused?

  3. src/Lunr/Vortex/MPNS/MPNSPayload.php (Diff revision 1)
     
     
     
     
     
     

    Why is this public?

  4. src/Lunr/Vortex/PAP/PAPPayload.php (Diff revision 1)
     
     
     
     
     

    Also public

  5. 
      
p.valk
pprkut
  1. 
      
  2. src/Lunr/Vortex/MPNS/MPNSDispatcher.php (Diff revision 2)
     
     
     
     

    Don't get it twice, just put it in a variable

  3. src/Lunr/Vortex/MPNS/MPNSPayload.php (Diff revision 2)
     
     
     
     
     
     
     

    We had a verification about correct values before. Would be nice to keep that.

  4. src/Lunr/Vortex/PAP/PAPPayload.php (Diff revision 2)
     
     

    How is that a string?

  5. 
      
smillernl
  1. 
      
  2. src/Lunr/Vortex/MPNS/MPNSPayload.php (Diff revision 2)
     
     

    integer*

  3. 
      
p.valk
p.valk
p.valk
k.woortman
  1. 
      
  2. src/Lunr/Vortex/MPNS/MPNSPayload.php (Diff revision 5)
     
     

    I don't really like having all these numbers here...

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

    No check for valid values?

  3. src/Lunr/Vortex/MPNS/MPNSPayload.php (Diff revision 6)
     
     

    why not use a constant here?

    1. why constant? this is the priority of the payload, it needs to be able to be set.

    2. no no, I mean a MPNSPriority constant as value

    3. We can do that but currently the old default was 0, All MPNSPriority values are different then 0.
      I would just keep it as is for the 'not existing' backward compatibility.

    4. I would just add a new constant for that

  4. src/Lunr/Vortex/MPNS/MPNSPayload.php (Diff revision 6)
     
     
     
     
     
     
     
     
     
     
     
     

    you can get the list of constants dynamically

    1. ok, also do that for the unittests or keep those as is?

    2. those can stay as is

  5. src/Lunr/Vortex/PAP/PAPPayload.php (Diff revision 6)
     
     

    that's not an integer

  6. 
      
p.valk
pprkut
  1. 
      
  2. src/Lunr/Vortex/MPNS/MPNSPriority.php (Diff revision 7)
     
     

    DEFAULT

  3. DEFAULT should be in there

  4. 
      
p.valk
p.valk
Review request changed

Status: Closed (submitted)

Change Summary:

Merged into master

Loading...