From 962069280857737f3aa8228ee27b5b86dd5b1616 Mon Sep 17 00:00:00 2001
From: doug-szeto-slalom <douglas.szeto@slalom.com>
Date: Mon, 3 Jun 2024 11:48:41 -0400
Subject: [PATCH] feat: Feature/secureli 494 refactor action.py (#556)

secureli-494


## Changes
<!-- A detailed list of changes -->
* creates `_update_config` helper function to be used in
`verify_install`
* creates `_pre_install_checks` helper function to be used in
`_install_secureli`

## Testing
* no additional tests, existing tests pass

## Clean Code Checklist
<!-- This is here to support you. Some/most checkboxes may not apply to
your change -->
- [x] Meets acceptance criteria for issue
- [x] New logic is covered with automated tests
- [x] Appropriate exception handling added
- [x] Thoughtful logging included
- [x] Documentation is updated
- [x] Follow-up work is documented in TODOs
- [x] TODOs have a ticket associated with them
- [x] No commented-out code included


<!--
Github-flavored markdown reference:
https://docs.github.com/en/get-started/writing-on-github
-->
---
 secureli/actions/action.py | 72 ++++++++++++++++++++++++++++----------
 1 file changed, 53 insertions(+), 19 deletions(-)

diff --git a/secureli/actions/action.py b/secureli/actions/action.py
index 49dd01c7..a96fef3c 100644
--- a/secureli/actions/action.py
+++ b/secureli/actions/action.py
@@ -78,16 +78,17 @@ def verify_install(
         :param files: A List of files to scope the install to. This allows language
         detection to run on only a selected list of files when scanning the repo.
         """
-        if (
+
+        is_config_out_of_date = (
             self.action_deps.secureli_config.verify()
             == ConfigModels.VerifyConfigOutcome.OUT_OF_DATE
-        ):
-            update_config = self._update_secureli_config_only(always_yes)
-            if update_config.outcome != install.VerifyOutcome.UPDATE_SUCCEEDED:
-                self.action_deps.echo.error("seCureLI could not be verified.")
-                return install.VerifyResult(
-                    outcome=update_config.outcome,
-                )
+        )
+        if is_config_out_of_date:
+            update_result = self._update_config(always_yes)
+            did_update_fail = update_result is not None
+            if did_update_fail:
+                return update_result
+
         pre_commit_config_location_is_correct = self.action_deps.hooks_scanner.pre_commit.get_pre_commit_config_path_is_correct(
             folder_path
         )
@@ -175,6 +176,18 @@ def verify_install(
                 config=config,
             )
 
+    def _update_config(self, always_yes: bool) -> install.VerifyResult:
+        """
+        Updates the secureli config
+        :param always_yes: Assume "Yes" to all prompts
+        """
+        update_config = self._update_secureli_config_only(always_yes)
+        if update_config.outcome != install.VerifyOutcome.UPDATE_SUCCEEDED:
+            self.action_deps.echo.error("seCureLI could not be verified.")
+            return install.VerifyResult(
+                outcome=update_config.outcome,
+            )
+
     def _install_secureli(
         self,
         folder_path: Path,
@@ -194,18 +207,13 @@ def _install_secureli(
 
         # pre-install
         new_install = len(detected_languages) == len(install_languages)
-        should_install = self._prompt_to_install(
-            install_languages, always_yes, new_install
-        )
-        if not should_install:
-            if new_install:
-                self.action_deps.echo.error("User canceled install process")
-                return install.VerifyResult(
-                    outcome=install.VerifyOutcome.INSTALL_CANCELED,
-                )
 
-            self.action_deps.echo.warning("Newly detected languages were not installed")
-            return install.VerifyResult(outcome=install.VerifyOutcome.UP_TO_DATE)
+        pre_install_result = self._pre_install_checks(
+            new_install, install_languages, always_yes
+        )
+        did_pre_install_fail = pre_install_result is not None
+        if did_pre_install_fail:
+            return pre_install_result
 
         settings = self.action_deps.settings.load(folder_path)
 
@@ -254,6 +262,32 @@ def _install_secureli(
             config=config,
         )
 
+    def _pre_install_checks(
+        self,
+        new_install: bool,
+        install_languages: list[str],
+        always_yes: bool,
+    ) -> install.VerifyResult:
+        """
+        Checks if secureli should not be installed due to user cancelllation or failure to install newly detected languages
+        :param new_install: boolean flag to determine if this is a new install
+        :param install_languages: list of specific langugages to install secureli features for
+        :param always_yes: Assume "Yes" to all prompts
+        :return: None or an install.VerifyResult if secureli should not be insalled
+        """
+        should_install = self._prompt_to_install(
+            install_languages, always_yes, new_install
+        )
+        if not should_install:
+            if new_install:
+                self.action_deps.echo.error("User canceled install process")
+                return install.VerifyResult(
+                    outcome=install.VerifyOutcome.INSTALL_CANCELED,
+                )
+
+            self.action_deps.echo.warning("Newly detected languages were not installed")
+            return install.VerifyResult(outcome=install.VerifyOutcome.UP_TO_DATE)
+
     def _prompt_to_install(
         self, languages: list[str], always_yes: bool, new_install: bool
     ) -> bool: