feat: (bot) proper startup and registration, fix stats

bot/handlers.go

@@ -2,7 +2,6 @@ package bot
 
 import (
 	"fmt"
-	"time"
 
 	"github.com/bwmarrin/discordgo"
 	log "github.com/sirupsen/logrus"
@@ -13,10 +12,9 @@ import (
 // ArchiverBot is the main type passed around throughout the code
 // It has many functions for overall bot management
 type ArchiverBot struct {
-	DB         *gorm.DB
-	DG         *discordgo.Session
-	Config     ArchiverBotConfig
-	StartingUp bool
+	DB     *gorm.DB
+	DG     *discordgo.Session
+	Config ArchiverBotConfig
 }
 
 // ArchiverBotConfig is attached to ArchiverBot so config settings can be
@@ -34,15 +32,10 @@ type ArchiverBotConfig struct {
 
 // BotReadyHandler is called when the bot is considered ready to use the Discord session
 func (bot *ArchiverBot) BotReadyHandler(s *discordgo.Session, r *discordgo.Ready) {
-	// Register all servers the bot is active in
-	for _, g := range r.Guilds {
-		err := bot.registerOrUpdateServer(g)
-		if err != nil {
-			log.Errorf("unable to register or update server: %v", err)
-		}
-	}
-
-	bot.updateServerRegistrations(r.Guilds)
+	// r.Guilds has all of our connected servers, so we should
+	// update server registrations and set any registered servers
+	// not in r.Guilds as inactive
+	bot.updateInactiveRegistrations(r.Guilds)
 
 	// Use this to clean up commands if IDs have changed
 	// TODO remove later if unnecessary
@@ -51,37 +44,48 @@ func (bot *ArchiverBot) BotReadyHandler(s *discordgo.Session, r *discordgo.Ready
 	// globals.RegisteredCommands, err = bot.DG.ApplicationCommandBulkOverwrite(bot.DG.State.User.ID, "", globals.Commands)
 	log.Debug("registering slash commands")
 	var err error
-	existingCommands, err := bot.DG.ApplicationCommands(bot.DG.State.User.ID, "")
-	for _, cmd := range globals.Commands {
-		for _, existingCmd := range existingCommands {
-			if existingCmd.Name == cmd.Name {
-				editedCmd, err := bot.DG.ApplicationCommandEdit(bot.DG.State.User.ID, "", existingCmd.ID, cmd)
-				if err != nil {
-					log.Errorf("cannot update command %s: %v", cmd.Name, err)
+	registeredCommands, err := bot.DG.ApplicationCommands(bot.DG.State.User.ID, "")
+	for _, botCommand := range globals.Commands {
+		for i, registeredCommand := range registeredCommands {
+			// Check if this registered command matches a configured bot command
+			if botCommand.Name == registeredCommand.Name {
+				// Only update if it differs from what's already registered
+				if botCommand != registeredCommand {
+					editedCmd, err := bot.DG.ApplicationCommandEdit(bot.DG.State.User.ID, "", registeredCommand.ID, botCommand)
+					if err != nil {
+						log.Errorf("cannot update command %s: %v", botCommand.Name, err)
+					}
+					globals.RegisteredCommands = append(globals.RegisteredCommands, editedCmd)
+
+					// Bot command was updated, so skip to the next bot command
+					break
 				}
-				globals.RegisteredCommands = append(globals.RegisteredCommands, editedCmd)
-			} else {
-				createdCmd, err := bot.DG.ApplicationCommandCreate(bot.DG.State.User.ID, "", cmd)
+			}
+
+			// Check on the last item of registeredCommands
+			if i == len(registeredCommands) {
+				// This is a stale registeredCommand, so we should delete it
+				err := bot.DG.ApplicationCommandDelete(bot.DG.State.User.ID, "", registeredCommand.ID)
 				if err != nil {
-					log.Errorf("cannot update command %s: %v", cmd.Name, err)
+					log.Errorf("cannot remove command %s: %v", registeredCommand.Name, err)
 				}
-				globals.RegisteredCommands = append(globals.RegisteredCommands, createdCmd)
-
 			}
 		}
-	}
-
-	if err != nil {
-		log.Errorf("cannot update commands: %v", err)
-	}
 
-	if bot.StartingUp {
-		time.Sleep(time.Second * 10)
-		bot.StartingUp = false
-		err := bot.updateServersWatched()
+		// If we're here, then we have a command that needs to be registered
+		createdCmd, err := bot.DG.ApplicationCommandCreate(bot.DG.State.User.ID, "", botCommand)
 		if err != nil {
-			log.Error("unable to update servers watched")
+			log.Errorf("cannot update command %s: %v", botCommand.Name, err)
 		}
+		globals.RegisteredCommands = append(globals.RegisteredCommands, createdCmd)
+		if err != nil {
+			log.Errorf("cannot update commands: %v", err)
+		}
+	}
+
+	err = bot.updateServersWatched()
+	if err != nil {
+		log.Error("unable to update servers watched")
 	}
 }
 

bot/servers.go

@@ -11,7 +11,7 @@ import (
 
 type ServerRegistration struct {
 	DiscordId string `gorm:"primaryKey"`
-	Name      string
+	Name      string `gorm:"default:default"`
 	UpdatedAt time.Time
 	Active    sql.NullBool `pretty:"Bot is active in the server" gorm:"default:true"`
 	Config    ServerConfig `gorm:"foreignKey:DiscordId"`
@@ -24,7 +24,7 @@ type ServerConfig struct {
 	AlwaysArchiveFirst sql.NullBool  `pretty:"Archive the page first (slower)" gorm:"default:false"`
 	ShowDetails        sql.NullBool  `pretty:"Show extra details" gorm:"default:true"`
 	RemoveRetry        sql.NullBool  `pretty:"Remove the retry button automatically" gorm:"default:true"`
-	RetryAttempts      sql.NullInt32 `pretty:"Number of attempts to archive a URL" gorm:"default:1"`
+	RetryAttempts      sql.NullInt32 `pretty:"Number of times to retry calling archive.org" gorm:"default:1"`
 	RemoveRetriesDelay sql.NullInt32 `pretty:"Seconds to wait to remove retry button" gorm:"default:30"`
 	UpdatedAt          time.Time
 }
@@ -44,7 +44,7 @@ func (bot *ArchiverBot) registerOrUpdateServer(g *discordgo.Guild) error {
 	bot.DB.Find(&registration, g.ID)
 	active := sql.NullBool{Bool: true}
 	// The server registration does not exist, so we will create with defaults
-	if (registration == ServerRegistration{}) {
+	if registration.Name == "default" {
 		log.Info("creating registration for new server: ", guild.Name, "(", g.ID, ")")
 		tx := bot.DB.Create(&ServerRegistration{
 			DiscordId: g.ID,
@@ -65,46 +65,43 @@ func (bot *ArchiverBot) registerOrUpdateServer(g *discordgo.Guild) error {
 
 	// Sort of a migration and also a catch-all for registrations that
 	// are not properly saved in the database
-	if !registration.Active.Valid {
+	if registration.Active != active {
 		bot.DB.Model(&ServerRegistration{}).
 			Where(&ServerRegistration{DiscordId: registration.DiscordId}).
 			Updates(&ServerRegistration{Active: active})
 	}
 
-	err = bot.updateServersWatched()
-	if err != nil {
-		return fmt.Errorf("unable to update servers watched: %v", err)
-	}
-
 	return nil
 }
 
-// updateServerRegistrations goes through every server registration and
+// updateInactiveRegistrations goes through every server registration and
 // updates the DB as to whether or not it's active
-func (bot *ArchiverBot) updateServerRegistrations(activeGuilds []*discordgo.Guild) {
+func (bot *ArchiverBot) updateInactiveRegistrations(activeGuilds []*discordgo.Guild) {
 	var sr []ServerRegistration
 	bot.DB.Find(&sr)
-	active := sql.NullBool{Bool: true}
-	inactive := sql.NullBool{Valid: true, Bool: false}
+	var status sql.NullBool
 
-	// Update all registrations for whether or not the server is active
+	// Check all registrations for whether or not the server is active
 	for _, reg := range sr {
 		// If there is no guild in r.Guilds, then we havea config
 		// for a server we're not in anymore
-		reg.Active = inactive
+		status = sql.NullBool{Valid: true, Bool: false}
 		for _, g := range activeGuilds {
 			if g.ID == reg.DiscordId {
-				reg.Active = active
+				status = sql.NullBool{Valid: true, Bool: true}
 			}
 		}
 
-		// Now the registration is accurate, update the DB
-		tx := bot.DB.Model(&ServerRegistration{}).Where(&ServerRegistration{DiscordId: reg.DiscordId}).
-			Updates(reg)
+		// Now the registration is accurate, update the DB if needed
+		if reg.Active != status {
+			reg.Active = status
+			tx := bot.DB.Model(&ServerRegistration{}).Where(&ServerRegistration{DiscordId: reg.DiscordId}).
+				Updates(reg)
 
-		if tx.RowsAffected != 1 {
-			log.Errorf("unexpected number of rows affected updating server registration, id: %s, rows updated: %v",
-				reg.DiscordId, tx.RowsAffected)
+			if tx.RowsAffected != 1 {
+				log.Errorf("unexpected number of rows affected updating server registration, id: %s, rows updated: %v",
+					reg.DiscordId, tx.RowsAffected)
+			}
 		}
 	}
 }
@@ -160,12 +157,10 @@ func (bot *ArchiverBot) updateServersWatched() error {
 		URL:  archiverRepoUrl,
 	}
 
-	if !bot.StartingUp {
-		log.Debug("updating discord bot status")
-		err := bot.DG.UpdateStatusComplex(*updateStatusData)
-		if err != nil {
-			return fmt.Errorf("unable to update discord bot status: %w", err)
-		}
+	log.Debug("updating discord bot status")
+	err := bot.DG.UpdateStatusComplex(*updateStatusData)
+	if err != nil {
+		return fmt.Errorf("unable to update discord bot status: %w", err)
 	}
 
 	return nil

bot/stats.go

@@ -44,7 +44,7 @@ func (bot *ArchiverBot) getGlobalStats() botStats {
 		Group("request_domain_name").Order("count DESC").Find(&topDomains)
 	bot.DB.Model(&ServerRegistration{}).Count(&ServersConfigured)
 	bot.DB.Find(&ServerRegistration{}).Where(&ServerRegistration{
-		Active: sql.NullBool{Bool: true}}).Count(&ServersActive)
+		Active: sql.NullBool{Valid: true, Bool: true}}).Count(&ServersActive)
 
 	var topDomainsFormatted string
 	for i := 0; i < 5 && i < len(topDomains); i++ {

main.go

@@ -117,10 +117,9 @@ func main() {
 	// it for controlling the bot. db is the database object, dg is the
 	// discordgo object
 	archiveBot := bot.ArchiverBot{
-		DB:         db,
-		DG:         dg,
-		Config:     config,
-		StartingUp: true,
+		DB:     db,
+		DG:     dg,
+		Config: config,
 	}
 
 	// Set up DB if necessary