Merge branch 'update-currator-doc-and-experience' into 'master'

update curator permissions

See merge request !90

doc/permissions.rst

@@ -74,14 +74,17 @@ Curator
 A curator can perform changes on **public** data sources even if he/she is not an editor of the data source. The mission
 of a curator is to keep naming between viruses, hosts coherent, but also edit data source details when needed.
 
- * Edit the list of :term:`viruses <virus>` and :term:`hosts <host>`
+ * Edit the :term:`viruses <virus>` and :term:`hosts <host>`
  * Edit the information of a :term:`data source` (except the the list of viewer/editor)
+ * Edit the mapping
 
 Although, an curator cannot:
 
- * See the responses, and thus cannot edit them either
- * Edit the mapping
+ * Edit the responses
+ * Delete :term:`viruses <virus>` or :term:`hosts <host>`
  * See the list of viewer/editor, and thus cannot edit it either
+ * Restore the data source to a previous version
+ * Download a previous version
  * Delete the data source
  * Transfer ownership to an other user
 

src/viralhostrange/viralhostrangedb/static/css/base.css

@@ -170,4 +170,7 @@ a[data-toggle=collapse].card-header  {
 }
 a[data-toggle=collapse].card-header>:not(i)  {
     color: #212529;
+}
+.not-allowed {
+     cursor: not-allowed !important;
 }
\ No newline at end of file

src/viralhostrange/viralhostrangedb/templates/viralhostrangedb/datasource_detail.html

@@ -150,19 +150,11 @@
                 </tbody>
             </table>
             <div class="btn-group  d-flex" role="group" aria-label="Get content">
-                {% if request|can_edit:object %}
-                <a href="{% url 'viralhostrangedb:data-source-data-update' pk=object.pk%}" role="button"
-                   class="btn btn-outline-primary"><i class="fa fa-upload"></i> {%trans "Update content" %}</a>
-                {%endif%}
                 {% if has_responses %}
                 <a href="{% url 'viralhostrangedb:data-source-download' pk=object.pk%}" role="button"
                    class="float-right btn btn-outline-primary">
                     <i class="fa fa-download"></i> {%trans "Download it" %}
                 </a>
-                <a href="http://hub.pages.pasteur.fr/viralhostrangedb/compatible_file.html" target="_blank" style="flex-grow:0"
-                   class="float-right btn btn-outline-primary">
-                    <i class="fa fa-question-circle"></i>
-                </a>
                 {%else%}
                 <a href="{% url 'viralhostrangedb:data-source-download' pk=object.pk%}" role="button"
                    class="float-right btn btn-primary">
@@ -173,6 +165,14 @@
                     <i class="fa fa-question-circle"></i> {%trans "How to fill it" %}
                 </a>
                 {%endif%}
+                {% if request|is_editor_or_owner_of_ds:object %}
+                <a href="{% url 'viralhostrangedb:data-source-data-update' pk=object.pk%}" role="button"
+                   class="btn btn-outline-primary"><i class="fa fa-upload"></i> {%trans "Update content" %}</a>
+                <a href="http://hub.pages.pasteur.fr/viralhostrangedb/compatible_file.html" target="_blank" style="flex-grow:0"
+                   class="float-right btn btn-outline-primary">
+                    <i class="fa fa-question-circle"></i>
+                </a>
+                {%endif%}
             </div>
 
         </div>
@@ -221,7 +221,7 @@
     <div class="card">
         <div class="card-header">
             {%trans "Viruses"%}
-            {% if request|can_edit:object %}
+            {% if request|is_editor_or_owner_of_ds:object %}
             <a class="btn btn-xs btn-outline-secondary float-right"
                href="{% url 'viralhostrangedb:data-source-virus-delete' pk=object.pk%}"
                role="button"
@@ -229,6 +229,8 @@
             >
                 <i class="fa fa-trash"></i> {%trans "Delete ..." %}
             </a>
+            {%endif%}
+            {% if request|can_edit:object %}
             <a class="btn btn-xs btn-outline-secondary float-right"
                href="{% url 'viralhostrangedb:data-source-virus-update' pk=object.pk%}"
                role="button"
@@ -249,7 +251,7 @@
     <div class="card">
         <div class="card-header">
             {%trans "Hosts"%}
-            {% if request|can_edit:object %}
+            {% if request|is_editor_or_owner_of_ds:object %}
             <a class="btn btn-xs btn-outline-secondary float-right"
                href="{% url 'viralhostrangedb:data-source-host-delete' pk=object.pk%}"
                role="button"
@@ -257,6 +259,8 @@
             >
                 <i class="fa fa-trash"></i> {%trans "Delete ..." %}
             </a>
+            {%endif%}
+            {% if request|can_edit:object %}
             <a class="btn btn-xs btn-outline-secondary float-right"
                href="{% url 'viralhostrangedb:data-source-host-update' pk=object.pk%}"
                role="button"

