Fix: Search results stop after first model due to incorrect indentation (#127)

.gitignore

@@ -2,10 +2,13 @@
 htmlcov/
 build/
 *~
-.DS_Store
-.idea/
+venv/
 __pycache__/
 *.pyc
+.env
+build/
+.DS_Store
+.idea/
 cyappstore.sql
 *.tgz
 *.swp

apps/tests.py

@@ -13,6 +13,10 @@
 from PIL import Image, ImageDraw
 
 from django.conf import settings
+from django.urls import reverse
+from datetime import date, timedelta
+from apps.views import get_top_downloaded_apps
+from download.models import ReleaseDownloadsByDate
 from django.core.files.uploadedfile import SimpleUploadedFile
 from django.contrib.auth.models import User
 from django.test import TestCase
@@ -676,4 +680,83 @@ def test_app_button_by_name_found_app(self):
                                     active=True)
         appobj.save()
         res = app_buttons.app_button_by_name('myapp')
-        self.assertEqual('myapp', res['app'].name)
+        self.assertEqual('myapp', res['app'].name)
+
+
+class TopDownloadedAppsTestCase(TestCase):
+
+    def setUp(self):
+        self.app1 = App.objects.create(name="app1", fullname="App One", active=True)
+        self.app2 = App.objects.create(name="app2", fullname="App Two", active=True)
+
+        uploaded = SimpleUploadedFile("file.jar", b"hello", content_type="text/plain")
+
+        self.rel1 = Release.objects.create(
+            app=self.app1,
+            version="1.0",
+            release_file=uploaded,
+            active=True
+        )
+
+        self.rel2 = Release.objects.create(
+            app=self.app2,
+            version="1.0",
+            release_file=uploaded,
+            active=True
+        )
+
+        today = date.today()
+
+        # within 24 months
+        ReleaseDownloadsByDate.objects.create(
+            release=self.rel1,
+            when=today - timedelta(days=100),
+            count=100
+        )
+
+        # older than 24 months (ignored)
+        ReleaseDownloadsByDate.objects.create(
+            release=self.rel2,
+            when=today - timedelta(days=900),
+            count=500
+        )
+
+    def test_top_downloads_returns_apps(self):
+        apps = list(get_top_downloaded_apps())
+
+        self.assertIn(self.app1, apps)
+        self.assertNotIn(self.app2, apps)
+
+class TagNavigationTests(TestCase):
+
+    def setUp(self):
+        self.tag = Tag.objects.create(name="cluster", fullname="Cluster")
+
+        self.active_app = App.objects.create(
+            name="clusteractive",
+            fullname="Cluster Active",
+            active=True
+        )
+
+        self.inactive_app = App.objects.create(
+            name="clusterinactive",
+            fullname="Cluster Inactive",
+            active=False
+        )
+
+        self.active_app.tags.add(self.tag)
+        self.inactive_app.tags.add(self.tag)
+
+    def test_tag_page_shows_only_active_apps(self):
+        url = reverse("tag_page", args=["cluster"])
+        response = self.client.get(url)
+
+        self.assertEqual(response.status_code, 200)
+        self.assertContains(response, "Cluster Active")
+        self.assertNotContains(response, "Cluster Inactive")
+
+    def test_inactive_app_page_returns_404(self):
+        url = reverse("app_page", args=["clusterinactive"])
+        response = self.client.get(url)
+
+        self.assertEqual(response.status_code, 404)

apps/views.py

@@ -1,6 +1,10 @@
 import re
 import datetime
 import html
+from datetime import date, timedelta
+from django.db.models import Case, When, IntegerField
+from django.db.models import Sum
+from download.models import ReleaseDownloadsByDate
 from urllib.parse import unquote
 from django.contrib.auth.models import User
 from django.contrib.auth.decorators import login_required
@@ -29,15 +33,16 @@ class _NavPanelConfig:
     tag_cloud_delta_font_size_em = tag_cloud_max_font_size_em - tag_cloud_min_font_size_em
 
 def _all_tags_of_count(min_count):
