Closed Bug 1346519 Opened 3 years ago Closed 5 months ago

Automatically create rooms for joined rooms on server

Categories

(Chat Core :: Matrix, enhancement)

enhancement
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Instantbird 72

People

(Reporter: matrixisreal, Assigned: clokep)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 3 obsolete files)

Not sure if this is required yet, But other clients work the same way,
Assignee: nobody → pavankarthikboddeda
This is definitely wanted! Something I found annoying when using Matrix in Instantbird.
Hmm,, Im not exactly sure what would be the best way to do this.

1. We could join all the rooms while calling account.connect()

2. Something else :P ,
Is there any function which the prpls call where this has to be implemented? let me know.
            *** Small Conv regarding this on IRC 

12:44:27 <matrixisreal> clokep_work: regarding the auto join matrix rooms bug, how/ where do I handle that. 
12:44:28 <matrixisreal> One way is to do it in connect () , but it doesnt seem to be elegant, 
12:44:28 <matrixisreal> and also there doesnt seem to ba a MatrixROom Protottype implemented yet (Would adding that help?)
12:47:06 <clokep_work> Why would we need a prototype?
12:47:38 <clokep_work> matrixisreal: I think doing it right *after* connect is probably the way to go, but I'm not 100% sure how we get that list of rooms, etc. :)
12:47:47 <clokep_work> So yeah I think you're on the right path of doing it after the connection is done.
12:50:13 <matrixisreal> clokep_work: A list of room objects is obtained by calling getRooms on client.
12:50:13 <matrixisreal> and each room object(matrix-sdk room) has a room name attr, using which we call JoinRoom()
12:50:26 <matrixisreal> Sounds  ok ?
12:51:13 <clokep_work> matrixisreal: I think that sounds correct.
Moving Matrix bugs to the new Matrix component.
Component: General → Matrix
Depends on: 1347202
Depends on: 1394397
Assignee: pavankarthikboddeda → nobody
Blocks: 1572636
Attached patch Patch v1 (obsolete) — Splinter Review

This uses the getJoinedRooms API to query the server for the joined rooms and then creates the proper UI elements.

This also implements other tweaks / fixes broken behavior:

  • Properly sets the joining / left flags on the conversation when joining a room.
  • Renames initListOfParticipants to initRoom since it now does more.
  • Uses some more arrow functions.

I tested this by logging into Matrix and seeing my channels pop up. Some of them appear with weird IDs, but that's bug 1347533.

Assignee: nobody → clokep
Status: NEW → ASSIGNED
Attachment #9104113 - Flags: review?(florian)
Summary: Matrix Protocol :: Auto Join the rooms in which the account is already in. → Automatically create rooms for joined rooms on server
Attached patch Patch v2 (obsolete) — Splinter Review

Forgot to run eslint over this.

Attachment #9104113 - Attachment is obsolete: true
Attachment #9104113 - Flags: review?(florian)
Attachment #9104118 - Flags: review?(florian)
Attached patch Patch v3 (obsolete) — Splinter Review

I found a minor bug in the previous logic. It treated an "invited" user as a "left" user, which is kind of weird.

Attachment #9104118 - Attachment is obsolete: true
Attachment #9104118 - Flags: review?(florian)
Attachment #9104124 - Flags: review?(florian)
Attached patch Patch v3.1Splinter Review

Unbitrotted.

The way I tested this was by having joined channels in my riot.im web-app. Connecting with Thunderbird should have all the channels be joined.

This also fixed a couple of other minor bugs:

  • Invited users were treated like left users.
  • The icons showing if you have left or are joining a channel was not working (the little loading icon when you first join a room).
Attachment #9104124 - Attachment is obsolete: true
Attachment #9104124 - Flags: review?(florian)
Attachment #9104966 - Flags: review?(paul)
Comment on attachment 9104966 [details] [diff] [review]
Patch v3.1

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

Looks good, with one nit.  When I opened up TB, several rooms that I had joined using the riot.im web app were automatically joined.  I saw the loading icon when I joined a room within TB.

One glitch I noticed, maybe for a follow-up, is the room names that were automatically joined appeared as random letters, something like `!Vhtashteshaeckide:matrix.org`.  The room I joined from within TB chat was human readable as expected (`#rust:matrix.org`).

::: chat/protocols/matrix/matrix.js
@@ +88,5 @@
> +  initRoom(aRoom) {
> +    // Store the ID of the room to look up information in the future.
> +    this._roomId = aRoom.roomId;
> +
> +    // If there's any participants, create them.

Grammar nit:  If there are any...
Attachment #9104966 - Flags: review?(paul) → review+

(In reply to Paul Morris [:pmorris] from comment #9)

Looks good, with one nit. When I opened up TB, several rooms that I had
joined using the riot.im web app were automatically joined. I saw the
loading icon when I joined a room within TB.

This behavior seems fine -- what about it seems incorrect?

One glitch I noticed, maybe for a follow-up, is the room names that were
automatically joined appeared as random letters, something like
!Vhtashteshaeckide:matrix.org. The room I joined from within TB chat was
human readable as expected (#rust:matrix.org).

This is bug 1347533...although this made me realize I should figure out why this works for rooms that are manually joined. I'll test one or two things out and if this isn't immediately fixable I'm going to leave it to the other bug.

::: chat/protocols/matrix/matrix.js
@@ +88,5 @@

  • initRoom(aRoom) {
  • // Store the ID of the room to look up information in the future.
  • this._roomId = aRoom.roomId;
  • // If there's any participants, create them.

Grammar nit: If there are any...

Thanks! I'll fix that before check-in.

Thanks for testing!

(In reply to Patrick Cloke [:clokep] from comment #10)

One glitch I noticed, maybe for a follow-up, is the room names that were
automatically joined appeared as random letters, something like
!Vhtashteshaeckide:matrix.org. The room I joined from within TB chat was
human readable as expected (#rust:matrix.org).

This is bug 1347533...although this made me realize I should figure out why this works for rooms that are manually joined. I'll test one or two things out and if this isn't immediately fixable I'm going to leave it to the other bug.

OK, so the process for joining is a bit different from the UI. When you type in the alias (instead of the ID) we already know the alias...so we use it as the name. There's definitely something off between these two code paths, but I'd like to handle it in the follow-up.

Pushed by clokep@gmail.com:
https://hg.mozilla.org/comm-central/rev/1902ef02323e
Auto-join Matrix rooms. r=pmorris

Status: ASSIGNED → RESOLVED
Closed: 5 months ago
Resolution: --- → FIXED
Target Milestone: --- → Instantbird 72

(In reply to Patrick Cloke [:clokep] from comment #10)

Looks good, with one nit. When I opened up TB, several rooms that I had
joined using the riot.im web app were automatically joined. I saw the
loading icon when I joined a room within TB.

This behavior seems fine -- what about it seems incorrect?

Oh, that is fine! I was just reporting what I did, but I made it confusing by doing that right after mentioning the nit, which was just the grammar nit in the review below. Makes sense about the follow-up for the room names.

You need to log in before you can comment on or make changes to this bug.