From 35fc9f2382eab8ec2a9b49c1187a9c5afc05f72f Mon Sep 17 00:00:00 2001
From: Daniel Nemergut <dnemergu@nrao.edu>
Date: Tue, 4 Jun 2024 22:51:04 -0400
Subject: [PATCH] Made get_versions return the recommended version first in the
 list

---
 .../capability/capability/views/capability.py | 20 ++++++----
 .../system/services/casa_matrix_service.py    | 39 ++++++++++++++-----
 .../workspaces/system/services/interfaces.py  |  4 +-
 3 files changed, 45 insertions(+), 18 deletions(-)

diff --git a/services/capability/capability/views/capability.py b/services/capability/capability/views/capability.py
index 9bd8df644..f32f7d3be 100644
--- a/services/capability/capability/views/capability.py
+++ b/services/capability/capability/views/capability.py
@@ -320,7 +320,9 @@ def get_casa_versions(request: Request) -> Response:
         or a 404 response (HTTPNotFound) if none are returned
     """
     params = {
+        "version": request.params["version"] if "version" in request.params else None,
         "capability": request.params["capability"] if "capability" in request.params else None,
+        "telescope": request.params["telescope"] if "telescope" in request.params else None,
     }
     params = {k: v for k, v in params.items() if v is not None}
 
@@ -360,10 +362,11 @@ def add_casa_version(request: Request) -> Response:
     """
     added = request.casa_matrix_service.add_version(
         version=request.params["version"],
-        capabilities=request.params["capabilities"].split(',') if "capabilities" in request.params else [],
-        is_cluster_compatible=False if "is_cluster_compatible" in request.params
-                                       and request.params["is_cluster_compatible"].lower() in ["0", "false"]
-                                       else True,
+        capabilities=request.params["capabilities"].split(",") if "capabilities" in request.params else [],
+        is_cluster_compatible=False
+        if "is_cluster_compatible" in request.params
+        and request.params["is_cluster_compatible"].lower() in ["0", "false"]
+        else True,
     )
 
     if added:
@@ -383,10 +386,11 @@ def update_casa_version(request: Request) -> Response:
     """
     updated = request.casa_matrix_service.update_version(
         version=request.params["version"],
-        capabilities=request.params["capabilities"].split(',') if "capabilities" in request.params else [],
-        is_cluster_compatible=False if "is_cluster_compatible" in request.params
-                                       and request.params["is_cluster_compatible"].lower() in ["0", "false"]
-                                       else True,
+        capabilities=request.params["capabilities"].split(",") if "capabilities" in request.params else [],
+        is_cluster_compatible=False
+        if "is_cluster_compatible" in request.params
+        and request.params["is_cluster_compatible"].lower() in ["0", "false"]
+        else True,
     )
 
     if updated:
diff --git a/shared/workspaces/workspaces/system/services/casa_matrix_service.py b/shared/workspaces/workspaces/system/services/casa_matrix_service.py
index ba094ef77..4a99498c9 100644
--- a/shared/workspaces/workspaces/system/services/casa_matrix_service.py
+++ b/shared/workspaces/workspaces/system/services/casa_matrix_service.py
@@ -26,7 +26,11 @@ from sqlalchemy.exc import SQLAlchemyError
 from sqlalchemy.ext.declarative import declarative_base
 from sqlalchemy.orm import Session
 
-from workspaces.capability.schema import Capability, CasaMatrixCasaVersion, matrix_capabilities
+from workspaces.capability.schema import (
+    Capability,
+    CasaMatrixCasaVersion,
+    matrix_capabilities,
+)
 from workspaces.system.services.interfaces import CasaMatrixServiceIF
 
 logger = logging.getLogger(__name__)
@@ -247,25 +251,40 @@ class CasaMatrixService(CasaMatrixServiceIF):
         else:
             return installed_version
 
-    def get_versions(self, capability: str | None = DEFAULT_CAPABILITY) -> list[dict[str, str]]:
+    def get_versions(
+        self,
+        version: str | None = None,
+        capability: str | None = DEFAULT_CAPABILITY,
+        telescope: str | None = DEFAULT_TELESCOPE,
+    ) -> list[dict[str, str]]:
         """
         Returns the CASA versions that are valid for processing as a dict.
+        The recommended version (either the given version or the default for the given capability) is first in the list.
 
-        :param capability: (Optional) The capability to filter versions on
+        :param version: (Optional) A potential CASA version to use as the default
+        :param capability: (Optional) The capability to filter versions on and to determine the recommended version
+        :param telescope: (Optional) The telescope to distinguish a recommended version from CAPO
         :return: A dict mapping valid CASA versions to their paths
         """
         versions = []
 
+        rec_version = version if version else self.get_default_version(capability, telescope)
+
         # Installed versions are a flat list of dicts containing version names and paths
         self.installed_versions = self.find_installed_versions()
 
         # Allowed versions are a dict of capabilities mapping to lists of version names
         self.allowed_versions = self.fetch_allowed_versions(capability)
 
+        # Add any versions that are both allowed and installed to the list
         for allowed_version in self.allowed_versions:
             installed_version = self.get_installed_version_by_basename(allowed_version)
             if installed_version:
-                versions.append(installed_version)
+                # Put the recommended version as first in the list
+                if allowed_version == rec_version:
+                    versions.insert(0, installed_version)
+                else:
+                    versions.append(installed_version)
 
         logger.info(f"Found CASA versions for {capability}: {versions}")
 
@@ -321,7 +340,7 @@ class CasaMatrixService(CasaMatrixServiceIF):
         :param capabilities: List of capability names to associate to the given version
         """
         matrix_version = self.session.query(CasaMatrixCasaVersion).filter_by(casa_version=version).first()
-        mc = [{'matrix_id': matrix_version.matrix_id, 'capability_name': c} for c in capabilities]
+        mc = [{"matrix_id": matrix_version.matrix_id, "capability_name": c} for c in capabilities]
 
         self.session.execute(insert(matrix_capabilities), mc)
         self.session.flush()
@@ -355,10 +374,12 @@ class CasaMatrixService(CasaMatrixServiceIF):
         :return: True if updated successfully
         """
         try:
-            self.session.query(CasaMatrixCasaVersion).filter_by(casa_version=version).update({
-                'casa_version': version,
-                'is_cluster_compatible': is_cluster_compatible,
-            })
+            self.session.query(CasaMatrixCasaVersion).filter_by(casa_version=version).update(
+                {
+                    "casa_version": version,
+                    "is_cluster_compatible": is_cluster_compatible,
+                }
+            )
             self.session.flush()
             if capabilities:
                 # Link the given capabilities to the new matrix version, wasn't sure how to do this in one go
diff --git a/shared/workspaces/workspaces/system/services/interfaces.py b/shared/workspaces/workspaces/system/services/interfaces.py
index 60e98b487..2086dd79e 100644
--- a/shared/workspaces/workspaces/system/services/interfaces.py
+++ b/shared/workspaces/workspaces/system/services/interfaces.py
@@ -65,7 +65,9 @@ class CasaMatrixServiceIF(ABC):
         raise NotImplementedError
 
     @abstractmethod
-    def get_versions(self, capability: str | None = None) -> list[dict[str, str]]:
+    def get_versions(
+        self, version: str | None = None, capability: str | None = None, telescope: str | None = None
+    ) -> list[dict[str, str]]:
         raise NotImplementedError
 
     @abstractmethod
-- 
GitLab