Hotfix/routine default file overwrite (#4927)

* bump pywry

* fix windows pyinstaller subprocess fail if space in path

* remove API_IEX_TOKEN

* remove API_SENTIMENTINVESTOR_TOKEN

* added logic to download_routines and save_routine to put default and personal routines in difference folders and changed logic in call_dowload in account controller to account for the change in logic for save_routine

* Changed logic in read_routine function to account for the new directory structure and also changed the corresponding unit tests. Slightly amended test_save_routine to account for the new routine param

* Changed the routine parameter of mock_save_routine.assert_called_once_with to account for the change in the input parameter of the save_routine function

* changed routine string in test_routines_handler

* Made unit tests for download_routines in routines_handler.

* added comments

* Added new logic to call_exe so that a user could execute the file with --file default/filename.openbb, --file personal/filename.openbb or --file Users/Username/openBBUserData/Routines/hub/default/filename.openbb

* linting: black

* linting: ruff

* ruff broke black

* removed regex in call_exe and changed logic in call_download to give personal routines priority

* Changed formatting

* Changed formatting and removed regex from call_exe

* Created class variables and moved dictionaries out of constructor to solve linting issue.
Changed routine args type annotation
Changed data list

* Added type annotations for the dictionary class variables

* Changed data annotation type for routine dictionaries

* Moved dictionaries back into constructor

* removed import pathlib

* ruff

* mypy

* pylint

---------

Co-authored-by: James Maslek <jmaslek11@gmail.com>
Co-authored-by: teh_coderer <me@tehcoderer.com>
Co-authored-by: Diogo Sousa <montezdesousa@gmail.com>
Co-authored-by: montezdesousa <79287829+montezdesousa@users.noreply.github.com>

6 files changed, 321 insertions, 79 deletions

diff --git a/openbb_terminal/account/account_controller.py b/openbb_terminal/account/account_controller.py
index f5fd75f952f..b788e6f9aac 100644
--- a/openbb_terminal/account/account_controller.py
+++ b/openbb_terminal/account/account_controller.py
@@ -371,6 +371,8 @@ class AccountController(BaseController):
                         self.REMOTE_CHOICES.append(name)
                         self.update_runtime_choices()
 
+    # store data in list with "personal/default" to identify data's routine type
+    # and for save_routine
     @log_start_end(log=logger)
     def call_download(self, other_args: List[str]):
         """Download"""
@@ -398,37 +400,36 @@ class AccountController(BaseController):
             print_guest_block_msg()
         else:
             if ns_parser:
-                data = None
-
-                # Default routine
+                data = []
                 name = " ".join(ns_parser.name)
-                if name in self.DEFAULT_CHOICES:
-                    data = next(
-                        (r for r in self.DEFAULT_ROUTINES if r["name"] == name), None
-                    )
-                else:
-                    # User routine
-                    response = Hub.download_routine(
-                        auth_header=get_current_user().profile.get_auth_header(),
-                        name=name,
-                    )
-                    data = (
-                        response.json()
-                        if response and response.status_code == 200
-                        else None
-                    )
+                # Personal routines
+                response = Hub.download_routine(
+                    auth_header=get_current_user().profile.get_auth_header(),
+                    name=name,
+                )
+                if response and response.status_code == 200:
+                    data = [response.json(), "personal"]
+                # Default routine
+                elif name in self.DEFAULT_CHOICES:
+                    data = [
+                        next(
+                            (r for r in self.DEFAULT_ROUTINES if r["name"] == name),
+                            None,
+                        ),
+                        "default",
+                    ]
 
                 # Save routine
-                if data:
-                    name = data.get("name", "")
+                if data[0]:
+                    name = data[0].get("name", "")
                     if name:
                         console.print(f"[info]Name:[/info] {name}")
 
-                    description = data.get("description", "")
+                    description = data[0].get("description", "")
                     if description:
                         console.print(f"[info]Description:[/info] {description}")
 
-                    script = data.get("script", "")
+                    script = [data[0].get("script", ""), data[1]]
                     if script:
                         file_name = f"{name}.openbb"
                         file_path = save_routine(
diff --git a/openbb_terminal/core/session/routines_handler.py b/openbb_terminal/core/session/routines_handler.py
index 8a4f0a931b2..bef93b1e747 100644
--- a/openbb_terminal/core/session/routines_handler.py
+++ b/openbb_terminal/core/session/routines_handler.py
@@ -1,4 +1,5 @@
 import os
+from os import walk
 from pathlib import Path
 from typing import Dict, List, Optional, Tuple, Union
 
@@ -10,7 +11,10 @@ from openbb_terminal.core.session.current_user import get_current_user
 from openbb_terminal.rich_config import console
 
 
-def download_routines(auth_header: str, silent: bool = False) -> Dict[str, str]:
+# created dictionaries for personal and default routines with the structure
+# {"file_name" :["script","personal/default"]}
+# and stored dictionaries in list
+def download_routines(auth_header: str, silent: bool = False) -> list:
     """Download default and personal routines.
 
     Parameters
@@ -25,7 +29,8 @@ def download_routines(auth_header: str, silent: bool = False) -> Dict[str, str]:
     Dict[str, str]
         The routines.
     """
-    routines_dict = {}
+    personal_routines_dict = {}
+    default_routines_dict = {}
 
     try:
         response = Hub.get_default_routines(silent=silent)
@@ -35,7 +40,7 @@ def download_routines(auth_header: str, silent: bool = False) -> Dict[str, str]:
             for routine in data:
                 name = routine.get("name", "")
                 if name:
-                    routines_dict[name] = routine.get("script", "")
+                    default_routines_dict[name] = [routine.get("script", ""), "default"]
     except Exception:
         console.print("[red]\nFailed to download default routines.[/red]")
 
@@ -54,13 +59,17 @@ def download_routines(auth_header: str, silent: bool = False) -> Dict[str, str]:
             for routine in items:
                 name = routine.get("name", "")
                 if name:
-                    routines_dict[name] = routine.get("script", "")
+                    personal_routines_dict[name] = [
+                        routine.get("script", ""),
+                        "personal",
+                    ]
     except Exception:
         console.print("[red]\nFailed to download personal routines.[/red]")
 
-    return routines_dict
+    return [personal_routines_dict, default_routines_dict]
 
 
+# use os.walk to search subdirectories and then construct file path
 def read_routine(file_name: str, folder: Optional[Path] = None) -> Optional[str]:
     """Read the routine.
 
@@ -85,12 +94,12 @@ def read_routine(file_name: str, folder: Optional[Path] = None) -> Optional[str]
 
     try:
         user_folder = folder / "hub"
-        file_path = (
-            user_folder / file_name
-            if os.path.exists(user_folder / file_name)
-            else folder / file_name
-        )
-
+        for path, _, files in walk(user_folder):
+            file_path = (
+                folder / os.path.relpath(path, folder) / file_name
+                if file_name in files
+                else folder / file_name
+            )
         with open(file_path) as f:
             routine = "".join(f.readlines())
         return routine
@@ -99,9 +108,10 @@ def read_routine(file_name: str, folder: Optional[Path] = None) -> Optional[str]
         return None
 
 
+# created new directory structure to account for personal and default routines
 def save_routine(
     file_name: str,
-    routine: str,
+    routine: list,
     folder: Optional[Path] = None,
     force: bool = False,
     silent: bool = False,
@@ -134,6 +144,11 @@ def save_routine(
 
     try:
         user_folder = folder / "hub"
+        if routine[1] == "default":
+            user_folder = folder / "hub" / "default"
+        elif routine[1] == "personal":
+            user_folder = folder / "hub" / "personal"
+
         if not os.path.exists(user_folder):
             os.makedirs(user_folder)
 
@@ -141,7 +156,7 @@ def save_routine(
         if os.path.exists(file_path) and not force:
             return "File already exists"
         with open(file_path, "w") as f:
-            f.write(routine)
+            f.write(routine[0])
         return user_folder / file_name
     except Exception:
         console_print("[red]\nFailed to save routine.[/red]")
diff --git a/openbb_terminal/core/session/session_model.py b/openbb_terminal/core/session/session_model.py
index f767a8ef20f..1731cb96752 100644
--- a/openbb_terminal/core/session/session_model.py
+++ b/openbb_terminal/core/session/session_model.py
@@ -133,8 +133,12 @@ def download_and_save_routines(auth_header: str):
         The authorization header, e.g. "Bearer <token>".
     """
     routines = download_routines(auth_header=auth_header)
+    personal_routines_dict = routines[0]
+    default_routines_dict = routines[1]
     try:
-        for name, content in routines.items():
+        for name, content in personal_routines_dict.items():
+            save_routine(file_name=f"{name}.openbb", routine=content, force=True)
+        for name, content in default_routines_dict.items():
             save_routine(file_name=f"{name}.openbb", routine=content, force=True)
     except Exception:
         console.print("[red]\nFailed to save routines.[/red]")
diff --git a/openbb_terminal/terminal_controller.py b/openbb_terminal/terminal_controller.py
index 50133a56599..483dd89e7cf 100644
--- a/openbb_terminal/terminal_controller.py
+++ b/openbb_terminal/terminal_controller.py
@@ -13,7 +13,7 @@ import time
 import webbrowser
 from datetime import datetime
 from pathlib import Path
-from typing import Dict, List, Optional
+from typing import Any, Dict, List, Optional
 
 import certifi
 import pandas as pd
@@ -126,6 +126,11 @@ class TerminalController(BaseController):
 
     def __init__(self, jobs_cmds: Optional[List[str]] = None):
         """Construct terminal controller."""
+        self.ROUTINE_FILES: Dict[str, str] = dict()
+        self.ROUTINE_DEFAULT_FILES: Dict[str, str] = dict()
+        self.ROUTINE_PERSONAL_FILES: Dict[str, str] = dict()
+        self.ROUTINE_CHOICES: Dict[str, Any] = dict()
+
         super().__init__(jobs_cmds)
 
         self.queue: List[str] = list()
@@ -147,8 +152,24 @@ class TerminalController(BaseController):
                 "*.openbb"
             )
         }
