Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/code improvements #7

Closed
wants to merge 34 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
108e3b1
QoC:
Badatos May 6, 2024
81dd851
Test with Coveralls report
Badatos May 6, 2024
75eb5f9
QoC
Badatos May 6, 2024
a7e07af
Python 3.8 compatibility
Badatos May 7, 2024
099aae9
PHP Code formatting
Badatos May 7, 2024
a035198
Test with AndreMiras/coveralls-python-action
Badatos May 7, 2024
44aeeee
add missing types
Badatos May 7, 2024
18f307e
add misisng quotes in shell scripts
Badatos May 7, 2024
a4a52cc
try to run coverage without --source (it takes source frome coveragerc)
Badatos May 7, 2024
1d6fcd5
Add Codacy coverage reporter
Badatos May 7, 2024
d63bedf
Ignore *_settings.py in flake8
Badatos May 7, 2024
3573ab1
Omit custom settings files in coverage
Badatos May 13, 2024
94c7e26
Js:
Badatos May 13, 2024
5840680
forgotten files in previous commit
Badatos May 13, 2024
aa4c77a
PHP code formatting
Badatos May 13, 2024
7f1f1b0
code formatting
Badatos May 13, 2024
b9add63
correct lang file
Badatos May 13, 2024
9a71653
Replace remaining innerHTML by textContent in js files
Badatos May 13, 2024
d9f9814
minor remaining code formatting
Badatos May 13, 2024
fea8e30
Delete the docker apt-get lists after each install
Badatos May 14, 2024
9df36eb
add missing quotes
Badatos May 14, 2024
c871f75
declare some `i` vars in JS `for` loops
Badatos May 14, 2024
63db9c9
JS vars declarations
Badatos May 14, 2024
81086fe
:shirt: Remove some linter warnings
Badatos May 14, 2024
ceea3e5
Remove unused JS functions
Badatos May 14, 2024
4bcd37e
Merge branch 'develop' into feature/Code_improvements
Badatos May 14, 2024
b632e7c
Remove merge artifacts
Badatos May 14, 2024
9942990
Merge branch 'develop' into feature/Code_improvements
Badatos May 14, 2024
4a58e2e
Cleanup merge artifacts
Badatos May 14, 2024
6503991
Add unit tests for video apps.py
Badatos May 15, 2024
d803327
Merge branch 'develop' into feature/Code_improvements
Badatos May 15, 2024
eadb791
Flake8
Badatos May 15, 2024
4963956
Disable the "branch" coverage option
Badatos May 15, 2024
13b0914
revert textContent by innerHTML when content contains tags
Badatos May 16, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions .coveragerc
Original file line number Diff line number Diff line change
@@ -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
Expand Down
11 changes: 10 additions & 1 deletion .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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" },
}
]
};
1 change: 0 additions & 1 deletion .github/workflows/code_formatting.yml
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
---
name: Code Formatting
run-name: ${{ github.actor }} is running Pod code formatting 🎨

on:
push:
Expand Down
17 changes: 11 additions & 6 deletions .github/workflows/pod_dev.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand Down
7 changes: 3 additions & 4 deletions .github/workflows/pod_main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Compiled source #
# Compiled source #
###################
*.com
*.class
Expand Down Expand Up @@ -60,6 +60,7 @@ chromedriver
pod/static/
*.bak
.coverage
.cache_ggshield
htmlcov
compile-model

Expand Down
7 changes: 7 additions & 0 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
repos:
- repo: https://github.com/gitguardian/ggshield
rev: v1.27.0
hooks:
- id: ggshield
language_version: python3
stages: [commit]
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 4 additions & 1 deletion dockerfile-dev-with-volumes/pod-back/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
13 changes: 8 additions & 5 deletions dockerfile-dev-with-volumes/pod-encode/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
11 changes: 7 additions & 4 deletions dockerfile-dev-with-volumes/pod-transcript/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
5 changes: 4 additions & 1 deletion dockerfile-dev-with-volumes/pod-xapi/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
15 changes: 9 additions & 6 deletions dockerfile-dev-with-volumes/pod/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
8 changes: 4 additions & 4 deletions pod/authentication/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand All @@ -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")
Expand All @@ -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))
Expand Down Expand Up @@ -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))
Expand Down
11 changes: 6 additions & 5 deletions pod/authentication/apps.py
Original file line number Diff line number Diff line change
@@ -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:
Expand All @@ -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
Expand All @@ -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()

Expand All @@ -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)
4 changes: 2 additions & 2 deletions pod/authentication/backends.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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():
Expand Down
3 changes: 2 additions & 1 deletion pod/authentication/context_processors.py
Original file line number Diff line number Diff line change
@@ -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
10 changes: 5 additions & 5 deletions pod/authentication/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand All @@ -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):
Expand All @@ -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):
Expand All @@ -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).
Expand All @@ -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"])

Expand Down
Loading