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.

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

Add offline messaging #8440

Merged
merged 15 commits into from
Nov 4, 2023
Merged

Add offline messaging #8440

merged 15 commits into from
Nov 4, 2023

Conversation

mia-pi-git
Copy link
Member

@mia-pi-git mia-pi-git commented Aug 20, 2021

Few things of note here.
FIrstly, as I was concerned about database load, all PMs are temporary. When user A sends a PM to user B while offline, the PM is retained for 60 days (assuming the user has not seen it). After that, it is deleted. When a user logs in, their pending PMs are sent to them, and they are all marked as seen (set to the current UNIX timestamp). The user may replay these PMs via a chat page for up to one week, after which they are deleted.
I have also presently unified the /blockpms command with the setting (stored in the database) that marks if a user wants to receive offline pms (and from what rank, if so). This can be forcibly set via Config.usesqlitepms, which indicates the rank that users must be to send offline PMs. If either the config setting or the user setting is set to friends, only friended users may send that user offline PMs.
Note that with the offline blockpms we currently do not support 'autoconfirmed' and 'unlocked' settings. Locked users should be blocked by default, though.
Closes #2151.

@mia-pi-git mia-pi-git requested a review from monsanto as a code owner August 20, 2021 21:02
@AnnikaCodes
Copy link
Collaborator

The user may replay these PMs via a chat page for up to one week, after which they are deleted.

Since we retain PMs for moderation purposes anyway, is there a reason we can't let them replay them indefinitely? I'm fairly sure SQLite WAL mode allows for concurrent reading and writing of the database. If this is some private thing feel free to move this to the Dev discord.

lib/sql.ts Outdated Show resolved Hide resolved
@@ -726,6 +773,13 @@ export const commands: Chat.ChatCommands = {
user.settings.blockPMs = true;
this.sendReply(this.tr`You are now blocking private messages, except from staff.`);
}
let saveValue: string | boolean | null = user.settings.blockPMs;
if (!saveValue) saveValue = 'none';
// todo: can we do this? atm. no.
Copy link
Collaborator

Choose a reason for hiding this comment

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

?

Copy link
Member Author

Choose a reason for hiding this comment

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

'unlocked' and 'autoconfirmed' settings aren't supported since that's.... awful to handle.

server/friends.ts Outdated Show resolved Hide resolved
server/friends.ts Outdated Show resolved Hide resolved
server/friends.ts Show resolved Hide resolved
server/private-messages/index.ts Show resolved Hide resolved
server/private-messages/index.ts Outdated Show resolved Hide resolved
server/private-messages/index.ts Outdated Show resolved Hide resolved
}
if (!Users.globalAuth.atLeast(user, Config.usesqlitepms)) {
if (forceBool) return false;
throw new Chat.ErrorMessage("You do not have the needed rank to send offline PMs.");
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should handle the Config.usesqlitepms === "friends" case (which will always lead to the error message being thrown here) and also probably say what rank is needed.

Copy link
Member Author

Choose a reason for hiding this comment

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

Why would we ever set this to friends? It should be an auth group.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Are we not planning to allow offline PMs only from friends? I'm pretty sure that was the idea, but I haven't thought about this in a year.

Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe? I think we'd need to discuss that. It does seem like maybe that should be up to users, but maybe we could default to friends.

Comment on lines 65 to 66
if (Config.usesqlitepms === 'friends') {
if (!user.friends?.has(pmTarget)) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why the nesting?

Copy link
Member Author

Choose a reason for hiding this comment

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

Mostly thought it looked nicer. Not super invested.

Co-authored-by: Annika <annika0uwu@gmail.com>
@AnnikaCodes AnnikaCodes marked this pull request as draft July 21, 2022 02:28
@mia-pi-git mia-pi-git marked this pull request as ready for review October 24, 2022 20:17
@mia-pi-git
Copy link
Member Author

Bump?

@mia-pi-git mia-pi-git merged commit 0211066 into smogon:master Nov 4, 2023
underscoreevelyn pushed a commit to underscoreevelyn/pokemon-showdown that referenced this pull request Dec 2, 2023
MathyFurret pushed a commit to MathyFurret/pokemon-showdown that referenced this pull request May 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Offline messaging
2 participants