+        if get_current_user().profile.get_token():
+            self.ROUTINE_DEFAULT_FILES = {
+                filepath.name: filepath
+                for filepath in Path(
+                    get_current_user().preferences.USER_ROUTINES_DIRECTORY
+                    / "hub"
+                    / "default"
+                ).rglob("*.openbb")
+            }
+            self.ROUTINE_PERSONAL_FILES = {
+                filepath.name: filepath
+                for filepath in Path(
+                    get_current_user().preferences.USER_ROUTINES_DIRECTORY
+                    / "hub"
+                    / "personal"
+                ).rglob("*.openbb")
+            }
 
-        self.ROUTINE_CHOICES = {}
         self.ROUTINE_CHOICES["--file"] = {
             filename: None for filename in self.ROUTINE_FILES
         }
@@ -564,19 +585,34 @@ class TerminalController(BaseController):
 
         if ns_parser:
             if ns_parser.example:
-                path = MISCELLANEOUS_DIRECTORY / "routines" / "routine_example.openbb"
+                routine_path = (
+                    MISCELLANEOUS_DIRECTORY / "routines" / "routine_example.openbb"
+                )
                 console.print(
                     "[info]Executing an example, please type `about exe` "
                     "to learn how to create your own script.[/info]\n"
                 )
                 time.sleep(3)
             elif ns_parser.file:
+                # if string is not in this format "default/file.openbb" then check for files in ROUTINE_FILES
                 file_path = " ".join(ns_parser.file)
-                path = self.ROUTINE_FILES.get(file_path, Path(file_path))
+                full_path = file_path
+                hub_routine = file_path.split("/")
+                if hub_routine[0] == "default":
+                    routine_path = Path(
+                        self.ROUTINE_DEFAULT_FILES.get(hub_routine[1], full_path)
+                    )
+                elif hub_routine[0] == "personal":
+                    routine_path = Path(
+                        self.ROUTINE_PERSONAL_FILES.get(hub_routine[1], full_path)
+                    )
+                else:
+                    routine_path = Path(self.ROUTINE_FILES.get(file_path, full_path))
+
             else:
                 return
 
-            with open(path) as fp:
+            with open(routine_path) as fp:
                 raw_lines = [
                     x for x in fp if (not is_reset(x)) and ("#" not in x) and x
                 ]
@@ -955,7 +991,7 @@ def replace_dynamic(match: re.Match, special_arguments: Dict[str, str]) -> str:
     return default
 
 
-def run_routine(file: str, routines_args=List[str]):
+def run_routine(file: str, routines_args=Optional[str]):
     """Execute command routine from .openbb file."""
     user_routine_path = (
         get_current_user().preferences.USER_DATA_DIRECTORY / "routines" / file
diff --git a/tests/openbb_terminal/account/test_account_controller.py b/tests/openbb_terminal/account/test_account_controller.py
index 8fbc384e12c..a3495117fdd 100644
--- a/tests/openbb_terminal/account/test_account_controller.py
+++ b/tests/openbb_terminal/account/test_account_controller.py
@@ -483,8 +483,7 @@ def test_call_download(mocker, test_user):
         name="script1",
     )
     mock_save_routine.assert_called_once_with(
-        file_name="script1.openbb",
-        routine="do something",
+        file_name="script1.openbb", routine=["do something", "personal"]
     )
 
 
diff --git a/tests/openbb_terminal/session/test_routines_handler.py b/tests/openbb_terminal/session/test_routines_handler.py
index 8ea78b16ebe..566c275fd10 100644
--- a/tests/openbb_terminal/session/test_routines_handler.py
+++ b/tests/openbb_terminal/session/test_routines_handler.py
@@ -6,6 +6,7 @@ import json
 from unittest.mock import mock_open
 
 import pytest
+from requests import Response
 
 # IMPORTATION INTERNAL
 from openbb_terminal.core.models.user_model import (
@@ -16,7 +17,11 @@ from openbb_terminal.core.models.user_model import (
     UserModel,
 )
 from openbb_terminal.core.session.current_user import get_current_user
-from openbb_terminal.core.session.routines_handler import read_routine, save_routine
+from openbb_terminal.core.session.routines_handler import (
+    download_routines,
+    read_routine,
+    save_routine,
+)
 
 
 @pytest.fixture(name="test_user")
@@ -29,14 +34,7 @@ def fixture_test_user():
     )
 
 
-@pytest.mark.parametrize(
-    "exists",
-    [
-        False,
-        True,
-    ],
-)
-def test_read_routine(mocker, exists: bool, test_user):
+def test_read_routine(mocker, test_user):
     file_name = "test_routine.openbb"
     routine = "do something"
     current_user = get_current_user()
@@ -46,50 +44,76 @@ def test_read_routine(mocker, exists: bool, test_user):
         target=path + ".get_current_user",
         return_value=test_user,
     )
