-
Notifications
You must be signed in to change notification settings - Fork 152
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.
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
optionally make users wait to join bridged rooms #1645
base: develop
Are you sure you want to change the base?
Conversation
a more ideal future here would be one involving knocking, i.e. 60 minutes after you knock, you are invited, or something like that. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As you mentioned, inviting people would probably make for better UX.
Is there a way to:
- First time the user knocks, send them a message along the lines of "For user safety, this bridge is configured with a join delay. You will be allowed to join on ${date}."
- After this delay, send an invite.
Maybe?
I don't believe knocking is widely implemented yet |
Sorry, I shouldn't have written "knocks" but rather "attempts to join". |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Promising stuff, I've got some suggestions.
@@ -1072,8 +1072,13 @@ export class IrcBridge { | |||
if (userIds) { | |||
for (const userId of userIds) { | |||
this.activityTracker.setLastActiveTime(userId); | |||
this.dataStore.updateLastSeenTimeForUser(userId).catch((ex) => { | |||
log.warn(`Failed to bump last active time for ${userId} in database`, ex); | |||
this.dataStore.getFirstSeenTimeForUser(userId).then((when) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd make this it's own function inside datastore personally, and have the logic for setting first seen time handled within. We might even be able to wrap the postgres stuff into one statement.
src/bridge/MatrixHandler.ts
Outdated
user.getId(), | ||
req, | ||
true, | ||
`Please wait ${remaining} seconds`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We probably need to let the user know why they are waiting :). Either a message to their admin room, or a link to a page.
@@ -66,6 +66,10 @@ export interface BridgeConfig { | |||
inactiveAfterDays?: number; | |||
}; | |||
banLists?: MatrixBanSyncConfig; | |||
delayBridging?: { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think these need more descriptions about what they do. It might even make sense to have this go under the membershipLists
configuration where we configure Matrix joins.
We should certainly make it obvious that these are seconds.
ALTER TABLE last_seen ADD COLUMN first BIGINT; | ||
ALTER TABLE last_seen RENAME COLUMN ts TO last; | ||
UPDATE last_seen SET first = last; | ||
ALTER TABLE last_seen ALTER COLUMN first SET NOT NULL; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That might explode for all the existing items that don't have a value set.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see the line above it; we backfill first
from last
couple of
TODO
s still; looking for feedback on if i'm going in the right direction