src/viralhostrange/viralhostrangedb/templates/viralhostrangedb/datasource_history.html

@@ -29,20 +29,33 @@
             <td>{{o.action_time}}</td>
             <td>{{o.user.last_name|upper}} {{o.user.first_name|title}}</td>
             <td>{{o|get_change_message_with_action}}</td>
+            {%with request|is_editor_or_owner_of_ds:o.object_id as can_restore %}
             <td class="text-center">
                 {% if o|should_have_backup_file %}
                     {% if o|has_backup_file %}
                         {% if forloop.first %}
                             <i>{%trans 'Current version' %}</i>
                         {% else %}
+                            {%if can_restore or o.user_id is request.user.id%}
                             <a href="{% url 'viralhostrangedb:data-source-history-download' pk=o.object_id log_pk=o.pk%}"
                                class="btn btn-xs btn-outline-primary">
                                 <i class="fa fa-download" aria-hidden="true"></i> {%trans 'Download saved data'%}
                             </a>
+                            {%else%}
+                            <button disabled="disabled" class="btn btn-xs btn-outline-primary disabled not-allowed">
+                                <i class="fa fa-download" aria-hidden="true"></i> {%trans 'Download saved data'%}
+                            </button>
+                            {%endif%}
+                            {%if can_restore%}
                             <a href="{% url 'viralhostrangedb:data-source-history-restoration' pk=o.object_id log_pk=o.pk%}"
                                class="btn btn-xs btn-outline-danger">
                                 <i class="fa fa-undo" aria-hidden="true"></i> {%trans 'Restore data at this point' %}
                             </a>
+                           {%else%}
+                            <button disabled="disabled" class="btn btn-xs btn-outline-danger disabled not-allowed">
+                                <i class="fa fa-undo" aria-hidden="true"></i> {%trans 'Restore data at this point'%}
+                            </button>
+                            {%endif%}
                         {%endif%}
                     {% else %}
                         <i>{%trans 'Backup missing' %}</i>
@@ -56,6 +69,7 @@
                 <i>{%trans 'No backup created' %}</i>
                 {%endif%}
             </td>
+            {%endwith %}
         </tr>
         {%endfor%}
         </tbody>

src/viralhostrange/viralhostrangedb/templatetags/viralhostrange_tags.py

@@ -68,6 +68,28 @@ def can_edit(user, obj):
     return False
 
 
+@register.filter
+def is_editor_or_owner_of_ds(user, obj_pk):
+    # used to access datasource history, should follow get_log_entry_with_permission_check_or_404
+    # if user is a WSGIRequest, get the attr user
+    if hasattr(obj_pk,'pk'):
+        obj_pk=obj_pk.pk
+    user = getattr(user, "user", user)
+    if not user.is_authenticated:
+        return False
+    try:
+        return mixins.only_editor_or_owned_queryset_filter(
+            self=None,
+            request=None,
+            queryset=models.DataSource.objects.filter(pk=obj_pk),
+            user=user,
+        ).exists()
+    except Exception as e:
+        if settings.DEBUG:
+            raise e
+    return False
+
+
 @register.filter
 def get_curators_list(_):
     return business_process.get_curators().order_by("last_name", "first_name")

src/viralhostrange/viralhostrangedb/tests/base_test_case.py

@@ -85,6 +85,15 @@ class OneDataSourceTestCase(TestCase):
         )
         Group.objects.get(name="Admin").user_set.add(self.staff_admin)
 
