1025779 - Generation (at POST /call-url) and retrieval (GET /calls/{token}) of the call-url should require/provide an optional "issuer" parameter

Status: VERIFIED FIXED

(Hello (Loop) :: Server, defect, P3)

Description

[Alexis Metaireau (:alexis)]

issuer: Friendly name, if any, configured by person who generated the URL. If no name is configured, this field will be omitted.

Comment 1

[Fernando R. Sela (no CC, needinfo please) [:frsela]]

Attachment: Proposed patch

Comment 2

[Alexis Metaireau (:alexis)]

Comment on attachment 8445140 [details] [review]
Proposed patch

That looks good to me, don't forget to update the tests and the documentation as well!

Comment 3

[Fernando R. Sela (no CC, needinfo please) [:frsela]]

Attachment: Documentation update

Comment 4

[Fernando R. Sela (no CC, needinfo please) [:frsela]]

Comment on attachment 8445140 [details] [review]
Proposed patch

Tests added and issuer renamed as suggested

Comment 5

[Alexis Metaireau (:alexis)]

Comment on attachment 8445207 [details] [review]
Documentation update

There is a misunderstanding about what we're trying to do here. I've commented on the documentation patch, I hope that will be clearer to you (it is for me, at least, thanks).

Comment 6

[Alexis Metaireau (:alexis)]

Comment on attachment 8445140 [details] [review]
Proposed patch

Obviously, implementation is not up to date anymore with what we want, because the docs isn't.

Let's iterate on the doc and then implement if that's okay for you?

Comment 7

[Fernando R. Sela (no CC, needinfo please) [:frsela]]

Comment on attachment 8445140 [details] [review]
Proposed patch

Github comments addressed

Comment 8

[Fernando R. Sela (no CC, needinfo please) [:frsela]]

Comment on attachment 8445207 [details] [review]
Documentation update

Documentation updated based on your comments

Comment 9

[Alexis Metaireau (:alexis)]

Comment on attachment 8445207 [details] [review]
Documentation update

There is still some confusion here. I commented on the github PR.

Comment 10

[Alexis Metaireau (:alexis)]

Comment on attachment 8445140 [details] [review]
Proposed patch

clearing the review on the code PR since the docs aren't right yet.

Comment 11

[Fernando R. Sela (no CC, needinfo please) [:frsela]]

Comment on attachment 8445207 [details] [review]
Documentation update

Fixed

Comment 12

[Alexis Metaireau (:alexis)]

Comment on attachment 8445207 [details] [review]
Documentation update

Cool, that looks better to me, minus the nitpick I did in the PR.

Comment 13

[Fernando R. Sela (no CC, needinfo please) [:frsela]]

(In reply to Alexis Metaireau (:alexis) from comment #12)
> Comment on attachment 8445207 [details] [review]
> Documentation update
> 
> Cool, that looks better to me, minus the nitpick I did in the PR.

\o/
Thanks !
Nit addressed. Now I'll upload the patch.
Regards

Comment 14

[Alexis Metaireau (:alexis)]

Comment on attachment 8445140 [details] [review]
Proposed patch

Tests aren't good enough yet, but we're almost there :)

Comment 15

[Alexis Metaireau (:alexis)]

https://github.com/mozilla-services/loop-server/commit/078f422488ac52e255d5519a9f5764d46cad7c27

Comment 16

[James Bonacci [:jbonacci]]

Verified in code and in unit tests.