Merge pull request #640 from eitchtee/fix/update-check

fix(updates): keep update banner across page loads and restarts

app/WYGIWYH/settings.py

@@ -328,7 +328,12 @@ CACHES = {
     "default": {
         "BACKEND": "django.core.cache.backends.filebased.FileBasedCache",
         "LOCATION": "/var/tmp/django_cache",
-    }
+    },
+    # Kept apart from "default": cachalot fills it and FileBasedCache culls random entries past MAX_ENTRIES
+    "update_check": {
+        "BACKEND": "django.core.cache.backends.filebased.FileBasedCache",
+        "LOCATION": "/var/tmp/django_cache_update_check",
+    },
 }
 
 DJANGO_VITE_ASSETS_PATH = STATIC_ROOT

app/apps/common/apps.py

@@ -1,5 +1,4 @@
 from django.apps import AppConfig
-from django.core.cache import cache
 
 
 class CommonConfig(AppConfig):
@@ -20,9 +19,5 @@ class CommonConfig(AppConfig):
         admin.site.unregister(SocialApp)
         admin.site.unregister(SocialToken)
 
-        # Delete the cache for update checks to prevent false-positives when the app is restarted
-        # this will be recreated by the check_for_updates task
-        cache.delete("update_check")
-
         # Register system checks for required environment variables
         from apps.common import checks  # noqa: F401

app/apps/common/tasks.py

@@ -5,7 +5,7 @@ from asgiref.sync import sync_to_async
 from django.conf import settings
 from django.core import management
 from django.db import DEFAULT_DB_ALIAS
-from django.core.cache import cache
+from django.core.cache import caches
 
 from procrastinate import builtin_tasks
 from procrastinate.contrib.django import app
@@ -107,26 +107,13 @@ def check_for_updates(timestamp=None):
 
         if latest_version:
             try:
-                current_v = parse_version(settings.APP_VERSION)
+                latest_version = str(parse_version(latest_version))
             except InvalidVersion:
-                current_v = parse_version("0.0.0")
-            try:
-                latest_v = parse_version(latest_version)
-            except InvalidVersion:
-                latest_v = parse_version("0.0.0")
-
-            update_info = {
-                "update_available": False,
-                "current_version": str(current_v),
-                "latest_version": str(latest_v),
-            }
-
-            if latest_v > current_v:
-                update_info["update_available"] = True
-
-            # Cache the entire dictionary
-            cache.set("update_check", update_info, 60 * 60 * 25)
-            logger.info(f"Update check complete. Result: {update_info}")
+                logger.warning(f"Invalid version tag from GitHub: {latest_version}")
+                return f"Invalid version tag: {latest_version}"
+            # Only the latest tag is stored; it's compared to APP_VERSION at render time so upgrades never leave it stale
+            caches["update_check"].set("latest_version", latest_version, None)
+            logger.info(f"Update check complete. Latest version: {latest_version}")
         else:
             logger.warning("Could not find 'tag_name' in GitHub API response.")
 

app/apps/common/templatetags/cache_access.py

@@ -1,6 +1,7 @@
-# core/templatetags/update_tags.py
 from django import template
-from django.core.cache import cache
+from django.conf import settings
+from django.core.cache import caches
+from packaging.version import InvalidVersion, parse as parse_version
 
 register = template.Library()
 
@@ -8,10 +9,18 @@ register = template.Library()
 @register.simple_tag
 def get_update_check():
     """
-    Retrieves the update status dictionary from the cache.
-    Returns a default dictionary if nothing is found.
+    Compares the latest GitHub release (cached by the check_for_updates task)
+    against the running APP_VERSION.
     """
-    return cache.get("update_check") or {
-        "update_available": False,
-        "latest_version": "N/A",
-    }
+    latest_version = caches["update_check"].get("latest_version")
+    # The cached tag never expires, so opting out must also hide what was found before
+    if not settings.CHECK_FOR_UPDATES or not latest_version:
+        return {"update_available": False, "latest_version": "N/A"}
+
+    try:
+        update_available = parse_version(latest_version) > parse_version(settings.APP_VERSION)
+    except InvalidVersion:
+        # Builds without a real version (dev, "unknown") can't be compared, so don't claim an update
+        update_available = False
+
+    return {"update_available": update_available, "latest_version": latest_version}