+        ################################################################################
+        self.u_curator = get_user_model().objects.create(
+            username="u_curator",
+            first_name="curatorF",
+            last_name="curatorL",
+            email="u_curator@a.b",
+        )
+        business_process.set_curator(self.u_curator, True)
+
         ################################################################################
         self.user = get_user_model().objects.create(
             username="user",

src/viralhostrange/viralhostrangedb/tests/test_changes_notifications.py

@@ -38,13 +38,13 @@ class DataSourceChangeNotificationTestCase(base_test_case.OneDataSourceTestCase)
             assert e.errno == errno.EEXIST, e
         super().setUp()
         ################################################################################
-        self.u_curator = get_user_model().objects.create(
-            username="u_curator",
-            first_name="Dany",
-            last_name="u_curatorL",
-            email="u_curator@a.b",
-        )
-        business_process.set_curator(self.u_curator, True)
+        # self.u_curator = get_user_model().objects.create(
+        #     username="u_curator",
+        #     first_name="Dany",
+        #     last_name="u_curatorL",
+        #     email="u_curator@a.b",
+        # )
+        # business_process.set_curator(self.u_curator, True)
         self.ds.public = True
         self.ds.save()
 

src/viralhostrange/viralhostrangedb/tests/test_history.py

@@ -62,6 +62,20 @@ class DataSourceHistoryTestCase(base_test_case.OneDataSourceTestCase):
         self.assertEqual(self.client.get(url_restore).status_code, 200)
         self.assertEqual(self.client.post(url_restore, dict(agree="True")).status_code, 302)
 
+        self.client.force_login(self.u_curator)
+        self.ds.public = True
+        self.ds.save()
+        self.assertEqual(self.client.get(url_see).status_code, 200)
+        self.assertEqual(self.client.get(url_get).status_code, 404)
+        self.assertEqual(self.client.get(url_restore).status_code, 404)
+        self.assertEqual(self.client.post(url_restore, dict(agree="True")).status_code, 404)
+        self.ds.public = False
+        self.ds.save()
+        self.assertEqual(self.client.get(url_see).status_code, 404)
+        self.assertEqual(self.client.get(url_get).status_code, 404)
+        self.assertEqual(self.client.get(url_restore).status_code, 404)
+        self.assertEqual(self.client.post(url_restore, dict(agree="True")).status_code, 404)
+
         self.client.force_login(self.user)
         self.assertEqual(self.client.get(url_see).status_code, 200)
         self.assertEqual(self.client.get(url_get).status_code, 200)

src/viralhostrange/viralhostrangedb/tests/test_templatetags.py

@@ -4,7 +4,7 @@ from django.test import TestCase, override_settings
 from django.utils import timezone
 
 from viralhostrangedb import models, business_process
-from viralhostrangedb.templatetags.viralhostrange_tags import can_edit, can_see, can_delete
+from viralhostrangedb.templatetags.viralhostrange_tags import can_edit, can_see, can_delete, is_editor_or_owner_of_ds
 
 
 class TemplateTagsTests(TestCase):
@@ -84,6 +84,21 @@ class TemplateTagsTests(TestCase):
         business_process.set_curator(self.user, True)
         self.assertTrue(can_edit(self.user, self.d_toto))
 
+    @override_settings(DEBUG=False)
+    def test_is_editor_or_owner_of_ds(self):
+        self.assertFalse(is_editor_or_owner_of_ds(self.user, self.d_toto))
+        self.assertTrue(is_editor_or_owner_of_ds(self.toto, self.d_toto))
+
+        self.assertTrue(is_editor_or_owner_of_ds(self.titi, self.d_toto_grant_titi))
+        business_process.set_curator(self.titi, True)
+        self.assertTrue(is_editor_or_owner_of_ds(self.titi, self.d_toto_grant_titi), "should still be allowed")
+
+        self.assertFalse(is_editor_or_owner_of_ds(self.user, self.d_toto_grant_titi))
+        self.assertFalse(is_editor_or_owner_of_ds(self.user, self.d_toto_grant_titi.pk))
+        business_process.set_curator(self.user, True)
+        self.assertFalse(is_editor_or_owner_of_ds(self.user, self.d_toto_grant_titi), "still not allowed")
+        self.assertFalse(is_editor_or_owner_of_ds(self.user, self.d_toto_grant_titi.pk), "still not allowed")
+
     @override_settings(DEBUG=False)
     def test_can_delete(self):
         self.assertFalse(can_delete(self.user, self.d_toto))

src/viralhostrange/viralhostrangedb/views.py

@@ -421,7 +421,8 @@ def data_source_history_list(request, pk):
     )
 
 
-def get_log_entry_with_permission_check_or_404(request, pk, log_pk):
+@login_required
+def get_log_entry_with_permission_check_or_404(request, pk, log_pk, allow_curator=False, user_id=None):
     if request.user.is_superuser and not models.DataSource.objects.filter(pk=pk).exists():
         # Allows superuser to access to back up of deleted data source after they have been deleted
         data_source = None
@@ -430,15 +431,31 @@ def get_log_entry_with_permission_check_or_404(request, pk, log_pk):
             self=None,
             request=request,
             queryset=models.DataSource.objects,
+            allow_curator=allow_curator,
         ), pk=pk)
+    log_entries = LogEntry.objects.filter(object_id=pk,
+                                          content_type=ContentType.objects.get_for_model(models.DataSource))
+    if user_id:
+        log_entries = log_entries.filter(user_id=user_id)
     return data_source, get_object_or_404(
-        LogEntry.objects.filter(object_id=pk, content_type=ContentType.objects.get_for_model(models.DataSource)),
+        log_entries,
         pk=log_pk,
     )
 
 
+@login_required
 def data_source_history_download(request, pk, log_pk):
-    data_source, le = get_log_entry_with_permission_check_or_404(request, pk, log_pk)
+    try:
+        data_source, le = get_log_entry_with_permission_check_or_404(request, pk, log_pk)
+    except Http404:
+        # if we cannot get it, then allows curators, but only on their own entry
+        data_source, le = get_log_entry_with_permission_check_or_404(
+            request,
+            pk,
+            log_pk,
+            allow_curator=True,
+            user_id=request.user.id,
+        )
     file_path = business_process.get_backup_file_path(le, test_if_exists=True)
 
     if file_path is None: