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

    Unused?

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

    Why is this public?

  4. src/Lunr/Vortex/PAP/PAPPayload.php (Diff revision 1)
     
     
     
     
     
    The issue has been resolved. Show all issues

    Also public

  5. 
      
p.valk
pprkut
  1. 
      
  2. src/Lunr/Vortex/MPNS/MPNSDispatcher.php (Diff revision 2)
     
     
     
     
    The issue has been resolved. Show all issues

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

  3. src/Lunr/Vortex/MPNS/MPNSPayload.php (Diff revision 2)
     
     
     
     
     
     
     
    The issue has been resolved. Show all issues

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

  4. src/Lunr/Vortex/PAP/PAPPayload.php (Diff revision 2)
     
     
    The issue has been resolved. Show all issues

    How is that a string?

  5. 
      
smillernl
  1. 
      
  2. src/Lunr/Vortex/MPNS/MPNSPayload.php (Diff revision 2)
     
     
    The issue has been resolved. Show all issues

    integer*

  3. 
      
p.valk
p.valk
p.valk
k.woortman
  1. 
      
  2. src/Lunr/Vortex/MPNS/MPNSPayload.php (Diff revision 5)
     
     
    The issue has been resolved. Show all issues

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

    No check for valid values?

  3. src/Lunr/Vortex/MPNS/MPNSPayload.php (Diff revision 6)
     
     
    The issue has been resolved. Show all issues

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

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

    that's not an integer

  6. 
      
p.valk
pprkut
  1. 
      
  2. src/Lunr/Vortex/MPNS/MPNSPriority.php (Diff revision 7)
     
     
    The issue has been resolved. Show all issues

    DEFAULT

  3. The issue has been resolved. Show all issues

    DEFAULT should be in there

  4. 
      
p.valk
p.valk
Review request changed

Status: Closed (submitted)

Change Summary:

Merged into master

Loading...