From 3ce70c8033e6e4e1c7d51a144f49f94207eb4cf5 Mon Sep 17 00:00:00 2001
From: Michael Woolnough <130465766+mjkw31@users.noreply.github.com>
Date: Fri, 15 Nov 2024 10:07:42 +0000
Subject: [PATCH] Fix incorrect email being sent out of build complete/fail.
 (#70)

* The builder uploads the build.log on a success as well as failure, so we shouldn't break after reading the build log in case there's also a module file to be noticed (indicating a success, instead of a failure).

* Fix cache updating.

* Add 'SoftPack' to email subject.
---
 softpack_core/schemas/environment.py  | 37 +++++++++++++--------------
 softpack_core/service.py              |  6 ++---
 tests/integration/test_environment.py | 12 ++++++---
 3 files changed, 29 insertions(+), 26 deletions(-)

diff --git a/softpack_core/schemas/environment.py b/softpack_core/schemas/environment.py
index c6974b4..48f662d 100644
--- a/softpack_core/schemas/environment.py
+++ b/softpack_core/schemas/environment.py
@@ -481,11 +481,7 @@ def create(cls, env: EnvironmentInput) -> CreateResponse:  # type: ignore
         if not isinstance(response, CreateEnvironmentSuccess):
             return response
 
-        environment = Environment.get_env(Path(env.path), env.name)
-        if environment is not None:
-            cls.insert_new_env(environment)
-        else:
-            return EnvironmentNotFoundError(path=env.path, name=env.name)
+        Environment.update_cache(Path(env.path, env.name))
 
         builder_response = cls.submit_env_to_builder(env)
         if builder_response is not None:
@@ -779,9 +775,7 @@ def delete(cls, name: str, path: str) -> DeleteResponse:  # type: ignore
             tree_oid = artifacts.delete_environment(name, path)
             artifacts.commit_and_push(tree_oid, "delete environment")
 
-            index = cls.env_index_from_path(str(Path(path, name)))
-            if index is not None:
-                del Environment.environments[index]
+            Environment.update_cache(Path(path, name))
 
             return DeleteEnvironmentSuccess(
                 message="Successfully deleted the environment"
@@ -958,17 +952,7 @@ async def write_artifacts(
             )
             artifacts.commit_and_push(tree_oid, commitMsg)
 
-            index = cls.env_index_from_path(str(folder_path))
-            path = Path(folder_path)
-            env = Environment.get_env(path.parent, path.name)
-
-            if index is None:
-                if env:
-                    Environment.insert_new_env(env)
-            elif env:
-                Environment.environments[index] = env
-            else:
-                del Environment.environments[index]
+            Environment.update_cache(folder_path)
 
             return WriteArtifactSuccess(
                 message="Successfully written artifact(s)",
@@ -978,6 +962,21 @@ async def write_artifacts(
                 error="".join(format_exception_only(type(e), e))
             )
 
+    @classmethod
+    def update_cache(cls, folder_path: str | Path) -> None:
+        """Regenerate the cached environment specified by the path."""
+        index = cls.env_index_from_path(str(folder_path))
+        path = Path(folder_path)
+        env = Environment.get_env(path.parent, path.name)
+
+        if index is None:
+            if env:
+                Environment.insert_new_env(env)
+        elif env:
+            Environment.environments[index] = env
+        else:
+            del Environment.environments[index]
+
     @classmethod
     def env_index_from_path(cls, folder_path: str) -> Optional[int]:
         """Return the index of a folder_path from the list of environments."""
diff --git a/softpack_core/service.py b/softpack_core/service.py
index 503a65d..bc22e68 100644
--- a/softpack_core/service.py
+++ b/softpack_core/service.py
@@ -144,8 +144,6 @@ async def upload_artifacts(  # type: ignore[no-untyped-def]
 
                     files[i] = (f.filename, contents)
 
-                    break
-
                 if f.filename == artifacts.module_file:
                     newState = State.ready
 
@@ -183,9 +181,9 @@ async def upload_artifacts(  # type: ignore[no-untyped-def]
                 )
 
                 subject = (
-                    "Your environment is ready!"
+                    "Your SoftPack environment is ready!"
                     if newState == State.ready
-                    else "Your environment failed to build"
+                    else "Your SoftPack environment failed to build"
                 )
 
                 send_email(
diff --git a/tests/integration/test_environment.py b/tests/integration/test_environment.py
index b09d526..228c75a 100644
--- a/tests/integration/test_environment.py
+++ b/tests/integration/test_environment.py
@@ -401,7 +401,7 @@ async def test_email_on_build_complete(
     assert send_email.call_args[0][0] == app.settings.environments
     assert "built sucessfully" in send_email.call_args[0][1]
     assert "The error was" not in send_email.call_args[0][1]
-    assert send_email.call_args[0][2] == "Your environment is ready!"
+    assert send_email.call_args[0][2] == "Your SoftPack environment is ready!"
     assert send_email.call_args[0][3] == "me"
 
     client = TestClient(app.router)
@@ -441,7 +441,10 @@ async def test_email_on_build_complete(
         "The error was a build error. Contact your softpack administrator."
         in send_email.call_args[0][1]
     )
-    assert send_email.call_args[0][2] == "Your environment failed to build"
+    assert (
+        send_email.call_args[0][2]
+        == "Your SoftPack environment failed to build"
+    )
     assert send_email.call_args[0][3] == "me"
 
     testable_env_input.username = ""
@@ -492,7 +495,10 @@ async def test_email_on_build_complete(
     assert send_email.call_args[0][0] == app.settings.environments
     assert "failed to build" in send_email.call_args[0][1]
     assert "version conflict" in send_email.call_args[0][1]
-    assert send_email.call_args[0][2] == "Your environment failed to build"
+    assert (
+        send_email.call_args[0][2]
+        == "Your SoftPack environment failed to build"
+    )
     assert send_email.call_args[0][3] == "me"