Delete list entry by number (#182)

Add `/list numbered` and `/list rm <number>`

README.md

@@ -32,6 +32,8 @@ You can use the bot [`@LB_Bean_Bot`](https://t.me/LB_Bean_Bot) ([https://t.me/LB
 * `/cancel`: Cancel either the current transaction recording questionnaire or the creation of a new template.
 * `/comment` or `/c`: Add arbitrary text to the transaction list (e.g. for follow-ups). Example: `/c Checking account balance needs to be asserted`. (Note that no comment prefix (`;`) is added automatically, so that by default the entered comment string causes a syntax error in a beancount file to ease follow-up and so that comments don't drown in long transaction lists)
 * `/list`: Show a list of all currently recorded transactions (for easy copy-and-paste into your beancount file). The parameter `/list dated` adds a comment prior to each transaction in the list with the date and time the transaction has been added. `/list archived` shows all archived transactions. The parameters can also be used in conjunction, i.e. `/list archived dated`.
+  * `/list [archived] numbered`: Shows the transactions list with preceded number identifier. 
+  * `/list [archived] rm <number>`: Remove a single transaction from the list
 * `/archiveAll`: Mark all currently opened transactions as archived. They can be revisited using `/list archived`.
 * `/deleteAll yes`: Permanently delete all transactions, both open and archived.
 

bot/controller.go

@@ -2,6 +2,7 @@ package bot
 
 import (
 	"fmt"
+	"strconv"
 	"strings"
 	"time"
 
@@ -117,7 +118,7 @@ func (bc *BotController) commandMappings() []*CMD {
 		{CommandAlias: []string{CMD_SIMPLE}, Handler: bc.commandCreateSimpleTx, Help: "Record a simple transaction, defaults to today; Can be omitted by sending amount directy", Optional: []string{"date"}},
 		{CommandAlias: CMD_COMMENT, Handler: bc.commandAddComment, Help: "Add arbitrary text to transaction list"},
 		{CommandAlias: CMD_TEMPLATE, Handler: bc.commandTemplates, Help: "Create and use template transactions"},
-		{CommandAlias: []string{CMD_LIST}, Handler: bc.commandList, Help: "List your recorded transactions", Optional: []string{"archived", "dated"}},
+		{CommandAlias: []string{CMD_LIST}, Handler: bc.commandList, Help: "List your recorded transactions or remove entries", Optional: []string{"archived", "dated", "numbered", "rm <number>"}},
 		{CommandAlias: []string{CMD_SUGGEST}, Handler: bc.commandSuggestions, Help: "List, add or remove suggestions"},
 		{CommandAlias: []string{CMD_CONFIG}, Handler: bc.commandConfig, Help: "Bot configurations"},
 		{CommandAlias: []string{CMD_ARCHIVE_ALL}, Handler: bc.commandArchiveTransactions, Help: "Archive recorded transactions"},
@@ -292,6 +293,9 @@ func (bc *BotController) commandList(m *tb.Message) {
 	command := strings.Split(m.Text, " ")
 	isArchived := false
 	isDated := false
+	isNumbered := false
+	isDeleteCommand := false
+	elementNumber := -1
 	if len(command) > 1 {
 		for _, option := range command[1:] {
 			if option == "archived" {
@@ -300,15 +304,33 @@ func (bc *BotController) commandList(m *tb.Message) {
 			} else if option == "dated" {
 				isDated = true
 				continue
+			} else if option == "numbered" {
+				isNumbered = true
+				continue
+			} else if option == "rm" {
+				isDeleteCommand = true
+				continue
+			} else {
+				var err error
+				elementNumber, err = strconv.Atoi(option)
+				if err != nil {
+					_, err := bc.Bot.Send(Recipient(m), fmt.Sprintf("The option '%s' could not be recognized. Please try again with '/list', with options added to the end separated by space.", option), clearKeyboard())
+					if err != nil {
+						bc.Logf(ERROR, m, "Sending bot message failed: %s", err.Error())
+					}
+					return
+				}
+				continue
 			}
-
-			_, err := bc.Bot.Send(Recipient(m), fmt.Sprintf("The option '%s' could not be recognized. Please try again with '/list', with options added to the end separated by space.", option), clearKeyboard())
-			if err != nil {
-				bc.Logf(ERROR, m, "Sending bot message failed: %s", err.Error())
-			}
-			return
 		}
 	}
+	if isDeleteCommand && (isNumbered || isDated || elementNumber <= 0) {
+		_, err := bc.Bot.Send(Recipient(m), "For removing a single element from the list, determine it's number by sending the command '/list numbered' and then removing an entry by sending '/list rm <number>'.", clearKeyboard())
+		if err != nil {
+			bc.Logf(ERROR, m, "Sending bot message failed: %s", err.Error())
+		}
+		return
+	}
 	tx, err := bc.Repo.GetTransactions(m, isArchived)
 	if err != nil {
 		_, err := bc.Bot.Send(Recipient(m), "Something went wrong retrieving your transactions: "+err.Error(), clearKeyboard())
@@ -321,11 +343,33 @@ func (bc *BotController) commandList(m *tb.Message) {
 		bc.Logf(ERROR, m, "Tx unexpectedly was nil")
 		return
 	}
-
+	if isDeleteCommand {
+		var err error
+		if elementNumber <= len(tx) {
+			elementDbId := tx[elementNumber-1].Id
+			err = bc.Repo.DeleteTransaction(m, isArchived, elementDbId)
+		} else {
+			err = fmt.Errorf("the number you specified was too high. Please use a correct number as seen from '/list [archived] numbered'")
+		}
+		if err != nil {
+			_, errSending := bc.Bot.Send(Recipient(m), "Something went wrong while trying to delete a single transaction: "+err.Error(), clearKeyboard())
+			if errSending != nil {
+				bc.Logf(ERROR, m, "Sending bot message failed: %s", errSending.Error())
+			}
+			return
+		}
+		_, errSending := bc.Bot.Send(Recipient(m), "Successfully deleted the list entry specified.", clearKeyboard())
+		if errSending != nil {
+			bc.Logf(ERROR, m, "Sending bot message failed: %s", errSending.Error())
+		}
+		return
+	}
 	SEP := "\n"
 	txList := []string{}
+	txEntryNumber := 0
 	for _, t := range tx {
 		var dateComment string
+		txEntryNumber++
 		if isDated {
 			tzOffset := bc.Repo.UserGetTzOffset(m)
 			timezoneOff := time.Duration(tzOffset) * time.Hour
@@ -340,13 +384,21 @@ func (bc *BotController) commandList(m *tb.Message) {
 				dateComment = "; recorded on " + date + SEP
 			}
 		}
-		txMessage := dateComment + t.Tx
+		numberPrefix := ""
+		if isNumbered {
+			numberPrefix = fmt.Sprintf("%d) ", txEntryNumber)
+		}
+		txMessage := dateComment + numberPrefix + t.Tx
 		txList = append(txList, txMessage)
 	}
 	messageSplits := bc.MergeMessagesHonorSendLimit(txList, "\n")
 	if len(messageSplits) == 0 {
+		archivedSuggestion := ""
+		if !isArchived {
+			archivedSuggestion = " archived"
+		}
 		_, err := bc.Bot.Send(Recipient(m), fmt.Sprintf("Your transaction list is empty. Create some first. Check /%s for commands to create a transaction."+
-			"\nYou might also be looking for archived transactions using '/list archived'.", CMD_HELP), clearKeyboard())
+			"\nYou might also be looking for%s transactions using '/list%s'.", CMD_HELP, archivedSuggestion, archivedSuggestion), clearKeyboard())
 		if err != nil {
 			bc.Logf(ERROR, m, "Sending bot message failed: %s", err.Error())
 		}

bot/controller_test.go

@@ -165,19 +165,19 @@ func TestTransactionListMaxLength(t *testing.T) {
 		log.Fatal(err)
 	}
 	mock.
-		ExpectQuery(`SELECT "value", "created" FROM "bot::transaction"`).
+		ExpectQuery(`SELECT "id", "value", "created" FROM "bot::transaction"`).
 		WithArgs(chat.ID, false).
-		WillReturnRows(sqlmock.NewRows([]string{"value", "created"}).AddRow(strings.Repeat("**********", 100), "").AddRow(strings.Repeat("**********", 100), "")) // 1000 + 1000
+		WillReturnRows(sqlmock.NewRows([]string{"id", "value", "created"}).AddRow(123, strings.Repeat("**********", 100), "").AddRow(124, strings.Repeat("**********", 100), "")) // 1000 + 1000
 	mock.
-		ExpectQuery(`SELECT "value", "created" FROM "bot::transaction"`).
+		ExpectQuery(`SELECT "id", "value", "created" FROM "bot::transaction"`).
 		WithArgs(chat.ID, false).
-		WillReturnRows(sqlmock.NewRows([]string{"value", "created"}).
+		WillReturnRows(sqlmock.NewRows([]string{"id", "value", "created"}).
 			// 5 * 1000
-			AddRow(strings.Repeat("**********", 100), "").
-			AddRow(strings.Repeat("**********", 100), "").
-			AddRow(strings.Repeat("**********", 100), "").
-			AddRow(strings.Repeat("**********", 100), "").
-			AddRow(strings.Repeat("**********", 100), ""),
+			AddRow(123, strings.Repeat("**********", 100), "").
+			AddRow(124, strings.Repeat("**********", 100), "").
+			AddRow(125, strings.Repeat("**********", 100), "").
+			AddRow(126, strings.Repeat("**********", 100), "").
+			AddRow(127, strings.Repeat("**********", 100), ""),
 		)
 
 	bc := NewBotController(db)
@@ -219,11 +219,11 @@ func TestTransactionsListArchivedDated(t *testing.T) {
 	bc.AddBotAndStart(bot)
 
 	// successful date enrichment
-	mock.ExpectQuery(`SELECT "value", "created" FROM "bot::transaction"`).WithArgs(12345, true).
+	mock.ExpectQuery(`SELECT "id", "value", "created" FROM "bot::transaction"`).WithArgs(12345, true).
 		WillReturnRows(
-			sqlmock.NewRows([]string{"value", "created"}).
-				AddRow("tx1", "2022-03-30T14:24:50.390084Z").
-				AddRow("tx2", "2022-03-30T15:24:50.390084Z"),
+			sqlmock.NewRows([]string{"id", "value", "created"}).
+				AddRow(123, "tx1", "2022-03-30T14:24:50.390084Z").
+				AddRow(124, "tx2", "2022-03-30T15:24:50.390084Z"),
 		)
 	mock.ExpectQuery(`SELECT "value" FROM "bot::userSetting"`).WithArgs(12345, helpers.USERSET_TZOFF).WillReturnRows(mock.NewRows([]string{"value"}))
 
@@ -234,11 +234,11 @@ func TestTransactionsListArchivedDated(t *testing.T) {
 	}
 
 	// fall back to undated if date parsing fails
-	mock.ExpectQuery(`SELECT "value", "created" FROM "bot::transaction"`).WithArgs(12345, true).
+	mock.ExpectQuery(`SELECT "id", "value", "created" FROM "bot::transaction"`).WithArgs(12345, true).
 		WillReturnRows(
-			sqlmock.NewRows([]string{"value", "created"}).
-				AddRow("tx1", "123456789").
-				AddRow("tx2", "456789123"),
+			sqlmock.NewRows([]string{"id", "value", "created"}).
+				AddRow(123, "tx1", "123456789").
+				AddRow(124, "tx2", "456789123"),
 		)
 	mock.ExpectQuery(`SELECT "value" FROM "bot::userSetting"`).WithArgs(12345, helpers.USERSET_TZOFF).WillReturnRows(mock.NewRows([]string{"value"}))
 

db/crud/bot_transaction.go

@@ -18,14 +18,15 @@ func (r *Repo) RecordTransaction(chatId int64, tx string) error {
 }
 
 type TransactionResult struct {
+	Id   int
 	Tx   string
 	Date string
 }
 
 func (r *Repo) GetTransactions(m *tb.Message, isArchived bool) ([]*TransactionResult, error) {
 	LogDbf(r, helpers.TRACE, m, "Getting transactions")
 	rows, err := r.db.Query(`
-		SELECT "value", "created" FROM "bot::transaction"
+		SELECT "id", "value", "created" FROM "bot::transaction"
 		WHERE "tgChatId" = $1 AND "archived" = $2
 		ORDER BY "created" ASC
 	`, m.Chat.ID, isArchived)
@@ -35,14 +36,16 @@ func (r *Repo) GetTransactions(m *tb.Message, isArchived bool) ([]*TransactionRe
 	defer rows.Close()
 
 	allTransactions := []*TransactionResult{}
+	var id int
 	var transactionString string
 	var created string
 	for rows.Next() {
-		err = rows.Scan(&transactionString, &created)
+		err = rows.Scan(&id, &transactionString, &created)
 		if err != nil {
 			return nil, err
 		}
 		allTransactions = append(allTransactions, &TransactionResult{
+			Id:   id,
 			Tx:   transactionString,
 			Date: created,
 		})
@@ -74,3 +77,11 @@ func (r *Repo) DeleteTemplates(m *tb.Message) error {
 		WHERE "tgChatId" = $1`, m.Chat.ID)
 	return err
 }
+
+func (r *Repo) DeleteTransaction(m *tb.Message, isArchived bool, elementId int) error {
+	LogDbf(r, helpers.TRACE, m, "Deleting single transaction")
+	_, err := r.db.Exec(`
+		DELETE FROM "bot::transaction"
+		WHERE "tgChatId" = $1 AND "archived" = $2 AND "id" = $3`, m.Chat.ID, isArchived, elementId)
+	return err
+}

db/crud/bot_transaction_test.go

@@ -25,11 +25,11 @@ func TestRecordGetTransaction(t *testing.T) {
 		t.Errorf("No error should have been returned")
 	}
 
-	mock.ExpectQuery(`SELECT "value", "created" FROM "bot::transaction"`).WithArgs(1122, true).
+	mock.ExpectQuery(`SELECT "id", "value", "created" FROM "bot::transaction"`).WithArgs(1122, true).
 		WillReturnRows(
-			sqlmock.NewRows([]string{"value", "created"}).
-				AddRow("tx1", "2022-03-30 14:24:50.390084").
-				AddRow("tx2", "2022-03-31 14:24:50.390084"),
+			sqlmock.NewRows([]string{"id", "value", "created"}).
+				AddRow(123, "tx1", "2022-03-30 14:24:50.390084").
+				AddRow(124, "tx2", "2022-03-31 14:24:50.390084"),
 		)
 	txs, err := r.GetTransactions(&tb.Message{Chat: &tb.Chat{ID: 1122}}, true)
 	if err != nil {

scenarioTests/features/list.feature

@@ -0,0 +1,70 @@
+Feature: List transactions
+
+  Scenario: List
+    Given I have a bot
+    When I send the message "/deleteAll yes"
+      And I wait 0.2 seconds
+    When I send the message "/list"
+    Then 1 messages should be sent back
+      And the response should include the message "You might also be looking for archived transactions using '/list archived'."
+    When I create a simple tx with amount 1.23 and accFrom someFromAccount and accTo someToAccount and desc Test Tx
+      And I wait 0.1 seconds
+    When I send the message "/list"
+    Then 1 messages should be sent back
+      And the response should include the message "-1.23 EUR"
+    When I send the message "/archiveAll"
+      And I wait 0.2 seconds
+      And I send the message "/list"
+    Then 1 messages should be sent back
+      And the response should include the message "You might also be looking for archived transactions using '/list archived'."
+
+  Scenario: List dated
+    Given I have a bot
+    When I send the message "/deleteAll yes"
+      And I wait 0.2 seconds
+      And I create a simple tx with amount 1.23 and accFrom someFromAccount and accTo someToAccount and desc Test Tx
+      And I wait 0.1 seconds
+    When I send the message "/list dated"
+    Then 1 messages should be sent back
+      And the response should include the message "; recorded on $today "
+
+  Scenario: List archived
+    Given I have a bot
+    When I send the message "/deleteAll yes"
+        And I wait 0.2 seconds
+    When I send the message "/list archived"
+    Then 1 messages should be sent back
+      And the response should include the message "You might also be looking for transactions using '/list'."
+    When I create a simple tx with amount 1.23 and accFrom someFromAccount and accTo someToAccount and desc Test Tx
+      And I wait 0.1 seconds
+    When I send the message "/list archived"
+    Then 1 messages should be sent back
+      And the response should include the message "You might also be looking for transactions using '/list'."
+    When I send the message "/archiveAll"
+      And I wait 0.2 seconds
+      And I send the message "/list archived"
+    Then 1 messages should be sent back
+      And the response should include the message "-1.23 EUR"
+
+  Scenario: List numbered
+    Given I have a bot
+    When I send the message "/deleteAll yes"
+      And I wait 0.2 seconds
+      And I create a simple tx with amount 1.23 and accFrom someFromAccount and accTo someToAccount and desc Test Tx
+      And I wait 0.1 seconds
+      And I create a simple tx with amount 1.23 and accFrom someFromAccount and accTo someToAccount and desc Another tx
+      And I wait 0.1 seconds
+    When I send the message "/list numbered"
+    Then 1 messages should be sent back
+      And the response should include the message "1) $today * "Test Tx""
+      And the same response should include the message "2) $today * "Another tx""
+    When I send the message "/list rm 1"
+    Then 1 messages should be sent back
+      And the response should include the message "Successfully deleted the list entry specified"
+    When I send the message "/list numbered"
+      And I wait 0.1 seconds
+    Then 1 messages should be sent back
+      And the response should include the message "1) $today * "Another tx""
+    When I send the message "/list rm 15"
+    Then 1 messages should be sent back
+      And the response should include the message "the number you specified was too high"

scenarioTests/steps/bot.py

@@ -3,6 +3,7 @@
 from client import TestBot
 import os, asyncio
 import requests
+from datetime import datetime
 
 bot = None
 async def getBotSingletonLazy():
@@ -63,12 +64,22 @@ async def step_impl(context, count):
         print(len(context.responses), "!=", count)
         assert False
 
-@then('the response should include the message "{message}"')
+def replacePlaceholders(msg: str) -> str:
+    # date placeholder '$today'
+    msg = msg.replace("$today", datetime.today().strftime('%Y-%m-%d'))
+    return msg
+
+@then('the{same}response should include the message "{message}"')
 @async_run_until_complete
-async def step_impl(context, message):
-    assert len(context.responses) > 0
-    message = message.strip()
-    response = context.responses.pop(-1) # messages are sorted by creation date from newest to oldest. Take oldest first
+async def step_impl(context, same, message):
+    message = replacePlaceholders(message.strip())
+    assert same in [" ", " same "]
+    if same == " ":
+        assert len(context.responses) > 0
+        response = context.responses.pop(-1) # messages are sorted by creation date from newest to oldest. Take oldest first
+        context.lastResponse = response
+    else:
+        response = context.lastResponse
     try:
         assert message in response.text
     except AssertionError: