[FIX] Correction de bugs (#617)

* Fix #600

* Fix #602

* Fixes & améliorations du nouveau CSS (#616)

* Fix #604

* should fix #605

* Fix #608

* Update core/views/site.py

Co-Authored-By: thomas girod <56346771+imperosol@users.noreply.github.com>

* Added back the permission denied

* Should fix #609

* Fix failing test when 2 user are merged

* Should fix #610

* Should fix #627

* Should fix #109

Block les URLs suivantes lorsque le fichier se trouve dans le dir `profiles` ou `SAS` :
- `/file/<id>/`
- `/file/<id>/[delete|prop|edit]`

> Les urls du SAS restent accessiblent pour les roots & les admins SAS
> Les urls de profiles sont uniquement accessiblent aux roots

* Fix root dir of SAS being unnaccessible for sas admins

⚠️ need to edit the SAS directory & save it (no changes required in sas directory properties)

* Remove overwritten code

* Should fix duplicated albums in user profile (wtf)

* Fix typo

* Extended profiles picture access to board members

* Should fix #607

* Fix keyboard navigation not working properly

* Fix user tagged pictures section inside python rather than in the template

* Update utils.py

* Apply suggested changes

* Fix #604

* Fix #608

* Added back the permission denied

* Should fix duplicated albums in user profile (wtf)

* Fix user tagged pictures section inside python rather than in the template

* Apply suggested changes

---------

Co-authored-by: thomas girod <56346771+imperosol@users.noreply.github.com>

club/models.py

@@ -127,12 +127,22 @@ class Club(models.Model):
     def clean(self):
         self.check_loop()
 
-    def _change_unixname(self, new_name):
+    def _change_unixname(self, old_name, new_name):
         c = Club.objects.filter(unix_name=new_name).first()
         if c is None:
+            # Update all the groups names
+            Group.objects.filter(name=old_name).update(name=new_name)
+            Group.objects.filter(name=old_name + settings.SITH_BOARD_SUFFIX).update(
+                name=new_name + settings.SITH_BOARD_SUFFIX
+            )
+            Group.objects.filter(name=old_name + settings.SITH_MEMBER_SUFFIX).update(
+                name=new_name + settings.SITH_MEMBER_SUFFIX
+            )
+
             if self.home:
                 self.home.name = new_name
                 self.home.save()
+
         else:
             raise ValidationError(_("A club with that unix_name already exists"))
 

core/models.py

@@ -919,6 +919,36 @@ class SithFile(models.Model):
     class Meta:
         verbose_name = _("file")
 
+    def can_be_managed_by(self, user: User) -> bool:
+        """
+        Tell if the user can manage the file (edit, delete, etc.) or not.
+        Apply the following rules:
+            - If the file is not in the SAS nor in the profiles directory, it can be "managed" by anyone -> return True
+            - If the file is in the SAS, only the SAS admins (or roots) can manage it -> return True if the user is in the SAS admin group or is a root
+            - If the file is in the profiles directory, only the roots can manage it -> return True if the user is a root
+
+        :returns: True if the file is managed by the SAS or within the profiles directory, False otherwise
+        """
+
+        # If the file is not in the SAS nor in the profiles directory, it can be "managed" by anyone
+        profiles_dir = SithFile.objects.filter(name="profiles").first()
+        if not self.is_in_sas and not profiles_dir in self.get_parent_list():
+            return True
+
+        # If the file is in the SAS, only the SAS admins (or roots) can manage it
+        if self.is_in_sas and (
+            user.is_in_group(settings.SITH_GROUP_SAS_ADMIN_ID) or user.is_root
+        ):
+            return True
+
+        # If the file is in the profiles directory, only the roots can manage it
+        if profiles_dir in self.get_parent_list() and (
+            user.is_root or user.is_board_member
+        ):
+            return True
+
+        return False
+
     def is_owned_by(self, user):
         if user.is_anonymous:
             return False
@@ -996,7 +1026,7 @@ class SithFile(models.Model):
 
     def save(self, *args, **kwargs):
         sas = SithFile.objects.filter(id=settings.SITH_SAS_ROOT_DIR_ID).first()
-        self.is_in_sas = sas in self.get_parent_list()
+        self.is_in_sas = sas in self.get_parent_list() or self == sas
         copy_rights = False
         if self.id is None:
             copy_rights = True
@@ -1130,12 +1160,6 @@ class SithFile(models.Model):
 
         return Album.objects.filter(id=self.id).first()
 
-    def __str__(self):
-        if self.is_folder:
-            return _("Folder: ") + self.name
-        else:
-            return _("File: ") + self.name
-
     def get_parent_list(self):
         l = []
         p = self.parent

core/static/core/navbar.scss

@@ -130,5 +130,67 @@ nav.navbar {
         }
       }
     }
