Merge pull request qgis#452 from Xpirix/feedback_clarification

Feedback clarification

.github/workflows/test.yaml

@@ -52,15 +52,15 @@ jobs:
 
 
       - name: Run the containers
-        run: docker-compose up -d db devweb
+        run: docker compose up -d db devweb
 
 
       - name: Wait for the containers to start
         run: sleep 15 
 
       - name: Run test
         run: |
-          docker-compose exec -T devweb bash -c '
+          docker compose exec -T devweb bash -c '
             set -e  # Exit immediately if any command fails
             python manage.py makemigrations &&
             python manage.py migrate &&

qgis-app/plugins/models.py

@@ -14,6 +14,8 @@
 from taggit_autosuggest.managers import TaggableManager
 from rest_framework_simplejwt.token_blacklist.models import OutstandingToken
 
+from django.db.models import OuterRef, Count, Subquery, F
+
 PLUGINS_STORAGE_PATH = getattr(settings, "PLUGINS_STORAGE_PATH", "packages/%Y")
 PLUGINS_FRESH_DAYS = getattr(settings, "PLUGINS_FRESH_DAYS", 30)
 
@@ -168,6 +170,16 @@ def get_queryset(self):
             super(UnapprovedPlugins, self)
             .get_queryset()
             .filter(pluginversion__approved=False, deprecated=False)
+            .extra(
+                select={
+                    "average_vote": "rating_score/(rating_votes+0.001)",
+                    "latest_version_date": (
+                        "SELECT created_on FROM plugins_pluginversion WHERE "
+                        "plugins_pluginversion.plugin_id = plugins_plugin.id "
+                        "ORDER BY created_on DESC LIMIT 1"
+                    ),
+                }
+            )
             .distinct()
         )
 
@@ -274,17 +286,78 @@ def get_queryset(self):
         return super(ServerPlugins, self).get_queryset().filter(server=True).distinct()
 
 
+class FeedbackCompletedPlugins(models.Manager):
+    """
+    Show only unapproved plugins with resolved feedbacks
+    """
+    def get_queryset(self):
+        feedback_count_subquery = PluginVersionFeedback.objects.filter(
+            version=OuterRef('pluginversion'),
+            is_completed=True
+        ).values('version').annotate(
+            completed_count=Count('id')
+        ).values('completed_count')
+
+        return (
+            super(FeedbackCompletedPlugins, self)
+            .get_queryset()
+            .filter(
+                pluginversion__approved=False,
+                deprecated=False
+            )
+            .annotate(
+                total_feedback_count=Count('pluginversion__feedback'),
+                completed_feedback_count=Subquery(feedback_count_subquery)
+            )
+            .filter(
+                total_feedback_count=F('completed_feedback_count')
+            )
+            .extra(
+                select={
+                    "average_vote": "rating_score/(rating_votes+0.001)",
+                    "latest_version_date": (
+                        "SELECT created_on FROM plugins_pluginversion WHERE "
+                        "plugins_pluginversion.plugin_id = plugins_plugin.id "
+                        "ORDER BY created_on DESC LIMIT 1"
+                    ),
+                }
+            ).distinct()
+        )
+
 class FeedbackReceivedPlugins(models.Manager):
     """
-    Show only unapproved plugins with a feedback
+    Show only unapproved plugins with a pending feedback
     """
     def get_queryset(self):
+        feedback_count_subquery = PluginVersionFeedback.objects.filter(
+            version=OuterRef('pluginversion'),
+            is_completed=False
+        ).values('version').annotate(
+            received_count=Count('id')
+        ).values('received_count')
+
         return (
             super(FeedbackReceivedPlugins, self)
             .get_queryset()
             .filter(
                 pluginversion__approved=False,
-                pluginversion__feedback__isnull=False
+                deprecated=False
+            )
+            .annotate(
+                received_feedback_count=Subquery(feedback_count_subquery)
+            )
+            .filter(
+                received_feedback_count__gte=1
+            )
+            .extra(
+                select={
+                    "average_vote": "rating_score/(rating_votes+0.001)",
+                    "latest_version_date": (
+                        "SELECT created_on FROM plugins_pluginversion WHERE "
+                        "plugins_pluginversion.plugin_id = plugins_plugin.id "
+                        "ORDER BY created_on DESC LIMIT 1"
+                    ),
+                }
             ).distinct()
         )
 
