Modify a feedback feature (qgis#430)

* Allow users to update a feedback comment

* Add test cases for feedback modification

qgis-app/plugins/migrations/0012_pluginversionfeedback_modified_on.py

@@ -0,0 +1,18 @@
+# Generated by Django 4.2.13 on 2024-06-13 08:06
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('plugins', '0011_alter_pluginversiondownload_unique_together'),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name='pluginversionfeedback',
+            name='modified_on',
+            field=models.DateTimeField(blank=True, editable=False, null=True, verbose_name='Modified on'),
+        ),
+    ]

qgis-app/plugins/models.py

@@ -890,6 +890,13 @@ class PluginVersionFeedback(models.Model):
         auto_now_add=True,
         editable=False
     )
+    modified_on = models.DateTimeField(
+        _("Modified on"), 
+        editable=False,
+        blank=True,
+        null=True
+    )
+
     completed_on = models.DateTimeField(
         verbose_name=_("Completed on"),
         blank=True,

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

@@ -18,17 +18,32 @@ <h2>{% trans "Feedback Plugin" %} {{ version.plugin.name }} {{ version.version }
     <div class="feedback-list">
         <p>Please tick the checkbox when the task is completed and click the "Update" button to update status.</p>
         {% for feedback in feedbacks %}
-        <div class=" previous-feedback {% if feedback.reviewer == request.user %}with-box{% endif %}" data-feedback-id="{{ feedback.id }}">
-            <input type="checkbox" class="statusCheckbox pull-left" name="statusCheckbox" data-feedback-id="{{ feedback.id }}" {% if feedback.is_completed %}checked disabled{% endif %}>
-            <label class="feedback">
-                {{ feedback.task }}
-                <span class="feedback-info"> &mdash;{% if feedback.reviewer.first_name %}{{ feedback.reviewer.first_name }} {{ feedback.reviewer.last_name }}{% else %}{{ feedback.reviewer.username }}{% endif %}
-                    wrote {{ feedback.created_on|timesince }} ago
-                </span>
-            </label>
-            {% if feedback.reviewer == request.user %}
-            <button type="button" class="btn btn-danger btn-mini deleteButton pull-right" data-feedback-id="{{ feedback.id }}"><i class="icon-remove"></i></button>
-            {% endif %}
+        <div class="previous-feedback {% if feedback.reviewer == request.user %}with-box{% endif %}" data-feedback-id="{{ feedback.id }}">
+            <span style="font-weight: bolder;"> 
+                {% if feedback.reviewer.first_name %}
+                    {{ feedback.reviewer.first_name }} {{ feedback.reviewer.last_name }}
+                {% else %}
+                    {{ feedback.reviewer.username }}
+                {% endif %}
+            </span>
+            <span class="feedback-info">wrote {{ feedback.created_on|timesince }} ago</span>
+            <div>
+                <input type="checkbox" class="statusCheckbox pull-left" name="statusCheckbox" data-feedback-id="{{ feedback.id }}" {% if feedback.is_completed %}checked disabled{% endif %}>
+                <label class="feedback">
+                    <span id="feedbackTask">{{ feedback.task }}</span>
+                    <span id="editedOn" class="feedback-info">
+                        {% if feedback.modified_on %}
+                            &mdash; (edited) {{ feedback.modified_on|timesince }} ago
+                        {% endif %}
+                    </span>
+                </label>
+                {% if feedback.reviewer == request.user %}
+                <div class="pull-right">
+                    <button type="button" id="editButton" class="btn btn-primary btn-mini" data-feedback-id="{{ feedback.id }}"><i class="icon-pencil"></i></button>
+                    <button type="button" class="btn btn-danger btn-mini deleteButton" data-feedback-id="{{ feedback.id }}"><i class="icon-remove"></i></button>
+                </div>
+                {% endif %}
+            </div>
         </div>
         {% endfor %}
         {% if feedbacks %}
@@ -151,7 +166,7 @@ <h2>{% trans "Feedback Plugin" %} {{ version.plugin.name }} {{ version.version }
             if (confirm(msg)) {
                 $.ajax({
                     type: 'POST',
-                    url: url + feedbackId + '/',
+                    url: url + feedbackId + '/delete/',
                     data: formData,
                     success: function (response) {
                         if (response.success) {
@@ -165,6 +180,56 @@ <h2>{% trans "Feedback Plugin" %} {{ version.plugin.name }} {{ version.version }
                 });
             }
         }
+        $(document).on('click', '#editButton', function() {
+            var $feedbackDiv = $(this).closest('.previous-feedback');
+            var feedbackId = $feedbackDiv.data('feedback-id');
+            var feedbackTask = $feedbackDiv.find('#feedbackTask').text();
+
+            // Store the original content
+            var originalContent = $feedbackDiv.html();
+
+            var inputForm = `
+                <input type="text" class="form-control" id="editFeedbackInput" value="${feedbackTask}" style="width: 100%;margin-bottom: 0 !important;">
+                <div style="display: flex;gap: 10px;">
+                    <button type="button" class="btn btn-success btn-mini" id="saveButton" data-feedback-id="${feedbackId}">Save</button>
+                    <button type="button" class="btn btn-secondary btn-mini" id="cancelButton" data-feedback-id="${feedbackId}">Cancel</button>
+                </div>
+            `;
+
+            $feedbackDiv.html(inputForm);
+            $feedbackDiv.attr("style", "display: flex;justify-content: space-between;align-items: center;gap: 10px;");
+
+            $(document).on('click', '#cancelButton', function() {
+                // Restore the original content
+                $feedbackDiv.html(originalContent);
+                $feedbackDiv.attr("style", "");
+            });
+
+            $(document).on('click', '#saveButton', function() {
+                var newFeedbackTask = $feedbackDiv.find('#editFeedbackInput').val();
+
+                $.ajax({
+                    url: url + feedbackId + '/edit/',
+                    method: 'POST',
+                    data: {
+                        'csrfmiddlewaretoken': '{{ csrf_token }}',
+                        'task': newFeedbackTask
+                    },
+                    success: function(response) {
+                        if (response.success) {
+                            $feedbackDiv.html(originalContent);
+                            $feedbackDiv.attr("style", "");
+                            $feedbackDiv.find('#feedbackTask').html(newFeedbackTask);
+                            $feedbackDiv.find('#editedOn').html(
+                                `&mdash; (edited)`
+                            );
+                        } else {
+                            alert('Failed to update feedback.');
+                        }
+                    }
+                });
+            });
+        });
     })
     </script>
 {% endblock %}

qgis-app/plugins/tests/test_plugin_version_feedback.py

@@ -10,6 +10,8 @@
 from plugins.models import Plugin, PluginVersion, PluginVersionFeedback
 from plugins.views import version_feedback_notify
 from django.conf import settings
+from django.utils.dateformat import format
+import json
 
 class SetupMixin:
     fixtures = ["fixtures/auth.json", "fixtures/simplemenu.json"]
@@ -399,3 +401,43 @@ def test_non_staff_and_non_editor_cannot_update_feedback(self):
         feedback = self.version_1.feedback.first()
         self.assertFalse(feedback.is_completed)
         self.assertIsNone(feedback.completed_on)
+
+class VersionFeedbackEditViewTests(SetupMixin, TestCase):
+
+    def setUp(self):
+        super().setUp()
+
+    def test_version_feedback_edit_not_logged_in(self):
+        url = reverse('version_feedback_edit', args=[self.plugin_1.package_name, self.version_1.version, self.feedback_1.pk])
+        response = self.client.post(url, {'task': 'updated task'})
+        self.assertEqual(response.status_code, 302)
+        self.assertIn('/accounts/login/', response.url)
+
+    def test_version_feedback_edit_logged_in_no_permission(self):
+        self.user2 = User.objects.create_user(username='otheruser', password='password')
+        self.client.login(username='otheruser', password='password')
+        url = reverse('version_feedback_edit', args=[self.plugin_1.package_name, self.version_1.version, self.feedback_1.pk])
+        response = self.client.post(url, {'task': 'updated task'})
+        self.assertEqual(response.status_code, 401)
+        self.assertJSONEqual(response.content, {'success': False})
+
+    def test_version_feedback_edit_logged_in_with_permission(self):
+        self.client.force_login(user=self.creator)
+        url = reverse('version_feedback_edit', args=[self.plugin_1.package_name, self.version_1.version, self.feedback_1.pk])
+        response = self.client.post(url, {'task': 'updated task'})
+        self.assertEqual(response.status_code, 201)
+        self.feedback_1.refresh_from_db()
+        self.assertEqual(self.feedback_1.task, 'updated task')
+        self.assertIn('modified_on', response.json())
+
+        response_modified_on = response.json()['modified_on']
+        expected_modified_on = self.feedback_1.modified_on.isoformat()
+        self.assertEqual(str(response_modified_on)[:20], expected_modified_on[:20])
+
+
+    def test_version_feedback_edit_invalid_feedback(self):
+        self.client.force_login(user=self.creator)
+        invalid_feedback_id = self.feedback_1.pk + 1
+        url = reverse('version_feedback_edit', args=[self.plugin_1.package_name, self.version_1.version, invalid_feedback_id])
+        response = self.client.post(url, {'task': 'updated task'})
+        self.assertEqual(response.status_code, 404)

qgis-app/plugins/urls.py

@@ -313,11 +313,17 @@
         name="version_feedback_update",
     ),
     url(
-        r"^(?P<package_name>[A-Za-z][A-Za-z0-9-_]+)/version/(?P<version>[^\/]+)/feedback/(?P<feedback>[0-9]+)/$",
+        r"^(?P<package_name>[A-Za-z][A-Za-z0-9-_]+)/version/(?P<version>[^\/]+)/feedback/(?P<feedback>[0-9]+)/delete/$",
         version_feedback_delete,
         {},
         name="version_feedback_delete",
     ),
+    url(
+        r"^(?P<package_name>[A-Za-z][A-Za-z0-9-_]+)/version/(?P<version>[^\/]+)/feedback/(?P<feedback>[0-9]+)/edit/$",
+        version_feedback_edit,
+        {},
+        name="version_feedback_edit",
+    ),
 ]
 
 # RPC

qgis-app/plugins/views.py

@@ -2,6 +2,7 @@
 import copy
 import logging
 import os
+import datetime
 
 from django.conf import settings
 from django.contrib import messages
@@ -1458,6 +1459,29 @@ def version_feedback_update(request, package_name, version):
     return JsonResponse({"success": True}, status=201)
 
 
+@login_required
+@require_POST
+def version_feedback_edit(request, package_name, version, feedback):
+    feedback = get_object_or_404(
+        PluginVersionFeedback,
+        version__plugin__package_name=package_name,
+        version__version=version,
+        pk=feedback
+    )
+    plugin = feedback.version.plugin
+
+    has_update_permission: bool = (
+        request.user in plugin.editors
+        or check_plugin_version_approval_rights(request.user, plugin)
+    )
+    if not has_update_permission:
+        return JsonResponse({"success": False}, status=401)
+    task = request.POST.get('task')
+    feedback.task = str(task)
+    feedback.modified_on = datetime.datetime.now()
+    feedback.save()
+    return JsonResponse({"success": True, "modified_on": feedback.modified_on}, status=201)
+
 @login_required
 @require_POST
 def version_feedback_delete(request, package_name, version, feedback):