-    return filter(lambda tag: tag.count >= min_count, Tag.objects.all())
+    return filter(lambda tag: tag.count >= min_count,
+    Tag.objects.filter(app__active=True).distinct())
 
 _NavPanelContextCache = None
 
 def _nav_panel_context(request):
     global _NavPanelContextCache
     if _NavPanelContextCache:
         return _NavPanelContextCache
-    all_tags = _all_tags_of_count(_NavPanelConfig.min_tag_count)
+    all_tags = list(_all_tags_of_count(_NavPanelConfig.min_tag_count))
     sorted_tags = sorted(all_tags, key=lambda tag: tag.count)
     sorted_tags.reverse()
 
@@ -79,10 +84,41 @@ def _flush_tag_caches():
 class _DefaultConfig:
     num_of_top_apps = 6
 
+def get_top_downloaded_apps(limit=None):
+    two_years_ago = date.today() - timedelta(days=730)
+
+    top_download_data = (
+        ReleaseDownloadsByDate.objects
+        .filter(
+            when__gte=two_years_ago,
+            release__app__active=True,
+            release__active=True
+        )
+        .values('release__app')
+        .annotate(total_downloads=Sum('count'))
+        .order_by('-total_downloads')
+    )
+
+    if limit is not None:
+        top_download_data = top_download_data[:limit]
+
+    app_ids = [item['release__app'] for item in top_download_data]
+
+    if not app_ids:
+        return App.objects.none()
+
+    preserved_order = Case(
+        *[When(id=pk, then=pos) for pos, pk in enumerate(app_ids)],
+        output_field=IntegerField()
+    )
+
+    return App.objects.filter(id__in=app_ids).order_by(preserved_order)
+
+
 def apps_default(request):
     latest_apps = App.objects.filter(active=True).order_by('-latest_release_date')[:_DefaultConfig.num_of_top_apps]