@@ -299,7 +372,23 @@ def get_queryset(self):
             .get_queryset()
             .filter(
                 pluginversion__approved=False,
-                pluginversion__feedback__isnull=True
+                deprecated=False
+            )
+            .annotate(
+                total_feedback_count=Count('pluginversion__feedback'),
+            )
+            .filter(
+                total_feedback_count=0
+            )
+            .extra(
+                select={
+                    "average_vote": "rating_score/(rating_votes+0.001)",
+                    "latest_version_date": (
+                        "SELECT created_on FROM plugins_pluginversion WHERE "
+                        "plugins_pluginversion.plugin_id = plugins_plugin.id "
+                        "ORDER BY created_on DESC LIMIT 1"
+                    ),
+                }
             ).distinct()
         )
 
@@ -407,6 +496,7 @@ class Plugin(models.Model):
     most_voted_objects = MostVotedPlugins()
     most_rated_objects = MostRatedPlugins()
     server_objects = ServerPlugins()
+    feedback_completed_objects = FeedbackCompletedPlugins()
     feedback_received_objects = FeedbackReceivedPlugins()
     feedback_pending_objects = FeedbackPendingPlugins()
 

qgis-app/plugins/templates/plugins/plugin_base.html

@@ -23,8 +23,9 @@ <h3>{% trans "Plugins" %}</h3>
         {% if user.is_staff %}
         <li class="staff"><a href="{% url "unapproved_plugins" %}">{% trans "Unapproved"%}</a></li>
         <ul>
-            <li class="sub-list"><a href="{% url "feedback_received_plugins" %}">{% trans "Feedback received"%}</a></li>
-            <li class="sub-list"><a href="{% url "feedback_pending_plugins" %}">{% trans "Feedback pending"%}</a></li>
+            <li class="sub-list"><a href="{% url "feedback_completed_plugins" %}">{% trans "Reviewed Plugins (Resolved)"%}</a></li>
+            <li class="sub-list"><a href="{% url "feedback_received_plugins" %}">{% trans "Reviewed Plugins (Pending)"%}</a></li>
+            <li class="sub-list"><a href="{% url "feedback_pending_plugins" %}">{% trans "Awaiting review"%}</a></li>
         </ul>
         <li class="staff"><a href="{% url "deprecated_plugins" %}">{% trans "Deprecated"%}</a></li>
         {% endif %}

qgis-app/plugins/tests/test_plugin_version_feedback.py

@@ -122,6 +122,66 @@ def test_add_recipient_in_email_notification(self):
             settings.EMAIL_HOST_USER
         )
 
+class TestPluginFeedbackCompletedList(SetupMixin, TestCase):
+    fixtures = ["fixtures/simplemenu.json", "fixtures/auth.json"]
+
+    def setUp(self):
+        super().setUp()
+        self.feedback_1.is_completed = True
+        self.feedback_1.save()
+        self.url = reverse("feedback_completed_plugins")
+
+    def test_non_staff_should_not_see_plugin_feedback_completed_list(self):
+        response = self.client.get(self.url)
+        self.assertEqual(response.status_code, 404)
+
+        self.client.force_login(user=self.creator)
+        response = self.client.get(self.url)
+        self.assertEqual(response.status_code, 404)
+
+    def test_staff_should_see_plugin_feedback_completed(self):
+        self.client.force_login(user=self.staff)
+        response = self.client.get(self.url)
+        self.assertEqual(response.status_code, 200)
+        self.assertTemplateUsed(
+            response, 'plugins/plugin_list.html'
+        )
+        self.assertEqual(
+            list(response.context['object_list']),
+            [self.plugin_1]
+        )
+        self.assertContains(response, "test plugin 1")
+        self.assertNotContains(response, "test plugin 2")
+
+        # add feedback for plugin 2
+        PluginVersionFeedback.objects.create(
+            version=self.version_2,
+            reviewer=self.staff,
+            task="test comment in a feedback for plugin 2."
+        )
+        response = self.client.get(self.url)
+
+        # The plugin should not appear in the feedback completed list
+        self.assertEqual(
+            list(response.context['object_list']),
+            [self.plugin_1]
+        )
+        self.assertNotContains(response, "test plugin 2")
+
+    def test_approved_plugin_should_not_show_in_feedback_completed_list(self):
+        self.client.force_login(user=self.staff)
+        response = self.client.get(self.url)
+        self.assertEqual(
+            list(response.context['object_list']),
+            [self.plugin_1]
+        )
+        self.version_1.approved = True
+        self.version_1.save()
+        response = self.client.get(self.url)
+        self.assertEqual(
+            list(response.context['object_list']),
+            []
+        )
 
 class TestPluginFeedbackReceivedList(SetupMixin, TestCase):
     fixtures = ["fixtures/simplemenu.json", "fixtures/auth.json"]
