Added popup menu in trades history treeview (#139)

CHANGELOG.md

@@ -14,6 +14,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Show application version in About dialog
 - Support for yfinance module to fetch stocks data
 - Support adding trades happened in the past
+- Added popup menu in trades history treeview with option to add and remove trades
 
 ### Fixed
 - Fixed bug where main window was hidden when closing app with unsaved changes

src/Model/DatabaseHandler.py

@@ -87,12 +87,13 @@ def add_trade(self, trade):
             logging.error(e)
             raise RuntimeError("Unable to add trade to the database")
 
-    def remove_last_trade(self):
+    def delete_trade(self, trade_id):
         """
-        Remove the last trade from the trade history
+        Remove the trade from the trade history
         """
         try:
-            del self.trading_history[-1]
+            item = next((t for t in self.trading_history if t.id == trade_id), None)
+            self.trading_history.remove(item)
         except Exception as e:
             logging.error(e)
-            raise RuntimeError("Unable to delete last trade")
+            raise RuntimeError("Unable to delete trade")

src/Model/Portfolio.py

@@ -176,14 +176,25 @@ def get_trade_history(self):
 
     def add_trade(self, new_trade):
         """Add a new trade into the Portfolio"""
-        self._validate_trade(new_trade, self._db_handler.get_trades_list())
+        current_list = self._db_handler.get_trades_list()
+        # Build the list of trades happened before and after the new trade to validate
+        older_trades = [trade for trade in current_list if trade.date < new_trade.date]
+        newer_trades = [trade for trade in current_list if trade.date >= new_trade.date]
+        # Build the new trade list inserting the new trade
+        new_trade_list = older_trades + [new_trade] + newer_trades
+        self._validate_trade_list(new_trade_list)
         self._db_handler.add_trade(new_trade)
         self._load(self._db_handler.get_trades_list())
         self._unsaved_changes = True
 
-    def remove_last_trade(self):
-        """Remove the last trade from the Portfolio"""
-        self._db_handler.remove_last_trade()
+    def delete_trade(self, trade_id):
+        """Remove a trade from the Portfolio"""
+        # Validate the trade list removing the trade
+        new_trade_list = [
+            t for t in self._db_handler.get_trades_list() if t.id != trade_id
+        ]
+        self._validate_trade_list(new_trade_list)
+        self._db_handler.delete_trade(trade_id)
         self._load(self._db_handler.get_trades_list())
         self._unsaved_changes = True
 
@@ -288,17 +299,12 @@ def _compute_avg_holding_open_price(self, symbol, trades_list):
         avg = total_cost / count
         return round(avg, 4)
 
-    def _validate_trade(self, new_trade, trade_list):
+    def _validate_trade_list(self, trade_list):
         """
-        Validate the new Trade request
+        Validate the trade list
         """
-        # Build the list of trades happened before and after the new trade to validate
-        older_trades = [trade for trade in trade_list if trade.date < new_trade.date]
-        newer_trades = [trade for trade in trade_list if trade.date >= new_trade.date]
-        # Build the new trade list inserting the new trade
-        new_trade_list = older_trades + [new_trade] + newer_trades
         # Verify that the new list is valid
-        deposited, available, holdings = self._load_from_trade_list(new_trade_list)
+        deposited, available, holdings = self._load_from_trade_list(trade_list)
 
     def _trade_is_allowed(self, new_trade, cash_available, holdings):
         """

src/TradingMate.py

@@ -96,16 +96,18 @@ def new_trade_event(self, new_trade, portfolio_id):
             if pf.get_id() == portfolio_id:
                 pf.add_trade(new_trade)
 
-    def delete_last_trade_event(self, portfolio_id):
+    def delete_trade_event(self, portfolio_id, trade_id):
         """
-        Callback function to handle delete of last trade request
+        Callback function to handle delete of a trade
         """
         logging.info(
-            "TradingMate - delete last trade for portfolio {}".format(portfolio_id)
+            "TradingMate - delete trade {} for portfolio {}".format(
+                trade_id, portfolio_id
+            )
         )
         for pf in self.portfolios:
             if pf.get_id() == portfolio_id:
-                pf.remove_last_trade()
+                pf.delete_trade(trade_id)
 
     def open_portfolio_event(self, filepath):
         """
@@ -166,6 +168,7 @@ def main():
     tm = TradingMate()
     # Initialise the user interface
     from UI.gtk.UIHandler import UIHandler
+
     UIHandler(tm).start()
 
 

src/UI/TradingMateClient.py

@@ -28,10 +28,6 @@ def new_trade_event(self, new_trade, portfolio_id):
         """Push new trade notification to the server"""
         self._server.new_trade_event(new_trade, portfolio_id)
 
-    def delete_last_trade_event(self, portfolio_id):
-        """Request last trade deletion to the server"""
-        self._server.delete_last_trade_event(portfolio_id)
-
     def manual_refresh_event(self, portfolio_id):
         """Request server to refresh portfolio data"""
         self._server.manual_refresh_event(portfolio_id)
@@ -78,3 +74,6 @@ def get_app_log_filepath(self):
 
     def get_app_version(self):
         return self._server.get_app_version()
+
+    def delete_trade(self, portfolio_id, trade_id):
+        return self._server.delete_trade_event(portfolio_id, trade_id)

src/UI/assets/gtk/notebook_page_layout.glade

@@ -27,6 +27,26 @@
       <column type="gchararray"/>
     </columns>
   </object>
+  <object class="GtkMenu" id="trading_history_menu">
+    <property name="visible">True</property>
+    <property name="can_focus">False</property>
+    <child>
+      <object class="GtkMenuItem" id="add_trade_menu_item">
+        <property name="visible">True</property>
+        <property name="can_focus">False</property>
+        <property name="label" translatable="yes">Add...</property>
+        <property name="use_underline">True</property>
+      </object>
+    </child>
+    <child>
+      <object class="GtkMenuItem" id="delete_trade_menu_item">
+        <property name="visible">True</property>
+        <property name="can_focus">False</property>
+        <property name="label" translatable="yes">Delete...</property>
+        <property name="use_underline">True</property>
+      </object>
+    </child>
+  </object>
   <object class="GtkListStore" id="trading_history_tree_model">
     <columns>
       <!-- column-name Date -->
@@ -45,6 +65,8 @@
       <column type="gchararray"/>
       <!-- column-name Total -->
       <column type="gchararray"/>
+      <!-- column-name Id -->
+      <column type="gchararray"/>
     </columns>
   </object>
   <object class="GtkBox" id="notebook_page_box">

src/UI/gtk/PortfolioPage.py

@@ -10,9 +10,10 @@
 parentdir = os.path.dirname(currentdir)
 sys.path.insert(0, parentdir)
 
-from Utils.Utils import Utils
+from Utils.Utils import Utils, Messages
 from .AddTradeWindow import AddTradeWindow
 from .MessageDialog import MessageDialog
+from .ConfirmDialog import ConfirmDialog
 
 INVALID_STRING = "-"
 
@@ -35,6 +36,10 @@
 BALANCES_PL_PC_VALUE = "balances_pl_pc_value"
 TREE_POSITIONS_MODEL = "positions_tree_model"
 TREE_TRADING_HISTORY_MODEL = "trading_history_tree_model"
+TREE_TRADING_HISTORY = "trading_history_tree"
+TREE_TRADING_HISTORY_MENU = "trading_history_menu"
+TREE_TRADING_HISTORY_ADD_MENU_ITEM = "add_trade_menu_item"
+TREE_TRADING_HISTORY_DELETE_MENU_ITEM = "delete_trade_menu_item"
 
 
 class PortfolioPage(gtk.Box):
@@ -70,19 +75,36 @@ def _load_UI(self, filepath):
         # Get the positions tree model reference
         self._positions_tree_model = builder.get_object(TREE_POSITIONS_MODEL)
         self._history_tree_model = builder.get_object(TREE_TRADING_HISTORY_MODEL)
+        self._history_tree = builder.get_object(TREE_TRADING_HISTORY)
+        # Get the popup menu
+        self._history_menu = builder.get_object(TREE_TRADING_HISTORY_MENU)
+        _history_menu_add_item = builder.get_object(TREE_TRADING_HISTORY_ADD_MENU_ITEM)
+        _history_menu_delete_item = builder.get_object(
+            TREE_TRADING_HISTORY_DELETE_MENU_ITEM
+        )
+        self._history_menu.show_all()
         # Link callbacks to widgets
         save_button.connect("clicked", self._on_save_event)
         save_as_button.connect("clicked", self._on_save_as_event)
         add_button.connect("clicked", self._on_add_event)
         self._refresh_button.connect("clicked", self._on_refresh_event)
         self._refresh_switch.connect("state-set", self._auto_refresh_switch_set_event)
+        self._history_tree.connect(
+            "button_press_event", self._on_trading_history_button_press
+        )
+        _history_menu_add_item.connect("activate", self._on_add_event)
+        _history_menu_delete_item.connect("activate", self._on_delete_event)
         # Set initial status of refresh switch and button based on portfolio status
         self._update_refresh_box()
         # Add the top level container to self
         self.set_hexpand(True)
         self.set_homogeneous(True)
         self.add(top_level)
 
+    def _on_trading_history_button_press(self, widget, event):
+        if event.button == 3:  # Right click
+            self._history_menu.popup(None, None, None, None, event.button, event.time)
+
     def _reset_cache(self):
         self._cache = {"trade_history": []}
 
@@ -125,6 +147,30 @@ def _on_save_as_event(self, widget):
     def _on_add_event(self, widget):
         AddTradeWindow(self._parent_window, self._server, self._id).show()
 
+    def _on_delete_event(self, widget):
+        ConfirmDialog(
+            self._parent_window,
+            Messages.ARE_YOU_SURE.value,
+            self._on_confirmed_delete_trade_event,
+        ).show()
+
+    def _on_confirmed_delete_trade_event(self):
+        try:
+            model, pathlist = self._history_tree.get_selection().get_selected_rows()
+            # Only single selection is supported
+            for path in pathlist:
+                i = model.get_iter(path)
+                trade_id = model.get_value(i, 8)  # hidden column 8
+                self._server.delete_trade(self._id, trade_id)
+                break
+        except Exception as e:
+            MessageDialog(
+                self._parent_window,
+                "Error",
+                Messages.INVALID_OPERATION.value,
+                gtk.MessageType.ERROR,
+            ).show()
+
     def _validate_value(self, value, negative_ok=False):
         if (
             value is None
@@ -184,6 +230,7 @@ def _update_trading_history_treeview(self, trade_list):
                         self._validate_value(t.fee),
                         self._validate_value(t.sdr),
                         self._validate_value(t.total, negative_ok=True),
+                        t.id,
                     ]
                 )
 

