Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Decouple invite from the creation of a PM room #1290

Merged
merged 4 commits into from
Apr 27, 2021
Merged

Decouple invite from the creation of a PM room #1290

merged 4 commits into from
Apr 27, 2021

Conversation

jaller94
Copy link
Contributor

@jaller94 jaller94 commented Apr 13, 2021 •
edited
Loading

Issue: #1254 (comment)

Split off from: #1282 (comment)

@jaller94 jaller94 changed the title J94/decouple invite WIP: Decouple invite from the creation of a PM room Apr 13, 2021
@jaller94 jaller94 self-assigned this Apr 13, 2021
@jaller94 jaller94 mentioned this pull request Apr 13, 2021
Merged
@jaller94 jaller94 marked this pull request as draft April 13, 2021 23:29
@jaller94 jaller94 changed the title WIP: Decouple invite from the creation of a PM room Decouple invite from the creation of a PM room Apr 26, 2021
@jaller94 jaller94 requested a review from a team April 26, 2021 07:53
@jaller94 jaller94 marked this pull request as ready for review April 26, 2021 08:08
Half-Shot
Half-Shot reviewed Apr 26, 2021
View reviewed changes
Copy link
Contributor

@Half-Shot Half-Shot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is okay, but are we inviting the user?

src/bridge/IrcHandler.ts
@@ -167,7 +169,6 @@ export class IrcHandler {
visibility: "private",
// We deliberately set our own power levels below.
// preset: "trusted_private_chat",
invite: [toUserId],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hold on a sec, are we actually inviting the user at all now?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ensureMatrixUserJoined should join the user right after the creation.

matrix-appservice-irc/src/bridge/IrcHandler.ts

Lines 308 to 311 in 7dc74b9

// make sure that the matrix user is (still) in the room
try {
await this.ensureMatrixUserJoined(
pmRoom.getId(), bridgedIrcClient.userId, virtualMatrixUser.getId(), req.log

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works for me.

Test plan

  1. Rebuild & Restart bridge with this branch
  2. As an IRC user, message a Matrix user.
  3. Use /nick new-name to a new username.
  4. Message the Matrix user again.
Peek.2021-04-27.01-34.mp4

Half-Shot
Half-Shot approved these changes Apr 27, 2021
View reviewed changes
Copy link
Contributor

@Half-Shot Half-Shot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this looks great!

@jaller94 jaller94 merged commit ee2b5fc into develop Apr 27, 2021
@jaller94 jaller94 deleted the j94/decouple-invite branch April 27, 2021 09:23
@Half-Shot Half-Shot mentioned this pull request Apr 29, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Half-Shot Half-Shot Half-Shot approved these changes

Assignees

@jaller94 jaller94

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jaller94 @Half-Shot