+
+    > .menu > .head,
+    > .link {
+      color: white;
+      padding: 10px 20px;
+      box-sizing: border-box;
+
+      @media (max-width: 500px) {
+        padding: 10px;
+      }
+    }
+
+    .link:hover,
+    .menu:hover {
+      background-color: rgba(0, 0, 0, .2);
+    }
+
+    > .menu:hover > .content,
+    > .menu > .head:hover + .content,
+    > .menu > .content:hover {
+      display: flex;
+    }
+
+    > .menu {
+      display: flex;
+      position: relative;
+
+      > .content {
+        z-index: 10;
+        display: none;
+        position: absolute;
+        top: 100%;
+        background-color: white;
+        margin: 0;
+        list-style-type: none;
+        width: 130px;
+        box-shadow: 3px 3px 3px 0 #dfdfdf;
+        flex-direction: column;
+
+        @media (max-width: 500px) {
+          position: absolute;
+          flex-direction: row;
+          flex-wrap: wrap;
+          width: 100%;
+          box-shadow: inset 3px 3px 3px 0 #dfdfdf;
+        }
+
+        > li > a {
+          display: flex;
+          padding: 15px 20px;
+
+          @media (max-width: 500px) {
+            padding: 10px;
+          }
+
+          &:hover {
+            color: hsl(203, 75%, 40%);
+            background-color: rgba(0, 0, 0, .05);
+          }
+        }
+      }
+    }
   }  
 }

core/utils.py

@@ -35,11 +35,13 @@ def get_git_revision_short_hash() -> str:
     """
     Return the short hash of the current commit
     """
-    return (
+    try:
-        subprocess.check_output(["git", "rev-parse", "--short", "HEAD"])
+        output = subprocess.check_output(["git", "rev-parse", "--short", "HEAD"])
-        .decode("ascii")
+        if isinstance(output, bytes):
-        .strip()
+            return output.decode("ascii").strip()
-    )
+        return output.strip()
+    except subprocess.CalledProcessError:
+        return ""
 
 
 def get_start_of_semester(d=date.today()):

core/views/files.py

@@ -23,7 +23,7 @@ from django.views.generic.detail import SingleObjectMixin
 from django.forms.models import modelform_factory
 from django.conf import settings
 from django.utils.translation import gettext_lazy as _
-from django.http import HttpResponse
+from django.http import Http404, HttpResponse
 from wsgiref.util import FileWrapper
 from django.urls import reverse
 from django.core.exceptions import PermissionDenied
@@ -34,7 +34,12 @@ import os
 from ajax_select import make_ajax_field
 
 from core.models import SithFile, RealGroup, Notification
-from core.views import CanViewMixin, CanEditMixin, CanEditPropMixin, can_view, not_found
+from core.views import (
+    CanViewMixin,
+    CanEditMixin,
+    CanEditPropMixin,
+    can_view,
+)
 from counter.models import Counter
 
 
@@ -58,6 +63,11 @@ def send_file(request, file_id, file_class=SithFile, file_attr="file"):
         raise PermissionDenied
     name = f.__getattribute__(file_attr).name
     filepath = os.path.join(settings.MEDIA_ROOT, name)
+
+    # check if file exists on disk
+    if not os.path.exists(filepath.encode("utf-8")):
+        raise Http404()
+
     with open(filepath.encode("utf-8"), "rb") as filename:
         wrapper = FileWrapper(filename)
         response = HttpResponse(wrapper, content_type=f.mime_type)
@@ -152,6 +162,13 @@ class FileEditView(CanEditMixin, UpdateView):
     template_name = "core/file_edit.jinja"
     context_object_name = "file"
 
+    def get(self, request, *args, **kwargs):
+        self.object = self.get_object()
+        if not self.object.can_be_managed_by(request.user):
+            raise PermissionDenied
+
+        return super(FileEditView, self).get(request, *args, **kwargs)
+
     def get_form_class(self):
         fields = ["name", "is_moderated"]
         if self.object.is_file:
