Remove suggestion limit logic (#187)

README.md

@@ -5,7 +5,7 @@ Supports you in keeping track of your beancount transactions also if you need to
 ## Features and advantages
 
 * [x] Quickly record beancount transactions while on-the-go. Start as simple as entering the amount - no boilerplate
-* [x] Suggestions for accounts and descriptions used in the past or configured manually (can be limited or turned off)
+* [x] Suggestions for accounts and descriptions used in the past or configured manually
 * [x] Templates with variables and advanced amount splitting for recurring or more complex transactions
 * [x] Reminder notifications of recorded transactions with flexible schedule
 * [x] Many optional commands, shorthands and parameters, leaving the full flexibility up to you

bot/config.go

@@ -17,7 +17,6 @@ func (bc *BotController) configHandler(m *tb.Message) {
 		Add("currency", bc.configHandleCurrency).
 		Add("tag", bc.configHandleTag).
 		Add("notify", bc.configHandleNotification).
-		Add("limit", bc.configHandleLimit).
 		Add("about", bc.configHandleAbout).
 		Add("tz_offset", bc.configHandleTimezoneOffset).
 		Add("delete_account", bc.configHandleAccountDelete)
@@ -51,11 +50,6 @@ Create a schedule to be notified of open transactions (i.e. not archived or dele
 /{{.CONFIG_COMMAND}} notify off - Disable reminder notifications
 /{{.CONFIG_COMMAND}} notify <delay> <hour> - Notify of open transaction after <delay> days at <hour> of the day. Honors configured timezone offset (see below)
 
-Set suggestion cache limits (i.e. only cache new values until limit is reached, then old ones get dismissed if new ones are added):
-
-/{{.CONFIG_COMMAND}} limit - Get currently set cache limits
-/{{.CONFIG_COMMAND}} limit <suggestionType> <amount>|off - Set or disable suggestion limit for a type
-
 Timezone offset from {{.TZ}} to honor for notifications and current date (if set automatically) in new transactions:
 
 /{{.CONFIG_COMMAND}} tz_offset - Get current timezone offset from {{.TZ}} (default 0)
@@ -225,92 +219,6 @@ func (bc *BotController) configHandleNotification(m *tb.Message, params ...strin
 	bc.configHelp(m, fmt.Errorf("invalid amount of parameters specified"))
 }
 
-func (bc *BotController) configHandleLimit(m *tb.Message, params ...string) {
-	if len(params) == 0 {
-		// GET limits for all types
-		cacheLimits, err := bc.Repo.CacheUserSettingGetLimits(m)
-		if err != nil {
-			bc.Logf(WARN, m, "CacheUserSettingGetLimits failed: %s", err.Error())
-			bc.configHelp(m, fmt.Errorf("could not get your cache limits"))
-			return
-		}
-		message := "You have the following cache limits configured:\n"
-		for limit, value := range cacheLimits {
-			message += fmt.Sprintf("\n%s: %d", limit, value)
-		}
-		message += "\n\n"
-		message += "If new cache entries are created for the given types, old ones are automatically deleted.\nPlease note: If suggestions were added using /suggestions add, they will not be deleted automatically by this mechanism. '-1' means no limit."
-		_, err = bc.Bot.Send(Recipient(m), message)
-		if err != nil {
-			bc.Logf(ERROR, m, "Sending bot message failed: %s", err.Error())
-		}
-		return
-	} else if len(params) == 1 {
-		// GET limit for type
-		cacheType := params[0]
-		cacheLimits, err := bc.Repo.CacheUserSettingGetLimits(m)
-		if err != nil {
-			bc.Logf(WARN, m, "CacheUserSettingGetLimits failed: %s", err.Error())
-			bc.configHelp(m, fmt.Errorf("an application error occurred while retrieving cache limits from database"))
-			return
-		}
-		message := "You have the following cache limit configured for '" + cacheType + "': " + strconv.Itoa(cacheLimits[cacheType])
-		message += "\n\n"
-		message += "If new cache entries are created for the given types, old ones are automatically deleted.\nPlease note: If suggestions were added using /suggestions add, they will not be deleted automatically by this mechanism. '-1' means no limit."
-		_, err = bc.Bot.Send(Recipient(m), message)
-		if err != nil {
-			bc.Logf(ERROR, m, "Sending bot message failed: %s", err.Error())
-		}
-		return
-	} else if len(params) == 2 {
-		// SET limit for type
-		cacheType := params[0]
-		limitValue := params[1]
-
-		if !helpers.ArrayContains(helpers.AllowedSuggestionTypes(), cacheType) {
-			bc.configHelp(m, fmt.Errorf("unknown suggestion type: %s. Must be one from: %v", cacheType, helpers.AllowedSuggestionTypes()))
-			return
-		}
-
-		limitValueParsed, errParsing := strconv.Atoi(limitValue)
-		if errParsing != nil {
-			bc.configHelp(m, fmt.Errorf("an application error occurred while interpreting your amount as a number to set the limit to"))
-			return
-		}
-
-		if limitValue == "off" || (errParsing == nil && limitValueParsed < 0) {
-			err := bc.Repo.CacheUserSettingSetLimit(m, cacheType, -1)
-			if err != nil {
-				bc.Logf(ERROR, m, "Error disabling suggestions cache limit: %s", err.Error())
-				bc.configHelp(m, fmt.Errorf("an application error occurred while disabling suggestions cache: %s", err.Error()))
-				return
-			}
-			_, err = bc.Bot.Send(Recipient(m), fmt.Sprintf("Successfully disabled suggestions cache limits for type %s", cacheType))
-			if err != nil {
-				bc.Logf(ERROR, m, "Sending bot message failed: %s", err.Error())
-			}
-		} else {
-			err := bc.Repo.CacheUserSettingSetLimit(m, cacheType, limitValueParsed)
-			if err != nil {
-				bc.Logf(ERROR, m, "Error setting suggestions cache limit: %s", err.Error())
-				bc.configHelp(m, fmt.Errorf("an application error occurred while setting a new suggestions cache limit: %s", err.Error()))
-				return
-			}
-			_, err = bc.Bot.Send(Recipient(m), fmt.Sprintf("Successfully set suggestions cache limits for type %s to %d", cacheType, limitValueParsed))
-			if err != nil {
-				bc.Logf(ERROR, m, "Sending bot message failed: %s", err.Error())
-			}
-		}
-		err := bc.Repo.PruneUserCachedSuggestions(m)
-		if err != nil {
-			bc.Logf(WARN, m, "Could not prune suggestions: %s", err.Error())
-		}
-		return
-	}
-
-	bc.configHelp(m, fmt.Errorf("invalid amount of parameters specified"))
-}
-
 func (bc *BotController) configHandleAbout(m *tb.Message, params ...string) {
 	if len(params) > 0 {
 		bc.configHelp(m, fmt.Errorf("no parameters expected"))
@@ -420,7 +328,6 @@ func (bc *BotController) deleteUserData(m *tb.Message) {
 
 	errors.handle1(bc.Repo.SetUserSetting(helpers.USERSET_ADM, "", m.Chat.ID))
 	errors.handle1(bc.Repo.SetUserSetting(helpers.USERSET_CUR, "", m.Chat.ID))
-	errors.handle1(bc.Repo.SetUserSetting(helpers.USERSET_LIM_PREFIX, "", m.Chat.ID))
 	errors.handle1(bc.Repo.SetUserSetting(helpers.USERSET_TAG, "", m.Chat.ID))
 	errors.handle1(bc.Repo.SetUserSetting(helpers.USERSET_TZOFF, "", m.Chat.ID))
 

bot/controller.go

@@ -687,11 +687,6 @@ func (bc *BotController) finishTransaction(m *tb.Message, tx Tx) {
 		bc.Logf(ERROR, m, "Something went wrong while caching transaction. Error: %s", err.Error())
 		// Don't return, instead continue flow (if recording was successful)
 	}
-	err = bc.Repo.PruneUserCachedSuggestions(m)
-	if err != nil {
-		bc.Logf(ERROR, m, "Something went wrong while pruning suggestions cache. Error: %s", err.Error())
-		// Don't return, instead continue flow (if recording was successful)
-	}
 
 	_, err = bc.Bot.Send(Recipient(m), fmt.Sprintf("Successfully recorded your transaction.\n"+
 		"You can get a list of all your transactions using /%s. "+

db/crud/bot_cache.go

@@ -2,9 +2,6 @@ package crud
 
 import (
 	"database/sql"
-	"fmt"
-	"strconv"
-	"strings"
 
 	"github.com/LucaBernstein/beancount-bot-tg/helpers"
 	tb "gopkg.in/tucnak/telebot.v2"
@@ -44,41 +41,6 @@ func (r *Repo) PutCacheHints(m *tb.Message, values map[string]string) error {
 	return r.FillCache(m)
 }
 
-func (r *Repo) PruneUserCachedSuggestions(m *tb.Message) error {
-	limits, err := r.CacheUserSettingGetLimits(m)
-	if err != nil {
-		return err
-	}
-	for key, limit := range limits {
-		if limit < 0 {
-			continue
-		}
-		// Cleanup automatically added values above cache limit
-		q := `DELETE FROM "bot::cache" WHERE "tgChatId" = $1 AND "type" = $2`
-		params := []interface{}{m.Chat.ID, key}
-		if limit != 0 {
-			q += ` AND ID NOT IN (SELECT "id" FROM "bot::cache" WHERE "tgChatId" = $1 AND "type" = $2 ORDER BY "lastUsed" DESC LIMIT $3)`
-			params = append(params, limit)
-		}
-		res, err := r.db.Exec(q, params...)
-		if err != nil {
-			return err
-		}
-		count, err := res.RowsAffected()
-		if err != nil {
-			LogDbf(r, helpers.WARN, m, "could not get affected rows count from pruning operation")
-		}
-		if count > 0 {
-			LogDbf(r, helpers.INFO, m, "Pruned %d '%s' suggestions", count, key)
-		}
-	}
-	err = r.FillCache(m)
-	if err != nil {
-		LogDbf(r, helpers.WARN, m, "could not get refill cache after pruning")
-	}
-	return nil
-}
-
 func (r *Repo) GetCacheHints(m *tb.Message, key string) ([]string, error) {
 	if _, exists := CACHE_LOCAL[m.Chat.ID]; !exists {
 		LogDbf(r, helpers.TRACE, m, "No cached data found for chat. Will fill cache first.")
@@ -154,78 +116,3 @@ func (r *Repo) DeleteAllCacheEntries(m *tb.Message) error {
 	}
 	return r.FillCache(m)
 }
-
-func (r *Repo) CacheUserSettingGetLimits(m *tb.Message) (limits map[string]int, err error) {
-	userSettingPrefix := helpers.USERSET_LIM_PREFIX
-
-	rows, err := r.db.Query(`SELECT "setting", "value" FROM "bot::userSetting" WHERE "tgChatId" = $1 AND "setting" LIKE $2`,
-		m.Chat.ID, userSettingPrefix+"%")
-	if err != nil {
-		return nil, fmt.Errorf("error selecting userSettingCacheLimits: %s", err.Error())
-	}
-
-	allCacheLimits := map[string]int{}
-
-	var setting string
-	var value string
-	for rows.Next() {
-		err = rows.Scan(&setting, &value)
-		if err != nil {
-			return nil, err
-		}
-		valueI, err := strconv.Atoi(value)
-		if err != nil {
-			return nil, fmt.Errorf("could not convert stored limit number to integer: %s", err.Error())
-		}
-		allCacheLimits[strings.TrimPrefix(setting, userSettingPrefix)] = valueI
-	}
-
-	// Fill with possible but not modified cache limits
-	for _, key := range helpers.AllowedSuggestionTypes() {
-		_, exists := allCacheLimits[key]
-		if !exists {
-			allCacheLimits[key] = -1
-		}
-	}
-
-	return allCacheLimits, nil
-}
-
-// CacheUserSettingSetLimit deletes probably existing and (re)creates caching limit in userSettings
-func (r *Repo) CacheUserSettingSetLimit(m *tb.Message, key string, limit int) (err error) {
-	if !helpers.ArrayContains(helpers.AllowedSuggestionTypes(), key) {
-		return fmt.Errorf("the key you provided is invalid. Should be one of the following: %s", helpers.AllowedSuggestionTypes())
-	}
-
-	tx, err := r.db.Begin()
-	defer tx.Rollback()
-	if err != nil {
-		return fmt.Errorf("caching userSettingLimit did not work as db tx could not be begun: %s", err.Error())
-	}
-
-	compositeKey := helpers.USERSET_LIM_PREFIX + key
-
-	q := `DELETE FROM "bot::userSetting" WHERE "tgChatId" = $1 AND "setting" = $2;`
-	params := []interface{}{m.Chat.ID, compositeKey}
-
-	_, err = tx.Exec(q, params...)
-	if err != nil {
-		return fmt.Errorf("could not delete existing userSetting on set: %s", err.Error())
-	}
-
-	if limit >= 0 {
-		q := `INSERT INTO "bot::userSetting" ("tgChatId", "setting", "value") VALUES ($1, $2, $3);`
-		params := []interface{}{m.Chat.ID, compositeKey, strconv.Itoa(limit)}
-		_, err = tx.Exec(q, params...)
-		if err != nil {
-			return fmt.Errorf("could not delete existing userSetting on set: %s", err.Error())
-		}
-	}
-
-	err = tx.Commit()
-	if err != nil {
-		return fmt.Errorf("could not commit db tx for setting userSetting: %s", err.Error())
-	}
-
-	return nil
-}

db/crud/bot_cache_test.go

@@ -2,13 +2,11 @@ package crud_test
 
 import (
 	"log"
-	"strings"
 	"testing"
 
 	"github.com/DATA-DOG/go-sqlmock"
 	"github.com/LucaBernstein/beancount-bot-tg/bot"
 	"github.com/LucaBernstein/beancount-bot-tg/db/crud"
-	"github.com/LucaBernstein/beancount-bot-tg/helpers"
 	tb "gopkg.in/tucnak/telebot.v2"
 )
 
@@ -58,91 +56,3 @@ func TestCacheOnlySuggestible(t *testing.T) {
 		t.Errorf("there were unfulfilled expectations: %s", err)
 	}
 }
-
-func TestSetCacheLimit(t *testing.T) {
-	// create test dependencies
-	crud.TEST_MODE = true
-	chat := &tb.Chat{ID: 12345}
-	db, mock, err := sqlmock.New()
-	if err != nil {
-		log.Fatal(err)
-	}
-	defer db.Close()
-
-	bc := crud.NewRepo(db)
-	message := &tb.Message{Chat: chat}
-
-	mock.ExpectBegin()
-	mock.
-		ExpectExec(`DELETE FROM "bot::userSetting"`).
-		WithArgs(chat.ID, helpers.USERSET_LIM_PREFIX+"description").
-		WillReturnResult(sqlmock.NewResult(1, 1))
-	mock.
-		ExpectExec(`INSERT INTO "bot::userSetting"`).
-		WithArgs(chat.ID, helpers.USERSET_LIM_PREFIX+"description", "23").
-		WillReturnResult(sqlmock.NewResult(1, 1))
-	mock.ExpectCommit()
-
-	err = bc.CacheUserSettingSetLimit(message, "description", 23)
-	if err != nil {
-		t.Errorf("CacheUserSettingSetLimit unexpectedly threw an error: %s", err.Error())
-	}
-
-	mock.ExpectBegin()
-	mock.
-		ExpectExec(`DELETE FROM "bot::userSetting"`).
-		WithArgs(chat.ID, helpers.USERSET_LIM_PREFIX+"description").
-		WillReturnResult(sqlmock.NewResult(1, 1))
-	mock.ExpectCommit()
-
-	err = bc.CacheUserSettingSetLimit(message, "description", -1)
-	if err != nil {
-		t.Errorf("CacheUserSettingSetLimit (with delete only) unexpectedly threw an error: %s", err.Error())
-	}
-
-	err = bc.CacheUserSettingSetLimit(message, "thisCacheKeyDefinitelyIsInvalidAndShouldFail", 5)
-	if err == nil {
-		t.Errorf("CacheUserSettingSetLimit should fail for invalid cache key")
-	}
-	if !strings.Contains(err.Error(), "key you provided is invalid") {
-		t.Errorf("CacheUserSettingSetLimit error message should provide further information for failure due to invalid key: %s", err.Error())
-	}
-
-	if err := mock.ExpectationsWereMet(); err != nil {
-		t.Errorf("there were unfulfilled expectations: %s", err)
-	}
-}
-
-func TestGetCacheLimit(t *testing.T) {
-	// create test dependencies
-	crud.TEST_MODE = true
-	chat := &tb.Chat{ID: 12345}
-	db, mock, err := sqlmock.New()
-	if err != nil {
-		log.Fatal(err)
-	}
-	defer db.Close()
-
-	bc := crud.NewRepo(db)
-	message := &tb.Message{Chat: chat}
-
-	mock.
-		ExpectQuery(`SELECT "setting", "value" FROM "bot::userSetting"`).
-		WithArgs(chat.ID, helpers.USERSET_LIM_PREFIX+"%").
-		WillReturnRows(sqlmock.NewRows([]string{"setting", "value"}).AddRow(helpers.USERSET_LIM_PREFIX+"description", "79"))
-
-	limits, err := bc.CacheUserSettingGetLimits(message)
-	if err != nil {
-		t.Errorf("TestSetCacheLimitGet unexpectedly threw an error: %s", err.Error())
-	}
-	if len(limits) != 2 {
-		t.Errorf("TestSetCacheLimitGet limit result was unexpected")
-	}
-	if limits["description"] != 79 {
-		t.Errorf("TestSetCacheLimitGet should return values correctly: %d", limits["txDesc"])
-	}
-
-	if err := mock.ExpectationsWereMet(); err != nil {
-		t.Errorf("there were unfulfilled expectations: %s", err)
-	}
-}

db/migrations/v11.go

@@ -83,14 +83,4 @@ func v11RemoveUserSettingTypesLimits(db *sql.Tx) {
 	if err != nil {
 		log.Fatal(err)
 	}
-
-	sqlStatement = `
-	INSERT INTO "bot::userSettingTypes"
-		("setting", "description")
-	VALUES ('user.limitCache', 'limit cached value count for transactions. Array by type.');;
-	`
-	_, err = db.Exec(sqlStatement)
-	if err != nil {
-		log.Fatal(err)
-	}
 }