🔧 fix(db): remove duplicate legacy farmer records and improve handling

[mkmccarty]

No description provided.

[Copilot]

Pull request overview

This PR aims to fix duplicate legacy farmer records in the database and improve their handling. The changes include adding a new query to remove duplicate records, fixing the update/insert logic, and removing redundant save operations.

Key changes:

• Added ClearExtraLegacyRecords query to remove duplicate farmer_state records based on MIN(rowid) grouped by id and key
• Fixed update/insert logic by changing the check from rows != 1 to rows == 0 to correctly handle the case when no records exist
• Removed redundant saveSqliteData call in SetEggIncName since SetMiscSettingString already handles the save internally
• Added debugging log for Discord channel parent type checking

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File	Description
src/farmerstate/query.sql	Added ClearExtraLegacyRecords and DeleteFarmerLegacyRecords queries to handle duplicate farmer state records
src/farmerstate/query.sql.go	Generated Go functions for the new SQL queries
src/farmerstate/farmerstate.go	Added call to clear duplicates, fixed update/insert logic, and removed redundant saveSqliteData call
src/boost/contract.go	Added logging for debugging parent channel type detection

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

This query DeleteFarmerLegacyRecords is a duplicate of the existing DeleteLegacyFarmerstate query (lines 20-22). Both perform identical operations: deleting records where id = ? and key = 'legacy'. Consider removing this duplicate and using the existing DeleteLegacyFarmerstate query instead.

Suggested change

	-- name: DeleteFarmerLegacyRecords :exec
	DELETE FROM farmer_state
	WHERE id = ? AND key = 'legacy';

[Copilot]

This function DeleteFarmerLegacyRecords is a duplicate of the existing DeleteLegacyFarmerstate function (which should exist from the query defined in query.sql lines 20-22). Both perform identical operations. Consider removing this duplicate and using the existing function instead.

Suggested change

	const deleteFarmerLegacyRecords = `-- name: DeleteFarmerLegacyRecords :exec
	DELETE FROM farmer_state
	WHERE id = ? AND key = 'legacy'
	`
	func (q *Queries) DeleteFarmerLegacyRecords(ctx context.Context, id string) error {
	_, err := q.db.ExecContext(ctx, deleteFarmerLegacyRecords, id)
	return err
	}

[Copilot]

The comment "Record exists, update instead" is misleading. When rows == 0, it means the UPDATE affected 0 rows (i.e., no record was found to update), so the code correctly performs an INSERT. The comment should say "No record exists, insert instead" or similar to accurately reflect the logic.

Suggested change

	// Record exists, update instead
	// No record exists, insert instead

[Copilot]

Potential nil pointer dereference: parentCh could be nil if s.Channel(ch.ParentID) returns an error. The error is being ignored with _, but parentCh is then accessed without a nil check. This could cause a panic. Add a nil check: if parentCh != nil && parentCh.Type == discordgo.ChannelTypeGuildCategory {

Suggested change

	if parentCh.Type == discordgo.ChannelTypeGuildCategory {
	if parentCh != nil && parentCh.Type == discordgo.ChannelTypeGuildCategory {