callType field in response is set to undefined

RESOLVED FIXED

Status

RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: aoprea, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [qa+])

Attachments

(1 attachment)

56 bytes, text/x-github-pull-request
alexis+bugs
: review+
Details | Review | Splinter Review
Response from [0] has callType field set to undefined.
Everything else in the response seems fine [1]

[0]: http://docs.services.mozilla.com/loop/apis.html#get-calls-version-version
[1]: http://imgur.com/T3gEVqG.png
Possibly related [0] req.callUrlData.callType is undefined but req.body.callType is set to the actual parameter sent in the request.

[0]: https://github.com/mozilla-services/loop-server/blob/master/loop/index.js#L711
Both standalone and desktop share the same conversation view but they call out different server endpoints. Would it make sense to add `callType` to the response from [0] if it would make reusing the same code easier?

[0] http://docs.services.mozilla.com/loop/apis.html#post-calls-token
Flags: needinfo?(alexis+bugs)
(In reply to Andrei Oprea [:andreio] from comment #2)
> Both standalone and desktop share the same conversation view but they call
> out different server endpoints. Would it make sense to add `callType` to the
> response from [0] if it would make reusing the same code easier?

IMHO I don't think so. The callType isn't the only field of difference in the responses, so we'd still need separate handling for the two different endpoints. There could also be potential confusion that in the POST /calls/{token} case, between if its the callType you've just passed to the server, or the call type received from the other end (which, in fact, wouldn't have happened yet).
Created attachment 8467851 [details] [review]
link to github PR

This should fix it!
Attachment #8467851 - Flags: review?(rhubscher)
Flags: needinfo?(alexis+bugs)
https://github.com/mozilla-services/loop-server/commit/853f246fb008481f52fc3c1552d61ca1b9bd3887
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Attachment #8467851 - Flags: review?(rhubscher) → review+
Whiteboard: [qa+]
You need to log in before you can comment on or make changes to this bug.