@@ -197,6 +214,13 @@ class FileEditPropView(CanEditPropMixin, UpdateView):
     context_object_name = "file"
     form_class = FileEditPropForm
 
+    def get(self, request, *args, **kwargs):
+        self.object = self.get_object()
+        if not self.object.can_be_managed_by(request.user):
+            raise PermissionDenied
+
+        return super(FileEditPropView, self).get(request, *args, **kwargs)
+
     def get_form(self, form_class=None):
         form = super(FileEditPropView, self).get_form(form_class)
         form.fields["parent"].queryset = SithFile.objects.filter(is_folder=True)
@@ -269,6 +293,9 @@ class FileView(CanViewMixin, DetailView, FormMixin):
 
     def get(self, request, *args, **kwargs):
         self.form = self.get_form()
+        if not self.object.can_be_managed_by(request.user):
+            raise PermissionDenied
+
         if "clipboard" not in request.session.keys():
             request.session["clipboard"] = []
         return super(FileView, self).get(request, *args, **kwargs)
@@ -316,6 +343,13 @@ class FileDeleteView(CanEditPropMixin, DeleteView):
     template_name = "core/file_delete_confirm.jinja"
     context_object_name = "file"
 
+    def get(self, request, *args, **kwargs):
+        self.object = self.get_object()
+        if not self.object.can_be_managed_by(request.user):
+            raise PermissionDenied
+
+        return super(FileDeleteView, self).get(request, *args, **kwargs)
+
     def get_success_url(self):
         self.object.file.delete()  # Doing it here or overloading delete() is the same, so let's do it here
         if "next" in self.request.GET.keys():

core/views/page.py

@@ -82,6 +82,11 @@ class PageRevView(CanViewMixin, DetailView):
 
     def dispatch(self, request, *args, **kwargs):
         res = super(PageRevView, self).dispatch(request, *args, **kwargs)