app/apps/common/tests/test_update_check.py

@@ -0,0 +1,67 @@
+from unittest import mock
+
+from django.core.cache import caches
+from django.test import SimpleTestCase, override_settings
+
+from apps.common.tasks import check_for_updates
+from apps.common.templatetags.cache_access import get_update_check
+
+# Distinct LOCATIONs: LocMemCache aliases with the same LOCATION share one store and would hide wrong-cache bugs
+CACHES = {
+    "default": {"BACKEND": "django.core.cache.backends.locmem.LocMemCache", "LOCATION": "default"},
+    "update_check": {"BACKEND": "django.core.cache.backends.locmem.LocMemCache", "LOCATION": "update_check"},
+}
+
+
+@override_settings(CACHES=CACHES)
+class GetUpdateCheckTests(SimpleTestCase):
+    def setUp(self):
+        caches["update_check"].clear()
+
+    def render_with(self, app_version, latest):
+        if latest:
+            caches["update_check"].set("latest_version", latest)
+        with self.settings(APP_VERSION=app_version):
+            return get_update_check()
+
+    def test_newer_release_is_available(self):
+        self.assertEqual(
+            self.render_with("1.0.0", "1.1.0"),
+            {"update_available": True, "latest_version": "1.1.0"},
+        )
+
+    def test_after_upgrade_no_update(self):
+        self.assertFalse(self.render_with("1.1.0", "1.1.0")["update_available"])
+        self.assertFalse(self.render_with("1.2.0", "1.1.0")["update_available"])
+
+    def test_nothing_cached(self):
+        self.assertEqual(
+            self.render_with("1.0.0", None),
+            {"update_available": False, "latest_version": "N/A"},
+        )
+
+    def test_unknown_app_version_hides_update(self):
+        self.assertFalse(self.render_with("unknown", "1.1.0")["update_available"])
+
+    @override_settings(CHECK_FOR_UPDATES=False)
+    def test_disabled_hides_cached_update(self):
+        self.assertFalse(self.render_with("1.0.0", "1.1.0")["update_available"])
+
+
+@override_settings(CACHES=CACHES, CHECK_FOR_UPDATES=True)
+class CheckForUpdatesTaskTests(SimpleTestCase):
+    def setUp(self):
+        caches["update_check"].clear()
+
+    def run_task(self, tag):
+        with mock.patch("apps.common.tasks.requests.get") as get:
+            get.return_value.json.return_value = {"tag_name": tag}
+            check_for_updates.func()
+        return caches["update_check"].get("latest_version")
+
+    def test_stores_normalized_version(self):
+        self.assertEqual(self.run_task("v1.8.0"), "1.8.0")
+
+    def test_invalid_tag_keeps_previous_value(self):
+        caches["update_check"].set("latest_version", "1.7.0")
+        self.assertEqual(self.run_task("not-a-version"), "1.7.0")

docker/dev/django/start

@@ -17,4 +17,7 @@ touch /tmp/migrations_complete
 python manage.py setup_users
 python manage.py setup_oauth
 
+# Check for updates on startup instead of waiting for the next periodic run; never block boot on it
+python manage.py procrastinate defer check_for_updates || true
+
 exec python manage.py runserver 0.0.0.0:$INTERNAL_PORT

docker/prod/django/start

@@ -18,4 +18,7 @@ touch /tmp/migrations_complete
 python manage.py setup_users
 python manage.py setup_oauth
 
+# Check for updates on startup instead of waiting for the next periodic run; never block boot on it
+python manage.py procrastinate defer check_for_updates || true
+
 exec gunicorn WYGIWYH.wsgi:application --bind 0.0.0.0:$INTERNAL_PORT --timeout 600