-    downloaded_apps = App.objects.filter(active=True).order_by('downloads').reverse()[:_DefaultConfig.num_of_top_apps]
-
+    # downloaded_apps = App.objects.filter(active=True).order_by('downloads').reverse()[:_DefaultConfig.num_of_top_apps]
+    downloaded_apps = get_top_downloaded_apps(_DefaultConfig.num_of_top_apps)
     c = {
         'latest_apps': latest_apps,
         'downloaded_apps': downloaded_apps,
@@ -110,7 +146,8 @@ def all_apps_newest(request):
 
 
 def all_apps_downloads(request):
-    apps = App.objects.filter(active=True).order_by('downloads').reverse()
+    # apps = App.objects.filter(active=True).order_by('downloads').reverse()
+    apps = get_top_downloaded_apps()
     c = {
         'apps': apps,
         'navbar_selected_link': 'all',
@@ -120,10 +157,10 @@ def all_apps_downloads(request):
 
 def wall_of_apps(request):
     nav_panel_context = _nav_panel_context(request)
-    tags = [(tag.fullname, tag.app_set.all()) for tag in nav_panel_context['top_tags']]
+    tags = [(tag.fullname, tag.app_set.filter(active=True)) for tag in nav_panel_context['top_tags']]
     apps_in_not_top_tags = set()
     for not_top_tag in nav_panel_context['not_top_tags']:
-        apps_in_not_top_tags.update(not_top_tag.app_set.all())
+        apps_in_not_top_tags.update(not_top_tag.app_set.filter(active=True))
     tags.append(('other', apps_in_not_top_tags))
     c = {
         'total_apps_count': App.objects.filter(active=True).count,
@@ -505,7 +542,7 @@ def app_page_edit(request, app_name):
         if is_ajax(request):
             return json_response(result)
 
-    all_tags = [tag.fullname for tag in Tag.objects.all()]
+    all_tags = [tag.fullname for tag in Tag.objects.filter(app__active=True).distinct()]
     c = {
         'app': app,
         'all_tags': all_tags,

search/tests.py

@@ -4,13 +4,78 @@
 
 Replace this with more appropriate tests for your application.
 """
+import sys
+from unittest.mock import MagicMock
+
+# Mock external dependencies
+sys.modules['xapian'] = MagicMock()
+
+mock_conf = MagicMock()
+mock_conf.XAPIAN_INDICES_DIR = "/tmp"
+sys.modules['conf'] = MagicMock()
+sys.modules['conf.xapian'] = mock_conf
 
 from django.test import TestCase
+from unittest.mock import patch, MagicMock
+from search.views import _xapian_search
+
+
+class DummyModelA:
+    __name__ = "ModelA"
+    search_key = "id"
+
+
+class DummyModelB:
+    __name__ = "ModelB"
+    search_key = "id"
+
+
+class SearchTestCase(TestCase):
+
+    @patch("search.views.get_object_or_none")
+    def test_search_returns_multiple_models(self, mock_get_obj):
+
+        from search import views
+
+        # Mock matches
+        mock_match_1 = MagicMock()
+        mock_match_1.document.get_data.return_value = "1"
+
+        mock_match_2 = MagicMock()
+        mock_match_2.document.get_data.return_value = "2"
+
+        # Mock enquire behavior
+        mock_enquire_1 = MagicMock()
+        mock_enquire_1.get_mset.return_value = [mock_match_1]
+
+        mock_enquire_2 = MagicMock()
+        mock_enquire_2.get_mset.return_value = [mock_match_2]
+
+        mock_qp = MagicMock()
+
+        mock_db_1 = MagicMock()
+        mock_db_1.get_doccount.return_value = 1
+
+        mock_db_2 = MagicMock()
+        mock_db_2.get_doccount.return_value = 1
+
+        # Inject fake Xapian data
+        views.Xapian_Enquires = {
+            DummyModelA: (mock_db_1, mock_enquire_1, mock_qp),
+            DummyModelB: (mock_db_2, mock_enquire_2, mock_qp),
+        }
+
+        # Mock object fetching
+        mock_get_obj.side_effect = lambda model, **kwargs: {
+            "model": model.__name__,
+            "id": kwargs.get("id")
+        }
+
+        results = _xapian_search("test-query")
 
+        # Assertions
+        self.assertIn("DummyModelA", results)
+        self.assertIn("DummyModelB", results)
 
-class SimpleTest(TestCase):
-    def test_basic_addition(self):
-        """
-        Tests that 1 + 1 always equals 2.
-        """
-        self.assertEqual(1 + 1, 2)
+        self.assertEqual(len(results["DummyModelA"]), 1)
+        self.assertEqual(len(results["DummyModelB"]), 1)

search/views.py

@@ -23,33 +23,46 @@ def _init_xapian_search():
         qp.set_stemming_strategy(xapian.QueryParser.STEM_SOME)
         Xapian_Enquires[model] = (db, enquire, qp)
 
-def _xapian_search(query_str, limit = None, only_matching_ids = False):
+def _xapian_search(query_str, limit=None, only_matching_ids=False):
     global Xapian_Enquires
     if not Xapian_Enquires:
         _init_xapian_search()
 
-    if limit and (not type(limit) == type(1) or limit <= 0):
+    if limit and (not isinstance(limit, int) or limit <= 0):
         raise ValueError('limit parameter must be a positive integer')
 
     all_results = {}
+
     for model, (db, enquire, qp) in Xapian_Enquires.items():
         q = qp.parse_query(query_str, qp.FLAG_PARTIAL | qp.FLAG_PHRASE)
         enquire.set_query(q)
         matches = enquire.get_mset(0, limit if limit else db.get_doccount())
-        if not len(matches): continue
+
+        if not len(matches):
+            continue
 
         matched_obj_ids = (match.document.get_data() for match in matches)
+
         if only_matching_ids:
             all_results[model.__name__] = list(matched_obj_ids)
         else:
-            matched_objs = list()
-        for matched_obj_id in matched_obj_ids:
-            matched_obj = get_object_or_none(model, **{model.search_key: matched_obj_id})
-        if not matched_obj: continue
-        matched_objs.append(matched_obj)
-        if matched_objs:
-            all_results[model.__name__] = matched_objs
-        return all_results
+            matched_objs = []
+
+            for matched_obj_id in matched_obj_ids:
+                matched_obj = get_object_or_none(
+                    model,
+                    **{model.search_key: matched_obj_id}
+                )
+
+                if not matched_obj:
+                    continue
+
+                matched_objs.append(matched_obj)
+
+            if matched_objs:
+                all_results[model.__name__] = matched_objs
+
+    return all_results
 
 def removespace(query):
     final_query=''