1086698 - [Loop][Server] We should store the user ID if no "issuer" is provided for POST /call-url

Status: RESOLVED FIXED

(Hello (Loop) :: Server, defect)

Description

[Fernando Jiménez Moreno [:ferjm]]

According to [1] the "issuer" parameter allows Loop clients to specify a friendly name of the issuer of the call URL, but so far none of the Loop clients is allowing the user to enter this friendly name. And even if we could allow it from the clients at some point, the user might choose not to specify a friendly name. In this case, we should use the user identity to fill this value if possible. Otherwise, the caller won't see the information about who is she calling on the Loop client UI cause the value of "calleeFriendlyName" from [2] won't be set.

[1] https://docs.services.mozilla.com/loop/apis.html#post-call-url
[2] https://docs.services.mozilla.com/loop/apis.html#get-calls-token

Comment 1

[Fernando Jiménez Moreno [:ferjm]]

Attachment: Link to Github pull-request: https://github.com/mozilla-services/loop-server/pull/230

I'll be adding some tests tomorrow

Comment 2

[Alexis Metaireau (:alexis)]

Comment on attachment 8508832 [details] [review]
Link to Github pull-request: https://github.com/mozilla-services/loop-server/pull/230

Hey fernando,

This works pretty well but needs to have tests in order for us to merge it :)

thanks!

Comment 3

[Alexis Metaireau (:alexis)]

>  I'll be adding some tests tomorrow
Oh, just seen your comment, sorry! I'll let you add them, please r? me when this is done.

Comment 4

[Fernando Jiménez Moreno [:ferjm]]

Comment on attachment 8508832 [details] [review]
Link to Github pull-request: https://github.com/mozilla-services/loop-server/pull/230

Thanks Alexis. Yeah, I had to write the tests :) sorry for the late. I've just updated the PR. It seems that Travis failed, but the error doesn't seem related to the patch. I can't retrigger the job though.

Comment 5

[Alexis Metaireau (:alexis)]

Comment on attachment 8508832 [details] [review]
Link to Github pull-request: https://github.com/mozilla-services/loop-server/pull/230

I've added a few nitpicks, but besides that it looks great. I'll review again once the extra test is added and we don't set the issuer if it's not provided.

Comment 6

[Fernando Jiménez Moreno [:ferjm]]

Comment on attachment 8508832 [details] [review]
Link to Github pull-request: https://github.com/mozilla-services/loop-server/pull/230

Done. Thank you Alexis!

Comment 7

[Alexis Metaireau (:alexis)]

Comment on attachment 8508832 [details] [review]
Link to Github pull-request: https://github.com/mozilla-services/loop-server/pull/230

https://github.com/mozilla-services/loop-server/commit/95f5c74a718e0f35055fe58ad82e4b7c11e67eff

Comment 8

[Alexis Metaireau (:alexis)]

Thanks Fernando!

Comment 9

[Maria Angeles Oteo (:oteo)]

Alexis, in this patch already in development server "https://loop-dev.stage.mozaws.net"?

Comment 10

[Alexis Metaireau (:alexis)]

It wasn't. I've deployed it now.

Comment 11

[Maria Angeles Oteo (:oteo)]

I am still seeing the issue pointing to Dev Server...

Comment 12

[James Bonacci [:jbonacci]]

Reopening since Maria is still seeing the issue...

Comment 13

[Maria Angeles Oteo (:oteo)]

Sorry for the inconvenience, I've retested it again and I think server is working fine, in fact I can see that the user ID/contact name is kept in the call log of the device that clicks on a received shared URL. 

I was wrong because I saw that the the call screen was not showing the ID/contact name but it seems a client issue.

So resolving the bug again.

Comment 14

[James Bonacci [:jbonacci]]

OK. Marking Verified before you change your mind. ;-)
Thanks for retesting this Maria!

Comment 15

[Maria Angeles Oteo (:oteo)]

(In reply to James Bonacci [:jbonacci] from comment #14)
> OK. Marking Verified before you change your mind. ;-)
> Thanks for retesting this Maria!

hahah,

James, do you know when this patch is planned to be in Production server?
Thanks a lot!

Comment 16

[James Bonacci [:jbonacci]]

This could go into the next release (next week).
Not sure if we will have a 0.12.6 or a 0.13.0.
Holding at Resolved/Fixed until this does, indeed, get into Production.
Somehow I misread Dev and Production...

Comment 17

[Alexis Metaireau (:alexis)]

We might want to backport this change to the 0.12.X branch since it's a bugfix. 0.13 will be rooms.

Comment 18

[Maria Angeles Oteo (:oteo)]

As sooner the better