Merge pull request 'Help: fall back to a private message in the channel when DMs fail' (#33) from fix/help-dms-off into dev

Reviewed-on: #33

docs/commands.md

@@ -35,7 +35,7 @@ Anyone can use these. `/rps`, `/rpsstats` and `/rpsleaderboard` work in servers
 
 | Command | What it does |
 | --- | --- |
-| `/help` | DMs you a command reference |
+| `/help` | DMs you a command reference, or shows it to you in the channel if DMs are off |
 
 ## Leveling
 

docs/features.md

@@ -117,4 +117,4 @@ When the bot is added to a server, it applies safe defaults (modules off) and DM
 
 ## Help
 
-`/help` DMs a command reference. The text is hand-written in `internal/commands/help/help.go`, so update it when you add or change a command.
+`/help` DMs a command reference. If the member has DMs off, or Discord refuses the DM, it shows the same help as a message only they can see. The text is hand-written in `internal/commands/help/help.go`, so update it when you add or change a command.

internal/commands/help/help.go

@@ -1,6 +1,7 @@
 package help
 
 import (
+	"fmt"
 	"log"
 	"unicode/utf8"
 
@@ -97,39 +98,85 @@ func embeds(author *discordgo.MessageEmbedAuthor) []*discordgo.MessageEmbed {
 	return append(out, cur)
 }
 
+// HelpHandler sends the help by direct message. If that isn't possible (the
+// member has DMs off, or shares no server with the bot), it shows the help to
+// them in the channel instead, as a message only they can see.
 func HelpHandler(s *discordgo.Session, i *discordgo.InteractionCreate) {
-	s.InteractionRespond(i.Interaction, &discordgo.InteractionResponse{
-		Type: discordgo.InteractionResponseChannelMessageWithSource,
-		Data: &discordgo.InteractionResponseData{
-			Content: "Check your DMs for help.",
-			Flags:   discordgo.MessageFlagsEphemeral,
-		},
-	})
-
-	userID := ""
-
-	// check if command is run in a guild
-	if i.Member != nil {
-		userID = i.Member.User.ID
-	} else {
-		userID = i.Interaction.User.ID
-	}
-
-	dmChannel, err := s.UserChannelCreate(userID)
-	if err != nil {
-		log.Printf("Error creating DM channel: %v", err)
-		return
-	}
-
 	author := discordgo.MessageEmbedAuthor{
 		Name: "Velox Bot",
 		URL:  "https://gitea.fernandovideira.com/FernandoJVideira/velox-bot",
 	}
+	list := embeds(&author)
 
-	for _, e := range embeds(&author) {
+	// Used in a DM with the bot, there is nowhere else to send it.
-		if _, err := s.ChannelMessageSendEmbed(dmChannel.ID, e); err != nil {
+	if i.GuildID == "" {
-			log.Printf("Error sending help: %v", err)
+		_ = s.InteractionRespond(i.Interaction, &discordgo.InteractionResponse{
-			return
+			Type: discordgo.InteractionResponseChannelMessageWithSource,
+			Data: &discordgo.InteractionResponseData{Embeds: list},
+		})
+		return
+	}
+
+	// Answer first (sending the DM can take a moment), and edit the answer
+	// once we know where the help went.
+	if err := s.InteractionRespond(i.Interaction, &discordgo.InteractionResponse{
+		Type: discordgo.InteractionResponseDeferredChannelMessageWithSource,
+		Data: &discordgo.InteractionResponseData{Flags: discordgo.MessageFlagsEphemeral},
+	}); err != nil {
+		log.Printf("help: cannot answer: %v", err)
+		return
+	}
+
+	userID := ""
+	if i.Member != nil && i.Member.User != nil {
+		userID = i.Member.User.ID
+	} else if i.User != nil {
+		userID = i.User.ID
+	}
+
+	if err := sendByDM(s, userID, list); err != nil {
+		// Typically "Cannot send messages to this user": DMs are off.
+		// The context says where the command came from, which is what to look at
+		// when "no mutual guilds" appears for someone who is in a server with
+		// the bot: a command run through a user install, or in a server the bot
+		// isn't actually a member of, has no shared server for Discord to see.
+		log.Printf("help: DM not possible, showing it here (guild %q, context %d, installs %v, %s): %v",
+			i.GuildID, i.Context, i.AuthorizingIntegrationOwners, botMembership(s, i.GuildID), err)
+		note := "I couldn't DM you (your DMs may be off), so here it is."
+		_, _ = s.InteractionResponseEdit(i.Interaction, &discordgo.WebhookEdit{Content: &note, Embeds: &list})
+		return
+	}
+	done := "Check your DMs for help."
+	_, _ = s.InteractionResponseEdit(i.Interaction, &discordgo.WebhookEdit{Content: &done})
+}
+
+func sendByDM(s *discordgo.Session, userID string, list []*discordgo.MessageEmbed) error {
+	dm, err := s.UserChannelCreate(userID)
+	if err != nil {
+		return err
+	}
+	for _, e := range list {
+		if _, err := s.ChannelMessageSendEmbed(dm.ID, e); err != nil {
+			return err
 		}
 	}
+	return nil
+}
+
+// botMembership says whether the bot is really a member of the server, which
+// is what Discord means by "mutual guilds". A server where the app was added
+// with only the applications.commands scope runs slash commands fine but has
+// no bot user in it.
+func botMembership(s *discordgo.Session, guildID string) string {
+	if s == nil || s.State == nil || s.State.User == nil {
+		return "bot user unknown"
+	}
+	inCache := false
+	if g, err := s.State.Guild(guildID); err == nil && g != nil {
+		inCache = true
+	}
+	if _, err := s.GuildMember(guildID, s.State.User.ID); err != nil {
+		return fmt.Sprintf("bot is NOT a member of that server (in gateway cache: %v): %v", inCache, err)
+	}
+	return fmt.Sprintf("bot is a member of that server (in gateway cache: %v)", inCache)
 }

internal/commands/help/help_test.go

@@ -1,6 +1,9 @@
 package help
 
 import (
+	"io"
+	"net/http"
+	"strings"
 	"testing"
 	"unicode/utf8"
 
@@ -39,6 +42,78 @@ func TestHelpFitsDiscordsLimits(t *testing.T) {
 	}
 }
 
+// fakeDiscord answers the calls /help makes and records what the member was
+// shown. dm decides whether the direct message works.
+type fakeDiscord struct {
+	dm        bool
+	edited    []byte
+	callbacks int
+	sent      int
+}
+
+func (f *fakeDiscord) RoundTrip(r *http.Request) (*http.Response, error) {
+	reply := func(code int, body string) (*http.Response, error) {
+		return &http.Response{StatusCode: code, Body: io.NopCloser(strings.NewReader(body)), Header: http.Header{"Content-Type": {"application/json"}}, Request: r}, nil
+	}
+	switch {
+	case strings.HasSuffix(r.URL.Path, "/callback"):
+		f.callbacks++
+		return reply(204, "")
+	case strings.HasSuffix(r.URL.Path, "/users/@me/channels"):
+		if !f.dm {
+			return reply(403, `{"message":"Cannot send messages to this user","code":50007}`)
+		}
+		return reply(200, `{"id":"900","type":1}`)
+	case strings.Contains(r.URL.Path, "/channels/900/messages"):
+		f.sent++
+		return reply(200, `{"id":"1"}`)
+	case strings.Contains(r.URL.Path, "/messages/@original"):
+		f.edited, _ = io.ReadAll(r.Body)
+		return reply(200, `{"id":"2"}`)
+	}
+	return reply(404, `{"message":"unexpected `+r.URL.Path+`"}`)
+}
+
+func run(t *testing.T, f *fakeDiscord, guild string) {
+	t.Helper()
+	s, err := discordgo.New("Bot x")
+	if err != nil {
+		t.Fatal(err)
+	}
+	s.Client = &http.Client{Transport: f}
+	s.MaxRestRetries = 0
+	i := &discordgo.InteractionCreate{Interaction: &discordgo.Interaction{
+		ID: "1", Token: "tok", AppID: "100", GuildID: guild,
+		Member: &discordgo.Member{User: &discordgo.User{ID: "42"}},
+	}}
+	HelpHandler(s, i)
+}
+
+func TestHelpGoesToDMs(t *testing.T) {
+	f := &fakeDiscord{dm: true}
+	run(t, f, "5")
+	if f.sent == 0 || !strings.Contains(string(f.edited), "Check your DMs") || strings.Contains(string(f.edited), `"embeds"`) {
+		t.Errorf("sent=%d edited=%s", f.sent, f.edited)
+	}
+}
+
+func TestHelpFallsBackToTheChannelWhenDMsAreOff(t *testing.T) {
+	f := &fakeDiscord{dm: false}
+	run(t, f, "5")
+	got := string(f.edited)
+	if f.sent != 0 || !strings.Contains(got, "couldn't DM you") || !strings.Contains(got, `"embeds"`) || !strings.Contains(got, "Velox Bot Help") {
+		t.Errorf("the help should be shown in the reply: sent=%d edited=%s", f.sent, got)
+	}
+}
+
+func TestHelpInADMIsShownDirectly(t *testing.T) {
+	f := &fakeDiscord{dm: true}
+	run(t, f, "")
+	if f.callbacks != 1 || f.sent != 0 || f.edited != nil {
+		t.Errorf("one direct answer, no DM: %+v", f)
+	}
+}
+
 // The help is meant to be wide and short: inline fields, so Discord puts
 // three side by side, and it should fit in one message.
 func TestHelpIsCompactAndInline(t *testing.T) {