fix: move _list_upgradable_apps() logic from tools.py to apps.py

share/actionsmap.yml

@@ -735,6 +735,10 @@ app:
                     full: --full
                     help: Display all details, including the app manifest and various other infos
                     action: store_true
+                -u:
+                    full: --upgradable
+                    help: List only apps that can upgrade to a newer version
+                    action: store_true
 
         ### app_info()
         info:

src/app.py

@@ -95,7 +95,7 @@ APP_FILES_TO_COPY = [
 ]
 
 
-def app_list(full=False):
+def app_list(full=False, upgradable=False):
     """
     List installed apps
     """
@@ -103,17 +103,19 @@ def app_list(full=False):
     out = []
     for app_id in sorted(_installed_apps()):
         try:
-            app_info_dict = app_info(app_id, full=full)
+            app_info_dict = app_info(app_id, full=full, upgradable=upgradable)
         except Exception as e:
             logger.error(f"Failed to read info for {app_id} : {e}")
             continue
         app_info_dict["id"] = app_id
+        if upgradable and app_info_dict.get("upgradable") != "yes":
+            continue
         out.append(app_info_dict)
 
     return {"apps": out}
 
 
-def app_info(app, full=False):
+def app_info(app, full=False, upgradable=False):
     """
     Get info for a specific app
     """
@@ -139,6 +141,25 @@ def app_info(app, full=False):
     if "domain" in settings and "path" in settings:
         ret["domain_path"] = settings["domain"] + settings["path"]
 
+    if not upgradable and not full:
+        return ret
+
+    absolute_app_name, _ = _parse_app_instance_name(app)
+    from_catalog = _load_apps_catalog()["apps"].get(absolute_app_name, {})
+
+    ret["upgradable"] = _app_upgradable({** ret, "from_catalog": from_catalog})
+
+    if ret["upgradable"] == "yes":
+        ret["current_version"] = ret.get("version", "?")
+        ret["new_version"] = from_catalog.get("manifest", {}).get("version", "?")
+
+        if ret["current_version"] == ret["new_version"]:
+            current_revision = settings.get("current_revision", "?")[:7]
+            new_revision = from_catalog.get("git", {}).get("revision", "?")[:7]
+
+            ret["current_version"] = " ({current_revision})"
+            ret["new_version"] = " ({new_revision})"
+
     if not full:
         return ret
 
@@ -149,9 +170,7 @@ def app_info(app, full=False):
     )
     ret["settings"] = settings
 
-    absolute_app_name, _ = _parse_app_instance_name(app)
+    ret["from_catalog"] = from_catalog
-    ret["from_catalog"] = _load_apps_catalog()["apps"].get(absolute_app_name, {})
-    ret["upgradable"] = _app_upgradable(ret)
 
     ret["is_webapp"] = "domain" in settings and "path" in settings
 

src/tools.py

@@ -37,8 +37,8 @@ from moulinette.utils.process import call_async_output
 from moulinette.utils.filesystem import read_yaml, write_to_yaml, cp, mkdir, rm
 
 from yunohost.app import (
-    app_info,
     app_upgrade,
+    app_list
 )
 from yunohost.app_catalog import (
     _initialize_apps_catalog_system,
@@ -56,8 +56,6 @@ from yunohost.utils.packages import (
 from yunohost.utils.error import YunohostError, YunohostValidationError
 from yunohost.log import is_unit_operation, OperationLogger
 
-# FIXME this is a duplicate from apps.py
-APPS_SETTING_PATH = "/etc/yunohost/apps/"
 MIGRATIONS_STATE_PATH = "/etc/yunohost/migrations.yaml"
 
 logger = getActionLogger("yunohost.tools")
@@ -391,7 +389,7 @@ def tools_update(target=None):
         except YunohostError as e:
             logger.error(str(e))
 
-        upgradable_apps = list(_list_upgradable_apps())
+        upgradable_apps = list(app_list(upgradable=True)["apps"])
 
     if len(upgradable_apps) == 0 and len(upgradable_system_packages) == 0:
         logger.info(m18n.n("already_up_to_date"))
@@ -399,41 +397,6 @@ def tools_update(target=None):
     return {"system": upgradable_system_packages, "apps": upgradable_apps}
 
 
-def _list_upgradable_apps():
-
-    app_list_installed = os.listdir(APPS_SETTING_PATH)
-    for app_id in app_list_installed:
-
-        app_dict = app_info(app_id, full=True)
-
-        if app_dict["upgradable"] == "yes":
-
-            # FIXME : would make more sense for these infos to be computed
-            # directly in app_info and used to check the upgradability of
-            # the app...
-            current_version = app_dict.get("manifest", {}).get("version", "?")
-            current_commit = app_dict.get("settings", {}).get("current_revision", "?")[
-                :7
-            ]
-            new_version = (
-                app_dict.get("from_catalog", {}).get("manifest", {}).get("version", "?")
-            )
-            new_commit = (
-                app_dict.get("from_catalog", {}).get("git", {}).get("revision", "?")[:7]
-            )
-
-            if current_version == new_version:
-                current_version += " (" + current_commit + ")"
-                new_version += " (" + new_commit + ")"
-
-            yield {
-                "id": app_id,
-                "label": app_dict["label"],
-                "current_version": current_version,
-                "new_version": new_version,
-            }
-
-
 @is_unit_operation()
 def tools_upgrade(operation_logger, target=None):
     """
@@ -466,7 +429,7 @@ def tools_upgrade(operation_logger, target=None):
 
         # Make sure there's actually something to upgrade
 
-        upgradable_apps = [app["id"] for app in _list_upgradable_apps()]
+        upgradable_apps = [app["id"] for app in app_list(upgradable=True)["apps"]]
 
         if not upgradable_apps:
             logger.info(m18n.n("apps_already_up_to_date"))