[DONE] Code improvements (#1121)

* QoC:
* replace some == by === in JS
* correct some double quotes in shell scripts
* Add some missing types in .py
* Replace `len(xxx.sites.all())` by `xxx.sites.count()`
* Remove unused vars in Php

* Test with Coveralls report

* QoC

* Python 3.8 compatibility

* PHP Code formatting

* Test with AndreMiras/coveralls-python-action

* add missing types
+ replace remaining xml.dom by defusedxml
+ replace some == by === in JS

* add misisng quotes in shell scripts
+ always run apt-get clean AFTER apt-get install
+ use --no-install-recommends on every apt-get install

* try to run coverage without --source (it takes source frome coveragerc)

* Add Codacy coverage reporter

* Ignore *_settings.py in flake8

* Omit custom settings files in coverage
+ generate cobertura.xml coverage report

* Js:
* Tell ESlint to use "module" type for some scripts
* tell parseInt functions to use 10 base
* replace some innerHTML by textContent

+ PHP code formatting

* forgotten files in previous commit

* PHP code formatting

* code formatting

* correct lang file

* Replace remaining innerHTML by textContent in js files
and replace some == by ===

* minor remaining code formatting

* Delete the docker apt-get lists after each install

* add missing quotes

* declare some `i` vars in JS `for` loops

* JS vars declarations

* 👕 Remove some linter warnings

* Remove unused JS functions

* Remove merge artifacts

* Cleanup merge artifacts

* Add unit tests for video apps.py

* Flake8

* Disable the "branch" coverage option

* revert textContent by innerHTML when content contains tags
+ include compiled .mo lang files

.coveragerc

@@ -1,12 +1,19 @@
 
 [report]
+# Add a column to the report with a summary of which lines (and branches) the tests missed.
+# This makes it easy to go from a failure to fixing it, rather than using the HTML report.
+show_missing = True
 
-# nb: you can also add a "# pragma: no cover"
-# on each function you don't want to be covered
 [run]
 relative_files = True
 source = .
+
+# This ensures that your code runs through both the True and False paths of each conditional statement.
+# branch = True
+
 # Here you can exclude a file from coverage testing
+# nb: you can also add a "# pragma: no cover"
+# on each function you don't want to be covered
 omit = pod/*settings*.py
        pod/custom/settings*.py
        pod/*apps.py

.eslintrc.js

@@ -30,6 +30,8 @@ module.exports = {
   },
   /* functions and Objects that will not trigger a "not defined" error. */
   "globals": {
+    "require": true,
+    "process": true,
     "Cookies": "readonly",
     "gettext": "readonly",
     "ngettext": "readonly",
@@ -41,5 +43,12 @@ module.exports = {
     "showalert": "writable",
     "showLoader": "writable",
     "manageDisableBtn": "writable"
-  }
+  },
+  overrides: [
+    {
+      // Allow use of import & export functions
+      files: [ "pod/main/static/js/utils.js", "pod/video/static/js/regroup_videos_by_theme.js" ],
+      parserOptions: { sourceType: "module" },
+    }
+  ]
 };

.github/workflows/code_formatting.yml

@@ -1,6 +1,5 @@
 ---
 name: Code Formatting
-run-name: ${{ github.actor }} is running Pod code formatting 🎨
 
 on:
   push:

.github/workflows/pod_dev.yml

@@ -56,18 +56,17 @@ jobs:
 
       - name: Install Dependencies
         run: |
-          sudo apt-get clean
           sudo apt-get update
-          sudo apt-get install ffmpeg
-          sudo apt-get install -y ffmpegthumbnailer
+          sudo apt-get install -y --no-install-recommends ffmpeg ffmpegthumbnailer
+          sudo apt-get clean
+          sudo rm -rf /var/lib/apt/lists/*
           python -m pip install --upgrade pip
           pip install -r requirements-dev.txt
           sudo npm install -g yarn
 
-      # FLAKE 8
+      # FLAKE 8 (see setup.cfg for configurations)
       - name: Flake8 compliance
-        run: |
-          flake8 --max-complexity=7 --ignore=E501,W503,E203 --exclude .git,pod/*/migrations/*.py,test_settings.py,node_modules/*/*.py,pod/static/*.py,pod/custom/tenants/*/*.py
+        run: flake8
 
       ## Start remote encoding and transcoding test ##
       - name: Create settings local file