@@ -159,10 +219,9 @@ def test_staff_should_see_plugin_feedback_received(self):
             task="test comment in a feedback for plugin 2."
         )
         response = self.client.get(self.url)
-        self.assertEqual(
-            list(response.context['object_list']),
-            [self.plugin_1, self.plugin_2]
-        )
+        object_list = set(response.context['object_list'])
+        expected_objects = {self.plugin_1, self.plugin_2}
+        self.assertEqual(object_list, expected_objects)
         self.assertContains(response, "test plugin 2")
 
     def test_approved_plugin_should_not_show_in_feedback_received_list(self):

qgis-app/plugins/urls.py

@@ -120,7 +120,7 @@
     url(
         r"^unapproved/$",
         PluginsList.as_view(
-            queryset=Plugin.unapproved_objects.all(),
+            queryset=Plugin.unapproved_objects.all().order_by("-latest_version_date"),
             additional_context={"title": _("Unapproved plugins")},
         ),
         name="unapproved_plugins",
@@ -205,17 +205,24 @@
         ),
         name="most_rated_plugins",
     ),
+    url(
+        r"^feedback_completed/$",
+        FeedbackCompletedPluginsList.as_view(
+            additional_context={"title": _("Reviewed Plugins (Resolved)")}
+        ),
+        name="feedback_completed_plugins",
+    ),
     url(
         r"^feedback_pending/$",
         FeedbackPendingPluginsList.as_view(
-            additional_context={"title": _("Feedback pending plugins")}
+            additional_context={"title": _("Awaiting review")}
         ),
         name="feedback_pending_plugins",
     ),
     url(
         r"^feedback_received/$",
         FeedbackReceivedPluginsList.as_view(
-            additional_context={"title": _("Feedback received plugins")}
+            additional_context={"title": _("Reviewed Plugins (Pending)")}
         ),
         name="feedback_received_plugins",
     ),

qgis-app/plugins/views.py

@@ -971,27 +971,40 @@ def get_context_data(self, **kwargs):
         return context
 
 
+class FeedbackCompletedPluginsList(PluginsList):
+    """List of Plugins that has feedback resolved in its versions.
+
+    The plugins editor can only see their plugin feedbacks.
+    The staff can see all plugin feedbacks.
+    """
+    queryset = Plugin.feedback_completed_objects.all().order_by("-latest_version_date")
+
+    def get_filtered_queryset(self, qs):
+        user = get_object_or_404(User, username=self.request.user)
+        if not user.is_staff:
+            raise Http404
+        return qs
+
 class FeedbackReceivedPluginsList(PluginsList):
     """List of Plugins that has feedback received in its versions.
 
     The plugins editor can only see their plugin feedbacks.
     The staff can see all plugin feedbacks.
     """
-    queryset = Plugin.feedback_received_objects.all()
+    queryset = Plugin.feedback_received_objects.all().order_by("-latest_version_date")
 
     def get_filtered_queryset(self, qs):
         user = get_object_or_404(User, username=self.request.user)
         if not user.is_staff:
             raise Http404
         return qs
 
-
 class FeedbackPendingPluginsList(PluginsList):
     """List of Plugins that has feedback pending in its versions.
 
     Only staff can see plugin feedback list.
     """
-    queryset = Plugin.feedback_pending_objects.all()
+    queryset = Plugin.feedback_pending_objects.all().order_by("-latest_version_date")
 
     def get_filtered_queryset(self, qs):
         user = get_object_or_404(User, username=self.request.user)