feat: Automatically skip and unsub inactive users #78
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jdkaplan
force-pushed
the
jdkaplan-auto-unsub-inactive
branch
from
9cb1749 to
51fe358
Compare
|
Added myself as a reviewer to put it on my to-do list! |
cceckman
approved these changes
Sep 12, 2024
| @@ -193,6 +214,18 @@ func (pl *PairingLogic) match(w http.ResponseWriter, r *http.Request) { | |||
| if err := pl.pdb.SetNumPairings(ctx, int(timestamp), numPairings); err != nil { | |||
| log.Printf("Failed to record today's pairings: %s", err) | |||
| } | |||
|
|
|||
| for _, r := range inactive { | |||
| if err := pl.rdb.Delete(ctx, r.id); err != nil { | |||
There was a problem hiding this comment.
Maybe: send the message, and then only delete if we successfully sent the message?
I'd hate to see someone "silently" unsubscribed if there's a bug here.
cceckman
reviewed
Sep 12, 2024
| @@ -13,9 +14,14 @@ import ( | |||
|
|
|||
| const owner string = `@_**Maren Beam (SP2'19)**` | |||
| const oddOneOutMessage string = "OK this is awkward.\nThere were an odd number of people in the match-set today, which means that one person couldn't get paired. Unfortunately, it was you -- I'm really sorry :(\nI promise it's not personal, it was very much random. Hopefully this doesn't happen again too soon. Enjoy your day! <3" | |||
| const matchedMessage = "Hi you two! You've been matched for pairing :)\n\nHave fun!" | |||
| const matchedMessage = "Hi you two! You've been matched for pairing :)\n\nHave fun!\n\nNote: In an effort to reduce the frequency of no-show partners, I'll soon start automatically unsubscribing users that I haven't heard from in a while. Please message me back so I know you're an active user (and messages in this chat count!) :heart:" | |||
There was a problem hiding this comment.
Maybe: "... in a while. Please reply (or message me separately) so I know you're an active user! ❤️ Thank you!"
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a potential implementation of inactive user handling (#75). This is a draft because I have not tested it at all.
From the issue/comments on #75 itself:
I've called this
openPairingshere, but I'm very open to alternative names!I chose to hook into the existing pairing cron for this. This gives users as much time as possible to respond to the existing pairing. It's also a time we're already regularly sending out lots of messages.
The consensus seems to be 3, but it's a quick patch to change it :)
Added! I'm still tuning my sense of "writing as Pairing Bot", so feedback is very much welcome!
I agree! I could see us sending a warning message at
max - 1, but I didn't want this to get too complicated before I actually understand the code 😅I've tried to extend the pairing message to indicate that we're looking for chat interaction now, but I struggled to find a great way to word it without making it a whole thing. Help please!