@@ -147,6 +146,12 @@ jobs:
           coverage run --append manage.py test -v 3 --settings=pod.main.test_settings
           coverage xml -o cobertura.xml
 
+      - name: Codacy coverage reporter
+        run: bash <(curl -Ls https://coverage.codacy.com/get.sh)
+        continue-on-error: true
+        env:
+          CODACY_PROJECT_TOKEN: ${{ secrets.CODACY_PROJECT_TOKEN }}
+
       - name: Send coverage to coveralls.io
         # coveralls command has been installed by requirements-dev.txt
         env:

.github/workflows/pod_main.yml

@@ -45,11 +45,10 @@ jobs:
 
             - name: Install Dependencies
               run: |
-                sudo apt-get clean
                 sudo apt-get update
-                sudo apt-get install ffmpeg
-                sudo apt-get install -y ffmpegthumbnailer
-                sudo apt-get install -y npm
+                sudo apt-get install -y --no-install-recommends ffmpeg ffmpegthumbnailer npm
+                sudo apt-get clean
+                sudo rm -rf /var/lib/apt/lists/*
                 python -m pip install --upgrade pip
                 pip install -r requirements-dev.txt
                 sudo npm install -g yarn

.gitignore

@@ -1,4 +1,4 @@
-#                       Compiled source #
+# Compiled source #
 ###################
 *.com
 *.class
@@ -60,6 +60,7 @@ chromedriver
 pod/static/
 *.bak
 .coverage
+.cache_ggshield
 htmlcov
 compile-model
 

.pre-commit-config.yaml

@@ -0,0 +1,7 @@
+repos:
+  - repo: https://github.com/gitguardian/ggshield
+    rev: v1.27.0
+    hooks:
+      - id: ggshield
+        language_version: python3
+        stages: [commit]

Makefile

@@ -67,7 +67,7 @@ migrate:
 
 # Launch all unit tests.
 tests:
-	coverage run --source='.' manage.py test --settings=pod.main.test_settings
+	coverage run manage.py test --settings=pod.main.test_settings
 	coverage html
 
 # Ensure coherence of all code style

dockerfile-dev-with-volumes/pod-back/Dockerfile

@@ -19,7 +19,10 @@ FROM $PYTHON_VERSION
 # TODO
 #FROM harbor.urba.univ-lille.fr/store/python:3.7-buster
 
-RUN apt-get clean && apt-get update && apt-get install -y netcat && apt-get install -y gettext
+RUN apt-get update \
+    && apt-get install -y --no-install-recommends netcat gettext \
+    && apt-get clean\
+    && rm -rf /var/lib/apt/lists/*
 
 WORKDIR /usr/src/app
 

dockerfile-dev-with-volumes/pod-encode/Dockerfile

@@ -16,11 +16,14 @@ COPY ./pod/ .
 # TODO
 #FROM harbor.urba.univ-lille.fr/store/python:3.7-buster
 
-RUN apt-get clean && apt-get update \
-    && apt-get install -y netcat \
-                          ffmpeg \
-                          ffmpegthumbnailer \
-                          imagemagick
+RUN apt-get update \
+    && apt-get install -y --no-install-recommends \
+        netcat \
+        ffmpeg \
+        ffmpegthumbnailer \
+        imagemagick \
+    && apt-get clean\
+    && rm -rf /var/lib/apt/lists/*
 
 WORKDIR /usr/src/app
 

dockerfile-dev-with-volumes/pod-transcript/Dockerfile

@@ -16,10 +16,13 @@ COPY ./pod/ .
 # TODO
 #FROM harbor.urba.univ-lille.fr/store/python:3.7-buster
 
-RUN apt-get clean && apt-get update \
-    && apt-get install -y netcat \
-                          sox \
-                          libsox-fmt-mp3
+RUN apt-get update \
+    && apt-get install -y --no-install-recommends \
+        netcat \
+        sox \
+        libsox-fmt-mp3 \
+    && apt-get clean\
+    && rm -rf /var/lib/apt/lists/*
 
 WORKDIR /usr/src/app
 

dockerfile-dev-with-volumes/pod-xapi/Dockerfile

@@ -16,7 +16,10 @@ COPY ./pod/ .
 # TODO
 #FROM harbor.urba.univ-lille.fr/store/python:3.7-buster
 
-RUN apt-get clean && apt-get update && apt-get install -y netcat
+RUN apt-get update \
+    && apt-get install -y --no-install-recommends netcat\
+    && apt-get clean\
+    && rm -rf /var/lib/apt/lists/*
 
 WORKDIR /usr/src/app
 

dockerfile-dev-with-volumes/pod/Dockerfile

@@ -19,12 +19,15 @@ FROM $PYTHON_VERSION
 # TODO
 #FROM harbor.urba.univ-lille.fr/store/python:3.7-buster
 
-RUN apt-get clean && apt-get update \
-    && apt-get install -y netcat \
-                          ffmpeg \
-                          ffmpegthumbnailer \
-                          imagemagick \
-                          gettext
+RUN apt-get update \
+    && apt-get install -y --no-install-recommends \
+        netcat \
+        ffmpeg \
+        ffmpegthumbnailer \
+        imagemagick \
+        gettext \
+    && apt-get clean\
+    && rm -rf /var/lib/apt/lists/*
 
 WORKDIR /usr/src/app
 

pod/authentication/admin.py

@@ -119,7 +119,7 @@ def get_readonly_fields(self, request, obj=None):
         self.readonly_fields += ("is_superuser",)
         return self.readonly_fields
 
-    def owner_hashkey(self, obj):
+    def owner_hashkey(self, obj) -> str:
         return "%s" % Owner.objects.get(user=obj).hashkey
 
     def formfield_for_manytomany(self, db_field, request, **kwargs):
@@ -130,7 +130,7 @@ def formfield_for_manytomany(self, db_field, request, **kwargs):
         kwargs["widget"] = widgets.FilteredSelectMultiple(db_field.verbose_name, False)
         return super().formfield_for_foreignkey(db_field, request, **kwargs)
 
-    def owner_establishment(self, obj):
+    def owner_establishment(self, obj) -> str:
         return "%s" % Owner.objects.get(user=obj).establishment
 
     owner_establishment.short_description = _("Establishment")
@@ -146,7 +146,7 @@ def get_queryset(self, request):
             qs = qs.filter(owner__sites=get_current_site(request))
         return qs
 
-    def save_model(self, request, obj, form, change):
+    def save_model(self, request, obj, form, change) -> None:
         super().save_model(request, obj, form, change)
         if not change:
             obj.owner.sites.add(get_current_site(request))
@@ -174,7 +174,7 @@ def get_queryset(self, request):
             qs = qs.filter(groupsite__sites=get_current_site(request))
         return qs
 
-    def save_model(self, request, obj, form, change):
+    def save_model(self, request, obj, form, change) -> None:
         super().save_model(request, obj, form, change)
         if not change:
             obj.groupsite.sites.add(get_current_site(request))

pod/authentication/apps.py

@@ -1,10 +1,11 @@
+"""Esup-Pod Authentication apps."""
 from django.apps import AppConfig
 from django.db.models.signals import post_migrate
 from django.core.exceptions import ObjectDoesNotExist
 from django.utils.translation import gettext_lazy as _
 
 
-def create_groupsite_if_not_exists(g):
+def create_groupsite_if_not_exists(g) -> None:
     from pod.authentication.models import GroupSite
 
     try:
@@ -13,7 +14,7 @@ def create_groupsite_if_not_exists(g):
         GroupSite.objects.create(group=g)
 
 
-def set_default_site(sender, **kwargs):
+def set_default_site(sender, **kwargs) -> None:
     from pod.authentication.models import Owner
     from django.contrib.sites.models import Site
     from django.contrib.auth.models import Group
@@ -22,11 +23,11 @@ def set_default_site(sender, **kwargs):
     for g in Group.objects.all():
         create_groupsite_if_not_exists(g)
     for gs in GroupSite.objects.all():
-        if len(gs.sites.all()) == 0:
+        if gs.sites.count() == 0:
             gs.sites.add(Site.objects.get_current())
             gs.save()
     for owner in Owner.objects.all():
-        if len(owner.sites.all()) == 0:
+        if owner.sites.count() == 0:
             owner.sites.add(Site.objects.get_current())
             owner.save()
 
@@ -36,5 +37,5 @@ class AuthConfig(AppConfig):
     default_auto_field = "django.db.models.BigAutoField"
     verbose_name = _("Authentication")
 
-    def ready(self):
+    def ready(self) -> None:
         post_migrate.connect(set_default_site, sender=self)

pod/authentication/backends.py

@@ -15,7 +15,7 @@
 CREATE_GROUP_FROM_AFFILIATION = getattr(settings, "CREATE_GROUP_FROM_AFFILIATION", False)
 
 
-def is_staff_affiliation(affiliation):
+def is_staff_affiliation(affiliation) -> bool:
     """Check if user affiliation correspond to AFFILIATION_STAFF."""
     return affiliation in AFFILIATION_STAFF
 
@@ -48,7 +48,7 @@ def authenticate(self, request, remote_user, shib_meta):
             return user if self.user_can_authenticate(user) else None
 
     @staticmethod
-    def update_owner_params(user, params):
+    def update_owner_params(user, params) -> None:
         """Update owner params from Shibboleth."""
         user.owner.auth_type = "Shibboleth"
         if get_current_site(None) not in user.owner.sites.all():

pod/authentication/context_processors.py

@@ -1,9 +1,10 @@
+"""Esup-Pod authentication context_processors."""
 from django.conf import settings as django_settings
 
 SHIB_NAME = getattr(django_settings, "SHIB_NAME", "Identify Federation")
 
 
-def context_authentication_settings(request):
+def context_authentication_settings(request) -> dict:
     new_settings = {}
     new_settings["SHIB_NAME"] = SHIB_NAME
     return new_settings

pod/authentication/forms.py

@@ -15,7 +15,7 @@
 
 
 class OwnerAdminForm(forms.ModelForm):
-    def __init__(self, *args, **kwargs):
+    def __init__(self, *args, **kwargs) -> None:
         super(OwnerAdminForm, self).__init__(*args, **kwargs)
         if __FILEPICKER__:
             self.fields["userpicture"].widget = CustomFileWidget(type="image")
@@ -26,7 +26,7 @@ class Meta(object):
 
 
 class GroupSiteAdminForm(forms.ModelForm):
-    def __init__(self, *args, **kwargs):
+    def __init__(self, *args, **kwargs) -> None:
         super(GroupSiteAdminForm, self).__init__(*args, **kwargs)
 
     class Meta(object):
@@ -52,7 +52,7 @@ class Meta(object):
 class SetNotificationForm(forms.ModelForm):
     """Push notification preferences form."""
 
-    def __init__(self, *args, **kwargs):
+    def __init__(self, *args, **kwargs) -> None:
         super(SetNotificationForm, self).__init__(*args, **kwargs)
 
     class Meta(object):
@@ -79,7 +79,7 @@ class Meta:
         fields = "__all__"
         exclude = []
 
-    def __init__(self, *args, **kwargs):
+    def __init__(self, *args, **kwargs) -> None:
         # Do the normal form initialisation.
         super(GroupAdminForm, self).__init__(*args, **kwargs)
         # If it is an existing group (saved objects have a pk).
@@ -90,7 +90,7 @@ def __init__(self, *args, **kwargs):
             owner__sites=Site.objects.get_current()
         )
 
-    def save_m2m(self):
+    def save_m2m(self) -> None:
         # Add the users to the Group.
         self.instance.user_set.set(self.cleaned_data["users"])