"callId" means two different things in conversation store

RESOLVED FIXED in Firefox 35

Status

defect
P1
normal
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: standard8, Assigned: standard8)

Tracking

unspecified
mozilla36
Points:
1
Dependency tree / graph
Bug Flags:
in-testsuite +
qe-verify -

Firefox Tracking Flags

(firefox35 fixed, firefox36+ fixed)

Details

(Whiteboard: [tech-debt])

Attachments

(1 attachment)

Assignee

Description

5 years ago
In the conversationStore, "callId" initially means the random id generated by the MozLoopService, but then it means the "callId" generated by the server for the particular call.

We need to clarify, and probably separate out the implementations of callId.
backlog: --- → Fx35?

Updated

5 years ago
backlog: Fx35? → Fx37?
Whiteboard: [tech debt]
Assignee

Comment 1

5 years ago
I need this for bug 1090209 - otherwise, its just too confusing to try and unravel the mess there.
Assignee: nobody → standard8
Iteration: --- → 36.2
Points: --- → 1
Depends on: 1090209
Whiteboard: [tech debt] → [tech-debt]

Updated

5 years ago
backlog: Fx37? → Fx35+
Priority: -- → P1
Assignee

Comment 2

5 years ago
This does the necessary replacing and helps to clarify what the id is for. I've used conversationWindowId from gecko, and windowId from within the actual conversation window code. I think that should make enough sense.

There's more to come building on this in bug 1090209 and bug 1074678.
Attachment #8513479 - Flags: review?(mdeboer)
Comment on attachment 8513479 [details] [diff] [review]
Replace 'callId' with 'windowId' in a Loop conversation window so that it represents what it is and is distinct from the real 'callId'

Review of attachment 8513479 [details] [diff] [review]:
-----------------------------------------------------------------

Well, boy, does this look a lot cleaner!

::: browser/components/loop/LoopCalls.jsm
@@ +363,4 @@
>     *
>     * The result of this call will be a free call session slot.
>     *
> +   * @param {int} conversationWindowId

Nit: can you change this to {Number} while you're here?
Attachment #8513479 - Flags: review?(mdeboer) → review+
Assignee

Comment 4

5 years ago
Landed with nit fixed:

https://hg.mozilla.org/integration/fx-team/rev/c0993b1e68c0
Target Milestone: --- → mozilla36
Assignee

Updated

5 years ago
Blocks: 1090209
No longer depends on: 1090209
https://hg.mozilla.org/mozilla-central/rev/c0993b1e68c0
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
[Tracking Requested - why for this release]: blocks Loop 35
Flags: qe-verify-
Flags: in-testsuite?
Comment on attachment 8513479 [details] [diff] [review]
Replace 'callId' with 'windowId' in a Loop conversation window so that it represents what it is and is distinct from the real 'callId'

Approval Request Comment
Landed on aurora per IRC with lsblakk with a=loop-only
Attachment #8513479 - Flags: approval-mozilla-aurora?
Attachment #8513479 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Assignee

Comment 9

4 years ago
This landed with unit tests.
Flags: in-testsuite? → in-testsuite+
You need to log in before you can comment on or make changes to this bug.