+        self.object = self.get_object()
+
+        if self.object is None:
+            return redirect("core:page_create", page_name=self.kwargs["page_name"])
+
         if self.object.need_club_redirection:
             return redirect(
                 "club:club_view_rev", club_id=self.object.club.id, rev_id=kwargs["rev"]

core/views/site.py

@@ -31,6 +31,7 @@ from django.utils import html
 from django.views.generic import ListView, TemplateView
 from django.conf import settings
 from django.utils.text import slugify
+from django.db.models.query import QuerySet
 
 import json
 
@@ -51,12 +52,15 @@ class NotificationList(ListView):
     model = Notification
     template_name = "core/notification_list.jinja"
 
-    def get_queryset(self):
+    def get_queryset(self) -> QuerySet[Notification]:
+        if self.request.user.is_anonymous:
+            return Notification.objects.none()
         # TODO: Bulk update in django 2.2
         if "see_all" in self.request.GET.keys():
             for n in self.request.user.notifications.filter(viewed=False):
                 n.viewed = True
                 n.save()
+
         return self.request.user.notifications.order_by("-date")[:20]
 
 

core/views/user.py

@@ -321,7 +321,7 @@ class UserPicturesView(UserTabsMixin, CanViewMixin, DetailView):
         last_album = None
         for picture in picture_qs:
             album = picture.parent
-            if album.id != last_album:
+            if album.id != last_album and album not in kwargs["albums"]:
                 kwargs["albums"].append(album)
                 kwargs["pictures"][album.id] = []
                 last_album = album.id
@@ -719,8 +719,12 @@ class UserPreferencesView(UserTabsMixin, CanEditMixin, UpdateView):
 
     def get_context_data(self, **kwargs):
         kwargs = super(UserPreferencesView, self).get_context_data(**kwargs)
-        if not hasattr(self.object, "trombi_user"):
+
+        if not (
+            hasattr(self.object, "trombi_user") and self.request.user.trombi_user.trombi
+        ):
             kwargs["trombi_form"] = UserTrombiForm()
+
         if hasattr(self.object, "customer"):
             kwargs["student_card_form"] = StudentCardForm()
         return kwargs

counter/views.py

@@ -583,7 +583,7 @@ class CounterClick(CounterTabsMixin, CanViewMixin, DetailView):
             - <str>, where the string is the code of the product
             - <int>X<str>, where the integer is the quantity and str the code
         """
-        string = parse_qs(request.body.decode())["code"][0].upper()
+        string = parse_qs(request.body.decode()).get("code", [""])[0].upper()
         if string == "FIN":
             return self.finish(request)
         elif string == "ANN":

locale/fr/LC_MESSAGES/django.po

@@ -5569,7 +5569,7 @@ msgid "German"
 msgstr "Allemant"
 
 #: sith/settings.py:449
-msgid "Spanich"
+msgid "Spanish"
 msgstr "Espagnol"
 
 #: sith/settings.py:453

pedagogy/views.py

@@ -206,7 +206,9 @@ class UVListView(CanViewMixin, CanCreateUVFunctionMixin, ListView):
         except TypeError:
             return self.model.objects.none()
 
-        return queryset.filter(id__in=([o.object.id for o in qs]))
+        return queryset.filter(
+            id__in=([o.object.id for o in qs if o.object is not None])
+        )
 
 
 class UVCommentReportCreateView(CanCreateMixin, CreateView):

rootplace/tests.py

@@ -144,11 +144,12 @@ class MergeUserTest(TestCase):
         self.assertTrue(self.to_keep.is_subscribed)
         # to_keep had 5 months of subscription remaining and received
         # 5 more months from to_delete, so he should be subscribed for 10 months
-        self.assertEqual(
+        self.assertAlmostEqual(
             today + timedelta(10 * 30),
             self.to_keep.subscriptions.order_by("subscription_end")
             .last()
             .subscription_end,
+            delta=timedelta(1),
         )
 
     def test_godfathers(self):

sas/templates/sas/picture.jinja

@@ -167,8 +167,10 @@
             switch (e.keyCode) {
                 case 37:
                     $('#prev a')[0].click();
+                    break;
                 case 39:
                     $('#next a')[0].click();
+                    break;
             }
         });
     });

sith/settings.py

@@ -452,7 +452,7 @@ SITH_PEDAGOGY_UV_LANGUAGE = [
     ("FR", _("French")),
     ("EN", _("English")),
     ("DE", _("German")),
-    ("SP", _("Spanich")),
+    ("SP", _("Spanish")),
 ]
 
 SITH_PEDAGOGY_UV_RESULT_GRADE = [

trombi/templates/trombi/detail.jinja

@@ -14,8 +14,8 @@
     <hr>
     <h4>{% trans %}Add user{% endtrans %}</h4>
     <form action="" method="post">
-        {% csrf_token %}
+        {% csrf_token %}
-        {{ form.as_p() }}
+        {{ form.as_p() }}
         <input type="submit" value="{% trans %}Add{% endtrans %}" />
     </form>
     <hr>

trombi/templates/trombi/user_tools.jinja

@@ -38,18 +38,18 @@
         </div>
         <div>{{ u.user.get_display_name() }}</div>
         {% if trombi.show_profiles %}
-        <div>
+            <div>
-            <a href="{{ url("trombi:user_profile", user_id=u.id) }}">{% trans %}Profile{% endtrans %}</a>
+                <a href="{{ url('trombi:user_profile', user_id=u.id) }}">{% trans %}Profile{% endtrans %}</a>
-        </div>
+            </div>
         {% endif %}
         <div>
             {% if can_comment %}
-            {% set comment = u.received_comments.filter(author__id=user.trombi_user.id).first() %}
+                {% set comment = u.received_comments.filter(author__id=user.trombi_user.id).first() %}
-            {% if comment %}
+                {% if comment %}
-            <a href="{{ url("trombi:edit_comment", comment_id=comment.id) }}">{% trans %}Edit comment{% endtrans %}</a>
+                    <a href="{{ url('trombi:edit_comment', comment_id=comment.id) }}">{% trans %}Edit comment{% endtrans %}</a>
-            {% else %}
+                {% else %}
-            <a href="{{ url("trombi:new_comment", user_id=u.id) }}">{% trans %}Comment{% endtrans %}</a>
+                    <a href="{{ url('trombi:new_comment', user_id=u.id) }}">{% trans %}Comment{% endtrans %}</a>
-            {% endif %}
+                {% endif %}
             {% endif %}
         </div>
     </div>

trombi/views.py

@@ -462,6 +462,10 @@ class UserTrombiProfileView(TrombiTabsMixin, DetailView):
 
     def get(self, request, *args, **kwargs):
         self.object = self.get_object()
+
+        if request.user.is_anonymous:
+            raise PermissionDenied()
+
         if (
             self.object.trombi.id != request.user.trombi_user.trombi.id
             or self.object.user.id == request.user.id