-
-    exists_mock = mocker.patch(path + ".os.path.exists", return_value=exists)
-    open_mock = mocker.patch(
-        path + ".open",
-        mock_open(read_data=json.dumps(routine)),
+    walk_mock = mocker.patch(
+        path + ".walk",
+        return_value=[
+            (
+                current_user.preferences.USER_ROUTINES_DIRECTORY / "hub",
+                ["personal"],
+                [],
+            ),
+            (
+                current_user.preferences.USER_ROUTINES_DIRECTORY / "hub" / "personal",
+                [],
+                [file_name],
+            ),
+        ],
     )
-
+    relpath_mock = mocker.patch(path + ".os.path.relpath", return_value="hub/personal")
+    open_mock = mocker.patch(path + ".open", mock_open(read_data=json.dumps(routine)))
     assert read_routine(file_name=file_name) == json.dumps(routine)
 
-    exists_mock.assert_called_with(
-        current_user.preferences.USER_ROUTINES_DIRECTORY / "hub" / file_name
+    walk_mock.assert_called_with(
+        current_user.preferences.USER_ROUTINES_DIRECTORY / "hub"
+    )
+    relpath_mock.assert_called_once_with(
+        current_user.preferences.USER_ROUTINES_DIRECTORY / "hub" / "personal",
+        current_user.preferences.USER_ROUTINES_DIRECTORY,
+    )
+    open_mock.assert_called_with(
+        current_user.preferences.USER_ROUTINES_DIRECTORY
+        / "hub"
+        / "personal"
+        / file_name
     )
-    if exists:
-        open_mock.assert_called_with(
-            current_user.preferences.USER_ROUTINES_DIRECTORY / "hub" / file_name
-        )
-    else:
-        open_mock.assert_called_with(
-            current_user.preferences.USER_ROUTINES_DIRECTORY / file_name
-        )
 
 
 def test_read_routine_exception(mocker, test_user):
     file_name = "test_routine.openbb"
     current_user = get_current_user()
     path = "openbb_terminal.core.session.routines_handler"
-
     mocker.patch(
         target=path + ".get_current_user",
         return_value=test_user,
     )
-    exists_mock = mocker.patch(path + ".os.path.exists")
+    walk_mock = mocker.patch(
+        path + ".walk",
+        return_value=[
+            (
+                current_user.preferences.USER_ROUTINES_DIRECTORY / "hub",
+                ["personal"],
+                [],
+            ),
+            (
+                current_user.preferences.USER_ROUTINES_DIRECTORY / "hub" / "personal",
+                [],
+                ["do something"],
+            ),
+        ],
+    )
+    relpath_mock = mocker.patch(path + ".os.path.relpath")
     open_mock = mocker.patch(
         path + ".open",
         side_effect=Exception("test exception"),
     )
-
     assert read_routine(file_name=file_name) is None
 
-    exists_mock.assert_called_with(
-        current_user.preferences.USER_ROUTINES_DIRECTORY / "hub" / file_name
+    walk_mock.assert_called_with(
+        current_user.preferences.USER_ROUTINES_DIRECTORY / "hub"
     )
+    relpath_mock.assert_not_called()
     open_mock.assert_called_with(
-        current_user.preferences.USER_ROUTINES_DIRECTORY / "hub" / file_name
+        current_user.preferences.USER_ROUTINES_DIRECTORY / file_name
     )
 
 
@@ -102,7 +126,7 @@ def test_read_routine_exception(mocker, test_user):
 )
 def test_save_routine(mocker, exists: bool, test_user):
     file_name = "test_routine.openbb"
-    routine = "do something"
+    routine = ["do something", "personal"]
     current_user = get_current_user()
     path = "openbb_terminal.core.session.routines_handler"
 
@@ -125,11 +149,18 @@ def test_save_routine(mocker, exists: bool, test_user):
     else:
         assert (
             result
-            == current_user.preferences.USER_ROUTINES_DIRECTORY / "hub" / file_name
+            == current_user.preferences.USER_ROUTINES_DIRECTORY
+            / "hub"
+            / "personal"
+            / file_name
         )
         makedirs_mock.assert_called_once()
         open_mock.assert_called_with(
-            current_user.preferences.USER_ROUTINES_DIRECTORY / "hub" / file_name, "w"
+            current_user.preferences.USER_ROUTINES_DIRECTORY
+            / "hub"
+            / "personal"
+            / file_name,
+            "w",
         )
 
     assert exists_mock.call_count == 2
@@ -137,7 +168,7 @@ def test_save_routine(mocker, exists: bool, test_user):
 
 def test_save_routine_exception(mocker, test_user):
     file_name = "test_routine.openbb"
-    routine = "do something"
+    routine = ["do something", "personal"]
     path = "openbb_terminal.core.session.routines_handler"
 
     mocker.patch(
@@ -154,3 +185,159 @@ def test_save_routine_exception(mocker, test_user):
 
     result = save_routine(file_name=file_name, routine=routine)
     assert result is None
+
+
+def test_download_routine(mocker, test_user, silent=False):
+    path_hub_model = "openbb_terminal.core.session.hub_model"
+    path_routines_handler = "openbb_terminal.core.session.routines_handler"
+    mocker.patch(
+        target=path_routines_handler + ".get_current_user",
+        return_value=test_user,
+    )
+    response_default = Response()
+    response_default.status_code = 200
+    content = {
+        "data": [{"name": "script1", "description": "abc", "script": "do something"}]
+    }
+    response_default._content = json.dumps(
+        content
+    ).encode(  # pylint: disable=protected-access
+        "utf-8"
+    )
+    # print(response_default._content)
+
+    response_personal = Response()
+    response_personal.status_code = 200
+    content = {
+        "items": [{"name": "script2", "description": "cde", "script": "do something"}]
+    }
+
+    response_personal._content = json.dumps(
+        content
+    ).encode(  # pylint: disable=protected-access
+        "utf-8"
+    )
+    get_default_routines_mock = mocker.patch(
+        target=path_hub_model + ".get_default_routines", return_value=response_default
+    )
+    get_personal_routines_mock = mocker.patch(
+        target=path_hub_model + ".list_routines", return_value=response_personal
+    )
+    assert download_routines(test_user.profile.get_auth_header()) == [
+        {"script2": ["do something", "personal"]},
+        {"script1": ["do something", "default"]},
+    ]
+
+    get_default_routines_mock.assert_called_once()
+    get_personal_routines_mock.assert_called_once_with(
+        auth_header=test_user.profile.get_auth_header(),
+        fields=["name", "script"],
+        page=1,
+        size=100,
+        silent=silent,
+    )
+
+
+def test_download_default_routine_exception(mocker, test_user, silent=False):
+    path_hub_model = "openbb_terminal.core.session.hub_model"
+    path_routines_handler = "openbb_terminal.core.session.routines_handler"
+    mocker.patch(
+        target=path_routines_handler + ".get_current_user",
+        return_value=test_user,
+    )
+    response_personal = Response()
+    response_personal.status_code = 200
+    content = {
+        "items": [{"name": "script2", "description": "cde", "script": "do something"}]
+    }
+    response_personal._content = json.dumps(
+        content
+    ).encode(  # pylint: disable=protected-access
+        "utf-8"
+    )
+    get_personal_routines_mock = mocker.patch(
+        target=path_hub_model + ".list_routines", return_value=response_personal
+    )
+    get_default_routines_mock = mocker.patch(
+        path_hub_model + ".get_default_routines",
+        side_effect=Exception("test exception"),
+    )
+    assert download_routines(test_user.profile.get_auth_header()) == [
+        {"script2": ["do something", "personal"]},
+        {},
+    ]
+    get_default_routines_mock.assert_called_once()
+    get_personal_routines_mock.assert_called_once_with(
+        auth_header=test_user.profile.get_auth_header(),
+        fields=["name", "script"],
+        page=1,
+        size=100,
+        silent=silent,
+    )
+
+
+def test_download_personal_routine_exception(mocker, test_user, silent=False):
+    path_hub_model = "openbb_terminal.core.session.hub_model"
+    path_routines_handler = "openbb_terminal.core.session.routines_handler"
+    mocker.patch(
+        target=path_routines_handler + ".get_current_user",
+        return_value=test_user,
+    )
+    response_default = Response()
+    response_default.status_code = 200
+    content = {
+        "data": [{"name": "script1", "description": "abc", "script": "do something"}]
+    }
+    response_default._content = json.dumps(
+        content
+    ).encode(  # pylint: disable=protected-access
+        "utf-8"
+    )
+    get_default_routines_mock = mocker.patch(
+        target=path_hub_model + ".get_default_routines", return_value=response_default
+    )
+    get_personal_routines_mock = mocker.patch(
+        path_hub_model + ".list_routines",
+        side_effect=Exception("test exception"),
+    )
+    assert download_routines(test_user.profile.get_auth_header()) == [
+        {},
+        {"script1": ["do something", "default"]},
+    ]
+    get_default_routines_mock.assert_called_once()
+    get_personal_routines_mock.assert_called_once_with(
+        auth_header=test_user.profile.get_auth_header(),
+        fields=["name", "script"],
+        page=1,
+        size=100,
+        silent=silent,
+    )
+
+
+def test_download_default_and_personal_routine_exception(
+    mocker, test_user, silent=False
+):
+    path_hub_model = "openbb_terminal.core.session.hub_model"
+    path_routines_handler = "openbb_terminal.core.session.routines_handler"
+    mocker.patch(
+        target=path_routines_handler + ".get_current_user",
+        return_value=test_user,
+    )
+    get_default_routines_mock = mocker.patch(
+        path_hub_model + ".get_default_routines",
+        side_effect=Exception("test exception"),
+    )
+
+    get_personal_routines_mock = mocker.patch(
+        path_hub_model + ".list_routines",
+        side_effect=Exception("test exception"),
+    )
+    assert download_routines(test_user.profile.get_auth_header()) == [{}, {}]
+    get_default_routines_mock.assert_called_once()
+    get_personal_routines_mock.assert_called_once_with(
+        auth_header=test_user.profile.get_auth_header(),
+        fields=["name", "script"],
+        page=1,
+        size=100,
+        silent=silent,
+    )