src/Utils/Utils.py

@@ -31,6 +31,7 @@ class Messages(Enum):
     UNSAVED_CHANGES = "There are unsaved changes, are you sure?"
     ERROR_SAVE_SETTINGS = "Unable to save the settings"
     WINDOW_UNSUPPORTED_ACTION = "This window does not support the selected action"
+    ARE_YOU_SURE = "Are you sure?"
 
 
 class Markets(Enum):

test/test_db_handler.py

@@ -87,13 +87,13 @@ def test_add_trade(dbh):
     assert len(dbh.trading_history) == prev_len + 1
 
 
-def test_remove_last_trade(dbh):
+def test_delete_trade(dbh):
     """
-    Test it removes the last trade from the in memory list
+    Test it removes the trade from the in memory list
     """
     prev_len = len(dbh.trading_history)
     item = {
-        "id": "0",
+        "id": "42",
         "date": "01/01/0001",
         "action": "BUY",
         "quantity": 1,
@@ -106,7 +106,7 @@ def test_remove_last_trade(dbh):
     trade = Trade.from_dict(item)
     dbh.add_trade(trade)
     assert len(dbh.trading_history) == prev_len + 1
-    dbh.remove_last_trade()
+    dbh.delete_trade("42")
     assert len(dbh.trading_history) == prev_len