Room owner receives a "Someone joins a Room" notification when he leaves his own room

RESOLVED FIXED

Status

Firefox OS
Gaia::Loop
--
critical
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: oteo, Assigned: frsela)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [Room1.1.1_Exploratory1][tef-triage])

Attachments

(1 attachment)

(Reporter)

Description

3 years ago
Reproduced with master Loop client (future 1.1.1 release) and latest 2.0 FxOS version

STR:
1. Contact A creates a Room1 and shares it with Contact B.
2. Contact B joins that Room1
3. Contact A receives a notification that Contact B has joined Room1
4. Contact A joins Room1
5. Contact A (owner) leaves Room1 (while Contact B is still in the Room1)

Actual Result: 
"Contact B has joined Room1" notification is received in Device A

Expected Result: 
No notification should be received
(Reporter)

Updated

3 years ago
Blocks: 1107894
Assignee: nobody → frsela
Created attachment 8546564 [details] [review]
Proposed patch

Hi, M.A.,

Can you check if this patch fixes the issue?
Attachment #8546564 - Flags: feedback?(oteo)
(Reporter)

Comment 2

3 years ago
Comment on attachment 8546564 [details] [review]
Proposed patch

Testing your branch, I can not the reproduce the issue :) but I think that something could have been broken with the patch.

With your branch, sometimes (I could not find a concrete STR), the notification that owner receives when an invited user joins a room is not received and what I reproduces 100% is this Bug 1111709 - "Someone joins to a Room" not shown if Loop app is killed. This issue was already resolved and I have verified that continues working fine in master so seems to be a regression from your patch.

Thanks a lot Fernando!
Attachment #8546564 - Flags: feedback?(oteo)
Attachment #8546564 - Flags: review?(josea.olivera)
Attachment #8546564 - Flags: feedback?(borja.bugzilla)
(Reporter)

Comment 3

3 years ago
Before continuing with the revision I've been rechecking the branch with the latest code and although the issues reported in comment 2 has been solved, I have detected that if the Room owner is already in the RoomX and after that, the invited user joins that RoomX, the owner is receiving the notification that the invited has joined RoomX.

According to the WF and the Acceptance criteria, this notification should not be received.
Only if the owner is not in that RoomX, that notification will be received.
(Reporter)

Comment 4

3 years ago
(In reply to Maria Angeles Oteo (:oteo) from comment #3)
> Only if the owner is not in that RoomX, that notification will be received.

I forgot to mention that the notification is also received if the owner is in the RoomX but it's in background mode and the invited user joins RoomX
Comment on attachment 8546564 [details] [review]
Proposed patch

Current patch drops the work we added at bug 1113117. Could you have a look at it please? Request review at me again when you are done. Thanks!
Attachment #8546564 - Flags: review?(josea.olivera)
(In reply to José Antonio Olivera Ortega [:jaoo] from comment #5)
> Comment on attachment 8546564 [details] [review]
> Proposed patch - WIP
> 
> Current patch drops the work we added at bug 1113117. Could you have a look
> at it please? Request review at me again when you are done. Thanks!

Fixed
Left a few nits more on the PR, please address them and review at me when you are done. Thanks.
(In reply to José Antonio Olivera Ortega [:jaoo] from comment #7)
> Left a few nits more on the PR, please address them and review at me when
> you are done. Thanks.

Nits addressed. Thank you JAOO
(Reporter)

Updated

3 years ago
Whiteboard: [Room1.1.1_Exploratory1]
Status: NEW → ASSIGNED
(Reporter)

Updated

3 years ago
Severity: normal → critical
Whiteboard: [Room1.1.1_Exploratory1] → [Room1.1.1_Exploratory1][tef-triage]
Comment on attachment 8546564 [details] [review]
Proposed patch

Great job!
Attachment #8546564 - Flags: feedback?(borja.bugzilla) → feedback+
Comment on attachment 8546564 [details] [review]
Proposed patch

Nits changed and rebased. A last check :p

Thank you !
Attachment #8546564 - Attachment description: Proposed patch - WIP → Proposed patch
Attachment #8546564 - Flags: review?(josea.olivera)
Comment on attachment 8546564 [details] [review]
Proposed patch

LGTM. r=me

Well done Fernando! Thanks!
Attachment #8546564 - Flags: review?(josea.olivera) → review+
Rebased again & Landed: https://github.com/mozilla-b2g/firefoxos-loop-client/commit/ccc98abf6519f7c625e23650b51b5a19fa0f075c

Thank you Jaoo and Borja for your reviews and feedbacks.
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.