[Vortex] WNS: Fixes the issues with sending notifications to the WNS service

Review Request #416 — Created Feb. 10, 2016 and submitted

smillernl
Lunr
personal/sean/wns_fixes
a6928fc...
lunr
Fixes the issues with sending notifications to the WNS service


  • 0
  • 0
  • 4
  • 0
  • 4
Description From Last Updated
smillernl
smillernl
tardypad
  1. 
      
  2. src/Lunr/Vortex/WNS/WNSDispatcher.php (Diff revision 3)
     
     
     
     
     
     

    I think this part is not unit tested

    Also I would return FALSE in case of an error like done elsewhere

    Since you've changed what this function can return, make sure as well that this new case is handled correctly where you call it

  3. src/Lunr/Vortex/WNS/WNSDispatcher.php (Diff revision 3)
     
     
     
     

    errors might happen making the http response not a correct JSON

  4. 
      
smillernl
smillernl
tardypad
  1. 
      
  2. src/Lunr/Vortex/WNS/WNSDispatcher.php (Diff revision 5)
     
     
     
     
     
     

    I don't feel this is the correct way to check if an error occured with the request.
    From what I see, the result is either FALSE or NULL on failure.
    I think you better check the error_number returned

  3. src/Lunr/Vortex/WNS/WNSDispatcher.php (Diff revision 5)
     
     

    this key is not necessarily in the json object response

  4. 
      
smillernl
smillernl
tardypad
  1. Ship It!
  2. 
      
pprkut
  1. Ship It!
  2. 
      
smillernl
Review request changed

Status: Closed (submitted)

Change Summary:

Merged into master

Loading...