diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 9871861dda8d58c96ff083a9bbaac9459d7bfab4..fb8ab5d9ed82795c9b5804f6895243cf440b9dcc 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -43,6 +43,12 @@ variables: when: manual +# Do not run this job if it’s running on a branch different from the main branch +.do-not-run-if-not-on-main-branch: + rules: + - if: $CI_COMMIT_BRANCH != $CI_DEFAULT_BRANCH + when: never + # A job that builds an image using buildah should inherit from this template .job-on-buildah: image: quay.io/gnome_infrastructure/buildah @@ -163,6 +169,7 @@ build_container:production: stage: build rules: - !reference [ .do-not-run-on-po-change, rules ] + - !reference [.do-not-run-if-not-on-main-branch, rules] - if: $CI_COMMIT_BRANCH == $CI_DEFAULT_BRANCH when: always - when: never @@ -234,7 +241,7 @@ static-analysis:pre-commit: - git config --global --add safe.directory "*" - pre-commit run --all-files -static-analysis:prospector: +static-analysis:prospector-all: extends: - .static-analysis - .job-on-runtime-image @@ -245,12 +252,29 @@ static-analysis:prospector: artifacts: name: "prospector-${CI_JOB_NAME}-${CI_COMMIT_REF_NAME}-${CI_COMMIT_SHA}" expire_in: 1 week - expose_as: "Prospector Analysis" + expose_as: "Prospector Analysis - all files" paths: - prospector.txt when: always -static-analysis:ruff: + +static-analysis:prospector-no-fail: + extends: + - .static-analysis + - .job-on-runtime-image + script: + - pip install ".[dev]" + - prospector . -I stats -I vertimus -I containers -I common | tee prospector-no-fail.txt + artifacts: + name: "prospector-${CI_JOB_NAME}-${CI_COMMIT_REF_NAME}-${CI_COMMIT_SHA}" + expire_in: 1 week + expose_as: "Prospector Analysis" + paths: + - prospector-no-fail.txt + when: always + + +static-analysis:ruff-all: extends: - .static-analysis - .job-on-runtime-image @@ -261,11 +285,27 @@ static-analysis:ruff: artifacts: name: "ruff-${CI_JOB_NAME}-${CI_COMMIT_REF_NAME}-${CI_COMMIT_SHA}" expire_in: 1 week - expose_as: "Ruff Analysis" + expose_as: "Ruff Analysis - all files" paths: - ruff.txt when: always + +static-analysis:ruff-no-fail: + extends: + - .static-analysis + - .job-on-runtime-image + script: + - pip install ".[dev]" + - ruff check api languages feeds people teams | tee ruff-no-fail.txt + artifacts: + name: "ruff-${CI_JOB_NAME}-${CI_COMMIT_REF_NAME}-${CI_COMMIT_SHA}" + expire_in: 1 week + expose_as: "Ruff Analysis" + paths: + - ruff-no-fail.txt + when: always + ############################################################################# ################### Deploy ################### ############################################################################# @@ -294,6 +334,7 @@ deploy:staging: deployment_tier: staging rules: - !reference [.do-not-run-on-po-change, rules] + - !reference [.do-not-run-if-not-on-main-branch, rules] - if: $CI_COMMIT_BRANCH == $CI_DEFAULT_BRANCH when: on_success @@ -312,6 +353,7 @@ deploy:production: deployment_tier: production rules: - !reference [ .do-not-run-on-po-change, rules ] + - !reference [.do-not-run-if-not-on-main-branch, rules] - if: $CI_COMMIT_BRANCH == $CI_DEFAULT_BRANCH when: manual - when: never diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 768a15d53980d6cedde514004cd9d7e6d6e2e389..423509191c21166bd6bbbe081238b880e28d893b 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -11,13 +11,6 @@ repos: - id: check-docstring-first - id: check-merge-conflict - - repo: https://github.com/astral-sh/ruff-pre-commit - rev: v0.1.7 - hooks: - # - id: ruff - # args: [ --fix ] - - id: ruff-format - - repo: https://github.com/rtts/djhtml rev: '3.0.6' hooks: @@ -31,6 +24,16 @@ repos: # Exclude JavaScript files in vendor directories exclude: .*.min.js + - repo: local + hooks: + - id: ruff_check + name: "ruff (packages: api, languages, feeds people and teams)" + always_run: true + entry: ruff --force-exclude api languages feeds people teams + types_or: [python, pyi] + pass_filenames: false + language: system + - repo: local hooks: - id: pofile_check diff --git a/.prospector.yml b/.prospector.yml index 000b8ca62125e35f409fef48fd9e272fbee1635c..fb35790a224b13e7c03ee959e70b11891ad1fe36 100644 --- a/.prospector.yml +++ b/.prospector.yml @@ -1,7 +1,7 @@ inherits: - - default - - strictness_high - - full_pep8 + - default + - strictness_high + - full_pep8 output-format: grouped test-warnings: false @@ -11,6 +11,7 @@ max-line-length: 119 ignore-paths: - docs - damnedlies + - build pylint: options: diff --git a/api/tests.py b/api/tests.py index f4d5b9e95abb19949833da3ed7ffe6ffd4b0a183..9ca88556982a7594178e0a9007fecae37c64e3db 100644 --- a/api/tests.py +++ b/api/tests.py @@ -15,7 +15,7 @@ from vertimus.views import get_vertimus_state class APITests(TestCase): - fixtures = ["test_data.json"] + fixtures = ("test_data.json",) def test_home(self): response = self.client.get(reverse("api-v1-home")) @@ -163,7 +163,7 @@ class APITests(TestCase): class APIUploadTests(TestCase): - fixtures = ["test_data.json"] + fixtures = ("test_data.json",) @classmethod def setUpTestData(cls): @@ -176,7 +176,8 @@ class APIUploadTests(TestCase): cls.url = reverse("api-v1-upload-file", args=["gnome-hello", "master", "po", "fr"]) cls.test_po = Path(__file__).parent.parent / "stats" / "tests" / "test.po" - def _get_module_state(self): + @staticmethod + def _get_module_state(): """Create basic data ready for testing file upload.""" _, _, state = get_vertimus_state( Branch.objects.get(module__name="gnome-hello"), diff --git a/api/urls.py b/api/urls.py index 3ae54797eefff17d0804e76541801554f1dee599..6fdb4f2ad7d501ed0f37c0ab7f502cc633be534b 100644 --- a/api/urls.py +++ b/api/urls.py @@ -26,7 +26,7 @@ urlpatterns = [ path("releases/", views.ReleaseView.as_view(), name="api-v1-info-release"), path("releases//stats", views.ReleaseStatsView.as_view(), name="api-v1-info-release-stats"), path( - "releases//languages/", + "releases//languages/", views.ReleaseLanguageView.as_view(), name="api-v1-info-release-language", ), diff --git a/api/views.py b/api/views.py index 3c0443968deddb600a12c111044f49dea2f58988..579d07c283047f359d1d304e762b1348cca1c4ed 100644 --- a/api/views.py +++ b/api/views.py @@ -1,7 +1,9 @@ import abc import traceback from abc import ABC +from collections.abc import Iterable from threading import Thread +from typing import TYPE_CHECKING, Any from django.contrib.auth.mixins import LoginRequiredMixin from django.core.mail import mail_admins @@ -21,58 +23,78 @@ from vertimus.forms import ActionForm from vertimus.models import ActionRT, ActionUT from vertimus.views import get_vertimus_state +if TYPE_CHECKING: + from django.http import HttpRequest + + from vertimus.models import State + class SerializeListView(View, ABC): - fields = [] + fields = tuple() @abc.abstractmethod - def get_href(self, obj): + def get_href(self: "SerializeListView", serialized_instance: dict[str, Any]) -> str: raise NotImplementedError() @abc.abstractmethod - def get_queryset(self): + def get_queryset(self: "SerializeListView") -> QuerySet: raise NotImplementedError() - def get(self, request, *args, **kwargs): - return JsonResponse(self.serialize_queryset(self.get_queryset().values(*["pk"] + self.fields)), safe=False) - - def serialize_object(self, obj, fields=None): - data = {field: obj[field] for field in fields} - data["href"] = self.get_href(obj) - return data + def get(self: "SerializeListView", *args, **kwargs) -> JsonResponse: # noqa: ANN002, ANN003, ARG002 + return JsonResponse(self.__serialize_queryset(self.get_queryset()), safe=False) + + def __serialize_queryset(self: "SerializeListView", queryset: "QuerySet") -> list[dict[str, str]]: + """ + Serialize a full QuerySet by calling serialize_object() of each of its item. + """ + return [ + self.__add_href_to_serialized_object(object_as_dict) + # We extract the values of the objects in the queryset to keep only + # some of the fields. + for object_as_dict in queryset.values(*self.fields) + ] - def serialize_queryset(self, queryset: "QuerySet"): - return [self.serialize_object(object, self.fields) for object in queryset] + def __add_href_to_serialized_object(self: "SerializeListView", object_as_dict: dict[str, Any]) -> dict[str, str]: + """ + Adds a ‘href’ key in order to link each element to its fully qualified URL. + """ + object_as_dict["href"] = self.get_href(object_as_dict) + return object_as_dict class SerializeObjectView(View, ABC): - fields = [] + fields = tuple() @abc.abstractmethod - def get_object(self): + def get_object(self: "SerializeObjectView") -> object: raise NotImplementedError() - def get(self, *args, **kwargs): + def get(self: "SerializeObjectView", *args, **kwargs) -> JsonResponse: # noqa: ARG002, ANN002, ANN003 return JsonResponse(self.serialize_object(self.get_object())) @abc.abstractmethod - def serialize_object(self, obj, fields=None): - return self.serialize_instance(obj, fields or self.fields) + def serialize_object(self: "SerializeObjectView", object_to_serialize: object) -> dict[str, Any]: + return self._serialize_instance(object_to_serialize, self.fields) @staticmethod - def serialize_instance(instance, field_names: list[str]): - data = {} - for field in field_names: + def _serialize_instance(instance: object, fields: Iterable[str]) -> dict[str, Any]: + serialized_instance = {} + for field in fields: value = getattr(instance, field) + # The object is a QuerySet, so all the elements should be retrieved. if hasattr(value, "all"): value = [str(o) for o in value.all()] - data[field] = value - return data + serialized_instance[field] = value + return serialized_instance # noinspection PyMethodMayBeStatic class APIHomeView(View): - def get(self, *args, **kwargs): + """ + List available API endpoints. + """ + + def get(self: "APIHomeView", *args, **kwargs) -> JsonResponse: # noqa: PLR6301, ARG002, ANN002, ANN003 return JsonResponse({ "modules": reverse("api-v1-list-modules"), "teams": reverse("api-v1-list-teams"), @@ -83,143 +105,171 @@ class APIHomeView(View): # noinspection PyMethodMayBeStatic class ModulesView(SerializeListView): - fields = ["name"] + """ + List all available and active modules. + """ - def get_queryset(self): + fields = ("name",) + + def get_queryset(self: "ModulesView") -> QuerySet[Module]: # noqa: PLR6301 return Module.objects.filter(archived=False) - def get_href(self, obj): - return reverse("api-v1-info-module", args=[obj["name"]]) + def get_href(self: "ModulesView", serialized_module: dict[str, Any]) -> str: # noqa: PLR6301 + return reverse("api-v1-info-module", kwargs={"module_name": serialized_module.get("name")}) class ModuleView(SerializeObjectView): @property - def fields(self): - return [field.name for field in Module._meta.get_fields() if not field.one_to_many and field.name != "id"] - - def get_object(self): + def fields(self: "ModuleView") -> list[str]: + """ + List all the fields of a module, excluding the ones involved in relation to other models and + private attributes. + """ + return [str(field.name) for field in Module._meta.get_fields() if not field.one_to_many and field.name != "id"] + + def get_object(self: "ModuleView") -> Module: return get_object_or_404(Module, name=self.kwargs["module_name"]) - def serialize_object(self, module: "Module", **kwargs): - data = self.serialize_instance(module, self.fields) - data["branches"] = [self.serialize_instance(branch, ["name"]) for branch in module.get_branches()] - data["domains"] = [ - self.serialize_instance(domain, ["name", "description", "dtype", "layout"]) - for domain in module.domain_set.all().order_by("name") + def serialize_object(self: "ModuleView", object_to_serialize: Module) -> dict[str, list | Any]: + serialized_module = self._serialize_instance(object_to_serialize, self.fields) + serialized_module["branches"] = [ + self._serialize_instance(branch, ["name"]) for branch in object_to_serialize.get_branches() ] - return data + serialized_module["domains"] = [ + self._serialize_instance(domain, ["name", "description", "dtype", "layout"]) + for domain in object_to_serialize.domain_set.all().order_by("name") + ] + return serialized_module class ModuleBranchView(SerializeObjectView): - def get_object(self): + def get_object(self: "ModuleBranchView") -> Branch: return get_object_or_404( Branch.objects.select_related("module"), module__name=self.kwargs["module_name"], name=self.kwargs["branch_name"], ) - def serialize_object(self, branch: "Branch", **kwargs): - data = { - "module": branch.module.name, - "branch": branch.name, + def serialize_object(self: "ModuleBranchView", object_to_serialize: "Branch") -> dict[str, Any]: # noqa: PLR6301 + serialized_branch = { + "module": object_to_serialize.module.name, + "branch": object_to_serialize.name, "domains": [], } - for dom_name in branch.connected_domains.keys(): - stats = {} - for stat in Statistics.objects.filter(branch=branch, domain__name=dom_name, language__isnull=False): - stats[stat.language.locale] = { - "trans": stat.translated(), - "fuzzy": stat.fuzzy(), - "untrans": stat.untranslated(), + for domain_name in object_to_serialize.connected_domains.keys(): + serialized_statistics = {} + statistics_for_domain_on_branch_with_language = Statistics.objects.filter( + branch=object_to_serialize, domain__name=domain_name, language__isnull=False + ) + for statistic in statistics_for_domain_on_branch_with_language: + serialized_statistics[statistic.language.locale] = { + "trans": statistic.translated(), + "fuzzy": statistic.fuzzy(), + "untrans": statistic.untranslated(), } - data["domains"].append({"name": dom_name, "statistics": stats}) - return data + serialized_branch["domains"].append({"name": domain_name, "statistics": serialized_statistics}) + return serialized_branch # noinspection PyMethodMayBeStatic class TeamsView(SerializeListView): - fields = ["name", "description"] + fields = ("name", "description") - def get_queryset(self): + def get_queryset(self: "TeamsView") -> QuerySet: # noqa: PLR6301 return Team.objects.all().order_by("name") - def get_href(self, obj): - return reverse("api-v1-info-team", args=[obj["name"]]) + def get_href(self: "TeamsView", serialized_team: dict[str, Any]) -> str: # noqa: PLR6301 + return reverse("api-v1-info-team", kwargs={"team_name": serialized_team.get("name")}) class TeamView(SerializeObjectView): @property - def fields(self): - return [f.name for f in Team._meta.get_fields() if not f.one_to_many and f.name not in ["id", "members"]] + def fields(self: "TeamView") -> list[str]: + """ + List all the fields that should be displayed about a team, without many-to-many relations and team members. + """ + return [str(f.name) for f in Team._meta.get_fields() if not f.one_to_many and f.name not in {"id", "members"}] - def get_object(self): + def get_object(self: "TeamView") -> Team: return get_object_or_404(Team, name=self.kwargs["team_name"]) - def serialize_object(self, team, **kwargs): - data = self.serialize_instance(team, self.fields) - data["coordinators"] = [self.serialize_instance(coord, ["name"]) for coord in team.get_coordinators()] + def serialize_object(self: "TeamView", object_to_serialize: Team) -> dict[str, Any]: + data = self._serialize_instance(object_to_serialize, self.fields) + data["coordinators"] = [ + self._serialize_instance(coord, ["name"]) for coord in object_to_serialize.get_coordinators() + ] return data # noinspection PyMethodMayBeStatic class LanguagesView(SerializeListView): - fields = ["name", "locale", "team__description", "plurals"] - - def get_queryset(self): + fields = ( + "name", + "locale", + "team__description", + "plurals", + ) + + def get_queryset(self: "LanguagesView") -> QuerySet[Language]: # noqa: PLR6301 return Language.objects.all().order_by("name") - def get_href(self, obj): - return reverse("api-v1-info-team", args=[obj["locale"]]) + def get_href(self: "LanguagesView", serialized_language: dict[str, Any]) -> str: # noqa: PLR6301 + return reverse("api-v1-info-team", kwargs={"team_name": serialized_language.get("locale")}) # noinspection PyMethodMayBeStatic class ReleasesView(SerializeListView): - fields = ["name", "description"] + fields = ("name", "description") - def get_queryset(self): + def get_queryset(self: "ReleasesView") -> Release: # noqa: PLR6301 return Release.objects.all().order_by("name") - def get_href(self, obj): - return reverse("api-v1-info-release", args=[obj["name"]]) + def get_href(self: "ReleasesView", serialized_release: dict[str, Any]) -> str: # noqa: PLR6301 + return reverse("api-v1-info-release", kwargs={"release": serialized_release.get("name")}) class ReleaseView(SerializeObjectView): - fields = ["name", "description", "string_frozen", "status", "branches"] + fields = ("name", "description", "string_frozen", "status", "branches") - def get_object(self): + def get_object(self: "ReleaseView") -> Release: return get_object_or_404(Release, name=self.kwargs["release"]) - def serialize_object(self, release_set, **kwargs): - data = self.serialize_instance(release_set, self.fields) - data["languages"] = [ + def serialize_object(self: "ReleaseView", object_to_serialize: Release) -> dict[str, Any]: + serialized_release = self._serialize_instance(object_to_serialize, self.fields) + serialized_release["languages"] = [ { - "locale": lang.locale, - "href": reverse("api-v1-info-release-language", args=[release_set.name, lang.locale]), + "locale": language.locale, + "href": reverse( + "api-v1-info-release-language", + kwargs={"release": object_to_serialize.name, "locale": language.locale}, + ), } - for lang in Language.objects.all() + for language in Language.objects.all() ] - data["statistics"] = reverse("api-v1-info-release-stats", args=[self.kwargs["release"]]) - return data + serialized_release["statistics"] = reverse( + "api-v1-info-release-stats", kwargs={"release": self.kwargs.get("release")} + ) + return serialized_release class ReleaseStatsView(SerializeObjectView): - fields = ["name", "description", "status"] + fields = ("name", "description", "status") - def serialize_object(self, obj, fields=None): - lang_stats = obj.get_global_stats() + def serialize_object(self: "ReleaseStatsView", object_to_serialize: Release) -> dict[str, Any]: + lang_stats = object_to_serialize.get_global_stats() return { - "release": self.serialize_instance(obj, self.fields), + "release": self._serialize_instance(object_to_serialize, self.fields), "statistics": lang_stats, } - def get_object(self): + def get_object(self: "ReleaseStatsView") -> Release: return get_object_or_404(Release, name=self.kwargs["release"]) class ReleaseLanguageView(View): - def get(self, *args, **kwargs): + def get(self: "ReleaseLanguageView", *args, **kwargs) -> JsonResponse: # noqa: ARG002, ANN002, ANN003 release = get_object_or_404(Release, name=self.kwargs["release"]) - language = get_object_or_404(Language, locale=self.kwargs["lang"]) + language = get_object_or_404(Language, locale=self.kwargs["locale"]) item_list = [] for branch in release.branches.all().select_related("module"): for domain in branch.connected_domains: @@ -243,24 +293,33 @@ class ReleaseLanguageView(View): }) -class VertimusPageMixin: +class VertimusPageMixin(View): """ Mixin to be used in conjunction with a View. This is the default of any API view to manage the Vertimus workflow. """ - def get_state_from_kwargs(self): + def __init__(self: "VertimusPageMixin", **kwargs) -> None: # noqa: ANN003 + super().__init__(**kwargs) + + def get_state_from_kwargs(self: "VertimusPageMixin") -> tuple[Statistics, "State"]: + """ + :raises Http404: when there is no domain connected to this module’s branch. + """ module = get_object_or_404(Module, name=self.kwargs["module_name"]) branch = get_object_or_404(module.branch_set, name=self.kwargs["branch_name"]) domain = branch.connected_domains.get(self.kwargs["domain_name"]) if not domain: - raise Http404 + raise Http404( + f"No connected domain ‘{self.kwargs.get('domain_name')}’ found for " + f"module {module.name} and branch {branch.name}." + ) language = get_object_or_404(Language, locale=self.kwargs["lang"]) return get_vertimus_state(branch, domain, language)[1:] -class ModuleLangStatsView(VertimusPageMixin, View): - def get(self, *args, **kwargs): +class ModuleLangStatsView(VertimusPageMixin): + def get(self: "ModuleLangStatsView", *args, **kwargs) -> JsonResponse: # noqa: ARG002, ANN002, ANN003 stats, state = self.get_state_from_kwargs() return JsonResponse({ "module": state.branch.module.name, @@ -275,8 +334,13 @@ class ModuleLangStatsView(VertimusPageMixin, View): @method_decorator(csrf_exempt, name="dispatch") -class ReserveTranslationView(LoginRequiredMixin, VertimusPageMixin, View): - def post(self, request, *args, **kwargs): +class ReserveTranslationView(LoginRequiredMixin, VertimusPageMixin): + def post( + self: "ReserveTranslationView", + request: "HttpRequest", + *args, # noqa: ARG002, ANN002 + **kwargs, # noqa: ARG002, ANN003 + ) -> JsonResponse | HttpResponseForbidden: _, state = self.get_state_from_kwargs() actions = [x.name for x in state.get_available_actions(request.user.person)] if "RT" not in actions: @@ -288,12 +352,19 @@ class ReserveTranslationView(LoginRequiredMixin, VertimusPageMixin, View): @method_decorator(csrf_exempt, name="dispatch") -class UploadTranslationView(LoginRequiredMixin, VertimusPageMixin, View): - def post(self, request, *args, **kwargs): +class UploadTranslationView(LoginRequiredMixin, VertimusPageMixin): + def post( + self: "UploadTranslationView", + request: "HttpRequest", + *args, # noqa: ARG002, ANN002 + **kwargs, # noqa: ARG002, ANN003 + ) -> JsonResponse | HttpResponseForbidden: _, state = self.get_state_from_kwargs() actions = state.get_available_actions(request.user.person) if "UT" not in [x.name for x in actions]: - return HttpResponseForbidden("You cannot upload a translation to this module") + return HttpResponseForbidden( + "You cannot upload a translation to this module. This action is not available for the moment." + ) action_form = ActionForm( request.user, @@ -313,7 +384,9 @@ class UploadTranslationView(LoginRequiredMixin, VertimusPageMixin, View): @csrf_exempt -def refresh_module_branch(request, module_name: str, branch_name: str): +def refresh_module_branch( + request: "HttpRequest", module_name: str, branch_name: str +) -> JsonResponse | HttpResponseForbidden: """ Refresh the statistics of a given module branch. This is intended to be called externally when the branch of this module has been updated, for @@ -337,9 +410,9 @@ def refresh_module_branch(request, module_name: str, branch_name: str): return JsonResponse({"result": "OK"}) -def _update_branch_statistics(branch: "Branch"): +def _update_branch_statistics(branch: "Branch") -> None: try: branch.update_statistics(force=False) except Exception: traceback_text = traceback.format_exc() - mail_admins("Error while updating %s %s" % (branch.module.name, branch.name), traceback_text) + mail_admins(f"Error while updating {branch.module.name} {branch.name}", traceback_text) diff --git a/common/backends.py b/common/backends.py index 56c2d1ca2ccf3ca4241cab01d28e3ea6743aca3e..990e86f63c9fd2887b98ccfe49f772a9bf61d23d 100644 --- a/common/backends.py +++ b/common/backends.py @@ -5,7 +5,7 @@ from people.models import Person class TokenBackend(ModelBackend): # pylint: disable=inconsistent-return-statements - def authenticate(self, request, **kwargs) -> Person | None: + def authenticate(self, request, **kwargs) -> Person | None: # noqa: PLR6301 auth = request.META.get("HTTP_AUTHENTICATION", "").split() if len(auth) != 2 or auth[0] != "Bearer": return None diff --git a/common/fields.py b/common/fields.py index 3e9fe7114505fc44054e7a8282e30f94e59d0f60..058f7943a0ec23f8a9f8bb8178e05ecc79143f96 100644 --- a/common/fields.py +++ b/common/fields.py @@ -13,13 +13,13 @@ from django.db import models class DictionaryField(models.Field): description = "Dictionary object" - def get_internal_type(self): + def get_internal_type(self): # noqa: PLR6301 return "TextField" def from_db_value(self, value, *args): if value is None: return None - if value == "": + if not value: return {} if isinstance(value, str): try: @@ -31,7 +31,7 @@ class DictionaryField(models.Field): return value return {} - def get_prep_value(self, value): + def get_prep_value(self, value): # noqa: PLR6301 if not value: return "" if isinstance(value, str): @@ -62,7 +62,7 @@ class JSONField(models.TextField): def from_db_value(self, value, *args): """Convert our string value to JSON after we load it from the DB""" - if value == "": + if not value: return None try: @@ -76,10 +76,10 @@ class JSONField(models.TextField): def get_db_prep_save(self, value, connection): """Convert our JSON object to a string before we save""" - if value == "": + if not value: return None - if isinstance(value, (dict, list)): + if isinstance(value, dict | list): value = json.dumps(value, cls=DjangoJSONEncoder) return super().get_db_prep_save(value, connection=connection) diff --git a/common/tests.py b/common/tests.py index 5b24b349f9098d55edfaa22136ed7156ba03211c..c3b52e65fc4bc85de42667d50f007f6ace2f2037 100644 --- a/common/tests.py +++ b/common/tests.py @@ -159,7 +159,7 @@ class TransSortObjectListTest(TestCase): class IndexViewsTest(DjangoTestCase): - fixtures = ["test_data.json"] + fixtures = ("test_data.json",) def test_join_team_suggestion_text_exists_on_index_page_if_user_not_authenticated(self): response = self.client.get(reverse("home")) diff --git a/common/utils.py b/common/utils.py index a509dabf526d8d3af02255c990e8c6e98670cce6..f766e1f31b660b05cf9c91536ca7549879d6b10b 100644 --- a/common/utils.py +++ b/common/utils.py @@ -1,6 +1,7 @@ import os import socket -from subprocess import PIPE, run +from subprocess import run +from typing import TYPE_CHECKING from django.conf import settings from django.core.mail import EmailMessage @@ -9,6 +10,9 @@ from django.utils.translation import gettext as _ from damnedlies import logger +if TYPE_CHECKING: + pass + try: import icu @@ -38,8 +42,7 @@ def run_shell_command(cmd, input_data=None, raise_on_error=False, env=None, cwd= shell=shell, input=input_data, encoding="utf-8", - stdout=PIPE, - stderr=PIPE, + capture_output=True, env=env, cwd=cwd, **extra_kwargs, @@ -47,7 +50,7 @@ def run_shell_command(cmd, input_data=None, raise_on_error=False, env=None, cwd= status = result.returncode logger.debug(result.stdout + result.stderr) if raise_on_error and status != STATUS_OK: - raise CommandLineError(status, "Command: ‘%s’, Error: %s" % (cmd, result.stderr or result.stdout)) + raise CommandLineError(status, f"Command: ‘{cmd}’, Error: {result.stderr or result.stdout}") return status, result.stdout, result.stderr @@ -74,20 +77,10 @@ def is_site_admin(user): return user.is_superuser or settings.ADMIN_GROUP in [g.name for g in user.groups.all()] -def get_user_locale(request): - # FIXME: move this away to prevent import cycle - from languages.models import Language - - curlang = Language.get_language_from_ianacode(request.LANGUAGE_CODE) - if curlang and curlang.locale == "en": - curlang = None - return curlang - - def send_mail(subject, message, **kwargs): """Wrapper to Django's send_mail allowing all EmailMessage init arguments.""" if not subject.startswith(settings.EMAIL_SUBJECT_PREFIX): - subject = "%s%s" % (settings.EMAIL_SUBJECT_PREFIX, subject) + subject = f"{settings.EMAIL_SUBJECT_PREFIX}{subject}" EmailMessage(subject, message, **kwargs).send() diff --git a/common/views.py b/common/views.py index 60544d50985143a8027bf4ad8822e1ad378c9ea1..cd88c3a2df61b22a9173b2a58bccd1614ecd1956 100644 --- a/common/views.py +++ b/common/views.py @@ -23,13 +23,12 @@ from people.models import Person, obfuscate_email from teams.models import Role from teams.views import context_data_for_view_get_person_teams_from_request -from .utils import check_gitlab_request, get_user_locale, run_shell_command +from common.utils import check_gitlab_request, run_shell_command def index(request): """Homepage view""" - current_user_language = get_user_locale(request) - context = {"pageSection": "home", "user_language": current_user_language} + context = {"pageSection": "home"} context.update(context_data_for_view_get_person_teams_from_request(request)) return render(request, "index.html", context) @@ -94,11 +93,8 @@ def site_register(request): except SMTPException as exc: messages.error( request, - _( - "An error occurred while sending mail to {email} ({err})".format( - email=form.cleaned_data["email"], err=str(exc) - ) - ), + _("An error occurred while sending mail to %s (%s)") % form.cleaned_data["email"], + str(exc), ) else: return HttpResponseRedirect(reverse("register_success")) diff --git a/damnedlies/settings.py b/damnedlies/settings.py index ce69b6377b43c6f40cc93d2fd531f6c634a26fde..a0d1fa9091a701d4e2248bb29ec0b8dd525ca48b 100644 --- a/damnedlies/settings.py +++ b/damnedlies/settings.py @@ -38,8 +38,8 @@ DAMNED_LIES_BUG_URL = f"{DAMNED_LIES_BASE_PROJECT_URL}/-/issues/new" DAMNED_LIES_COMMIT_BASE_URL = f"{DAMNED_LIES_BASE_PROJECT_URL}/-/commit" INTERNATIONALISATION_TEAM_BUG_URL = DAMNED_LIES_BUG_URL -ENTER_LANGUAGE_BUG_URL = "https://gitlab.gnome.org/Teams/Translation/%(lang)s/issues/new" -BROWSE_LANGUAGE_BUG_URL = "https://gitlab.gnome.org/Teams/Translation/%(lang)s/issues?state=opened" +ENTER_LANGUAGE_BUG_URL = "https://gitlab.gnome.org/Teams/Translation/%(locale)s/issues/new" +BROWSE_LANGUAGE_BUG_URL = "https://gitlab.gnome.org/Teams/Translation/%(locale)s/issues?state=opened" # Local time zone for this installation. Choices can be found here: # https://en.wikipedia.org/wiki/List_of_tz_zones_by_name diff --git a/docs/source/conf.py b/docs/source/conf.py index 0845035b17dad53842c15a86df4480b7ff6d2a4c..14ea81806f1d510d2d9939d0e52482c989a3c413 100644 --- a/docs/source/conf.py +++ b/docs/source/conf.py @@ -6,8 +6,8 @@ import sys import django -sys.path.insert(0, os.path.abspath("..")) -sys.path.insert(1, os.path.abspath("../..")) +sys.path.insert(0, os.path.abspath("..")) # noqa: PTH100 +sys.path.insert(1, os.path.abspath("../..")) # noqa: PTH100 os.environ["DJANGO_SETTINGS_MODULE"] = "damnedlies.settings_tests" django.setup() diff --git a/feeds/tests.py b/feeds/tests.py index 66881e04e80ce9bb9eb0abc194a9144fe24d6a28..244dbfbdf25e4793f804c9eda9626f07635a2bea 100644 --- a/feeds/tests.py +++ b/feeds/tests.py @@ -3,7 +3,7 @@ from django.urls import reverse class FeedTests(TestCase): - fixtures = ["test_data.json"] + fixtures = ("test_data.json",) def test_language_feed_downloaded_filename_is_correct(self): response = self.client.get(reverse("lang_feed", kwargs={"locale": "fr"})) diff --git a/feeds/views.py b/feeds/views.py index a7d3a43259f5decfa72bfad40b92a07cc9a5fc6d..5ffc5d353d39e14ffdf835c0e74408fca8e1f275 100644 --- a/feeds/views.py +++ b/feeds/views.py @@ -1,5 +1,9 @@ +from datetime import datetime +from typing import TYPE_CHECKING + from django.conf import settings from django.contrib.syndication.views import Feed, FeedDoesNotExist +from django.http import HttpRequest from django.urls import reverse from django.utils.text import slugify from django.utils.translation import gettext_lazy as _ @@ -7,51 +11,59 @@ from django.utils.translation import gettext_lazy as _ from languages.models import Language from vertimus.models import Action, ActionArchived +if TYPE_CHECKING: + from people.models import Person + class LatestActionsByLanguage(Feed): title_template = "feeds/actions_title.html" description_template = "feeds/actions_description.html" - def __call__(self, request, *args, **kwargs): + def __call__(self: "LatestActionsByLanguage", request: HttpRequest, *args, **kwargs): # noqa ANN002, ANN003 """ Rewrite __call__ method to set the downloaded filename. """ response = super().__call__(request, *args, **kwargs) # Object should exist or exception is already raised by super().__call__() - language = self.get_object(request, *args, **kwargs) - response["Content-Disposition"] = "attachment; filename=%s" % ( - f"gnome_damned_lies_rss_feed_for_{slugify(language.name)}.rss" - ) + language = self.get_object(request, **kwargs) + response[ + "Content-Disposition" + ] = f"attachment; filename=gnome_damned_lies_rss_feed_for_{slugify(language.name)}.rss" return response - def get_object(self, request, locale): + def get_object(self: "LatestActionsByLanguage", request: "HttpRequest", locale: str, **kwargs) -> Language: # noqa: PLR6301, ANN003, ARG002 return Language.objects.get(locale=locale) - def title(self, obj): + def title(self: "LatestActionsByLanguage", language: "Language") -> str: # noqa: PLR6301 return _("%(site)s — Workflow actions for the %(lang)s language") % { "site": settings.SITE_DOMAIN, - "lang": obj.name, + "lang": language.name, } - def link(self, obj): - if not obj: + def link(self: "LatestActionsByLanguage", action: "Language") -> str: # noqa: PLR6301 + """ + :raises FeedDoesNotExist: when the requested feed does not exist. + """ + if not action: raise FeedDoesNotExist - return obj.get_team_url() + return action.get_team_url() - def description(self, obj): - return _("Latest actions of the GNOME Translation Project for the %s language") % obj.name + def description(self: "LatestActionsByLanguage", action: "Language") -> str: # noqa: PLR6301 + return _("Latest actions of the GNOME Translation Project for the %s language") % action.name - def items(self, obj): + def items(self: "LatestActionsByLanguage", action: "Language") -> list[Action]: # noqa: PLR6301 actions = ( - Action.objects.filter(state_db__language=obj.id) + Action.objects.filter(state_db__language=action.id) .select_related("state_db") .union( - ActionArchived.objects.filter(state_db__language=obj.id).defer("sequence").select_related("state_db") + ActionArchived.objects.filter(state_db__language=action.id) + .defer("sequence") + .select_related("state_db") ) ) return actions.order_by("-created")[:20] - def item_link(self, item): + def item_link(self: "LatestActionsByLanguage", item: "Action") -> str: # noqa: PLR6301 link = reverse( "vertimus_by_names", args=( @@ -63,8 +75,8 @@ class LatestActionsByLanguage(Feed): ) return "%s#%d" % (link, item.id) - def item_pubdate(self, item): + def item_pubdate(self: "LatestActionsByLanguage", item: "Action") -> "datetime": # noqa: PLR6301 return item.created - def item_author_name(self, item): + def item_author_name(self: "LatestActionsByLanguage", item: "Action") -> "Person": # noqa: PLR6301 return item.person diff --git a/languages/models.py b/languages/models.py index 586f80e8f3ce268ddb7cc633ecd12fd1bed25a88..08c1b332b993dd1df42785a74511f15553c7c3f6 100644 --- a/languages/models.py +++ b/languages/models.py @@ -1,20 +1,35 @@ +from typing import ClassVar, Union + from django.conf import settings from django.db import models from django.db.models import Q +from django.http import HttpRequest from django.urls import NoReverseMatch from django.utils.translation import gettext as _ +from damnedlies import logger from teams.models import FakeTeam, Team +class NoLanguageFoundError(Exception): + """ + When no language is found in a lookup. + """ + + class Language(models.Model): + """ + The representation of a language in the system. + It is intended to be associated to translations. + """ + name = models.CharField(max_length=50, unique=True) locale = models.CharField(max_length=15, unique=True) team = models.ForeignKey(Team, null=True, blank=True, default=None, on_delete=models.SET_NULL) plurals = models.CharField(max_length=200, blank=True) # Mapping for code differences between GNOME and Django standards. - lang_mapping = { + lang_mapping: ClassVar[dict[str, str]] = { "zh_CN": "zh_Hans", "zh_TW": "zh_Hant", } @@ -23,79 +38,102 @@ class Language(models.Model): db_table = "language" ordering = ("name",) - def __str__(self): - return "%s (%s)" % (self.name, self.locale) + def __str__(self: "Language") -> str: + return f"{self.name} ({self.locale})" @classmethod - def django_locale(cls, locale): + def django_locale(cls: "Language", locale: str) -> str: return cls.lang_mapping.get(locale, locale) @classmethod - def get_language_from_ianacode(cls, ianacode): - """Return a matching Language object corresponding to LANGUAGE_CODE (BCP47-formatted) - This is a sort of BCP47 to ISO639 conversion function""" - iana_splitted = ianacode.split("-", 1) - lang_code = iana_splitted[0] - iana_suffix = iana_splitted[1] if len(iana_splitted) > 1 else "" + def get_language_from_iana_code(cls: "Language", iana_language_code: str) -> "Language": + """ + Return a matching Language object corresponding to LANGUAGE_CODE (BCP47-formatted). + This converts BCP47 to ISO639 conversion function + + :raises NoLanguageFoundError: when no language corresponding to the IANA language code is found. + + .. seealso: `IANA Language Subtag Registry _` + + """ + iana_split = iana_language_code.split("-", 1) + language_code, iana_suffix = iana_split[0], iana_split[1] if len(iana_split) > 1 else "" iana_suffix = iana_suffix.replace("Latn", "latin").replace("Cyrl", "cyrillic") - lang_list = cls.objects.filter(Q(locale=lang_code) | Q(locale__regex=r"^%s[@_].+" % lang_code)) - if len(lang_list) == 0: - return None - if len(lang_list) > 1: + + candidate_languages = cls.objects.filter( + Q(locale=language_code) | Q(locale__regex=r"^%s[@_].+" % language_code) + ) + + if len(candidate_languages) == 0: + raise NoLanguageFoundError( + _("No corresponding Language was found for this IANA BCP47 code (%s).") % iana_language_code + ) + + best_candidate_language = candidate_languages[0] + if len(candidate_languages) > 1: # Find the best language to return - best_lang = lang_list[0] - for lang in lang_list: - if lang.get_suffix(): - if iana_suffix.lower() == lang.get_suffix().lower(): - return lang + for candidate_language in candidate_languages: + if candidate_language.get_suffix(): + if iana_suffix.lower() == candidate_language.get_suffix().lower(): + best_candidate_language = candidate_language + break else: - best_lang = lang - return best_lang - return lang_list[0] + best_candidate_language = candidate_language - def get_name(self): + return best_candidate_language + + def get_name(self: "Language") -> str: + """ + Get the locale of the Language object, and if it differs from the locale, + translate it. + """ if self.name != self.locale: return _(self.name) return self.locale - def get_suffix(self): - splitted = self.locale.replace("@", "_").split("_") - if len(splitted) > 1: - return splitted[-1] - return None - - def get_plurals(self): + def get_suffix(self: "Language") -> str: + """ + Get the locale suffix, for instance ‘BR’ for ‘pt_BR’ or ‘CA’ for ‘fr_CA’. + """ + split_locale = self.locale.replace("@", "_").split("_") + if len(split_locale) > 1: + return split_locale[-1] + return "" + + def get_plurals(self: "Language") -> str: + """ + Get the plural form of the language. If none, return ‘Unknown’, translated for the user. + """ # Translators: this concerns an unknown plural form return self.plurals or _("Unknown") - def bugs_url_enter(self): - return settings.ENTER_LANGUAGE_BUG_URL % { - "lang": self.locale, - } - - def bugs_url_show(self): - return settings.BROWSE_LANGUAGE_BUG_URL % { - "lang": self.locale, - } - - def get_release_stats(self, archives=False): - # FIXME: move this function away to avoid the circular import. - """Get summary stats for all releases""" - from stats.models import Release - - if archives: - releases = Release.objects.all().filter(weight__lt=0).order_by("status", "-weight", "-name") - else: - releases = Release.objects.all().filter(weight__gte=0).order_by("status", "-weight", "-name") - stats = [] - for rel in releases: - stats.append(rel.total_for_lang(self)) - return stats - - def get_team_url(self): + def bugs_url_enter(self: "Language") -> str: + return settings.ENTER_LANGUAGE_BUG_URL % {"locale": self.locale} + + def bugs_url_show(self: "Language") -> str: + return settings.BROWSE_LANGUAGE_BUG_URL % {"locale": self.locale} + + def get_team_url(self: "Language") -> str: if self.team: return self.team.get_absolute_url() try: return FakeTeam(self).get_absolute_url() except NoReverseMatch: return "" + + +def get_user_language_from_request(request: "HttpRequest") -> Union["Language", None]: + """ + Get the Language, and so locale that is active for a given Django request. + """ + current_language = None + + try: + current_language = Language.get_language_from_iana_code(request.LANGUAGE_CODE) + except NoLanguageFoundError as no_language_found_error: + logger.debug(no_language_found_error) + + if current_language and current_language.locale == "en": + current_language = None + + return current_language diff --git a/languages/tests.py b/languages/tests.py index b62cd7ad6fe4f1f23afcb2d455c7dd971d32d21c..a2569767d186edfba8f4f715b38feae8cf384a2f 100644 --- a/languages/tests.py +++ b/languages/tests.py @@ -2,30 +2,29 @@ from django.conf import settings from django.test import TestCase from django.urls import reverse -from languages.models import Language +from languages.models import Language, NoLanguageFoundError from people.models import Person from teams.models import Role, Team class LanguageTestCase(TestCase): - fixtures = ["test_data.json"] + fixtures = ("test_data.json",) - def test_language_release_xml(self): - response = self.client.get(reverse("language_release_xml", args=["fr", "gnome-3-8"])) - self.assertContains(response, """""") - - def test_language_from_ianacode(self): + def test_language_from_iana_code(self): Language.objects.create(name="Belarussian", locale="be") Language.objects.create(name="French (Belgium)", locale="fr_BE") Language.objects.create(name="Chinese (Taiwan)", locale="zh_TW") - func = Language.get_language_from_ianacode + func = Language.get_language_from_iana_code self.assertEqual(func("fr-ch").locale, "fr") self.assertEqual(func("fr-be").locale, "fr_BE") self.assertEqual(func("be").locale, "be") self.assertEqual(func("be-latin-RU").locale, "be") self.assertEqual(func("zh-tw").locale, "zh_TW") - self.assertEqual(func("xx"), None) + + def test_language_from_iana_code_invalid_language_code(self): + with self.assertRaises(NoLanguageFoundError): + self.assertEqual(Language.get_language_from_iana_code("xx"), None) def test_language_suggestion_for_non_authenticated_user_in_french(self): self.client.cookies.load({settings.LANGUAGE_COOKIE_NAME: "fr"}) diff --git a/languages/urls.py b/languages/urls.py index 2cb6d9d7ca306d8dcb81be843360f7e156e00e3e..6da93f7e40a6b1f4a31dfc708e1cd46e76d717ec 100644 --- a/languages/urls.py +++ b/languages/urls.py @@ -8,7 +8,6 @@ urlpatterns = [ path("/all//", views.language_all, name="language_all"), path("/rel-archives/", views.release_archives, name="language_release_archives"), path("///", views.language_release, name="language_release"), - path("//xml", views.language_release_xml, name="language_release_xml"), path( "//.tar.gz", views.language_release_tar, diff --git a/languages/views.py b/languages/views.py index 689509440a620ce47df695b5eb68746d57c490da..32257bc6e74a84a85cb8c5840e09f9990338f963 100644 --- a/languages/views.py +++ b/languages/views.py @@ -1,19 +1,20 @@ import os import tarfile from datetime import UTC, date, datetime +from pathlib import Path from django.conf import settings -from django.http import HttpResponse, HttpResponseRedirect +from django.http import HttpRequest, HttpResponse, HttpResponseRedirect from django.shortcuts import get_object_or_404, render from django.utils.translation import gettext as _ from common import utils from languages.models import Language from people.models import Person -from stats.models import Release, Statistics +from stats.models import Release, Statistics, get_release_statistics_in_a_given_language -def languages(request): +def languages(request: HttpRequest) -> HttpResponse: all_team_related_languages = Language.objects.select_related("team") context = { "pageSection": "languages", @@ -28,9 +29,9 @@ def languages(request): return render(request, "languages/language_list.html", context) -def language_all(request, locale, dtype): +def language_all(request: HttpRequest, locale: str, domain_type: str) -> HttpResponse: language = get_object_or_404(Language, locale=locale) - stats = Statistics.get_lang_stats_by_type(language, dtype, release=None) + stats = Statistics.get_lang_stats_by_type(language, domain_type, release=None) context = { "pageSection": "languages", "language": language, @@ -38,148 +39,77 @@ def language_all(request, locale, dtype): "ui": _("UI Translations"), "ui-part": _("UI Translations (reduced)"), "doc": _("Documentation"), - }.get(dtype), + }.get(domain_type), "stats": stats, - "scope": dtype.endswith("-part") and "part" or "full", + "scope": "part" if domain_type.endswith("-part") else "full", } return render(request, "languages/language_all_modules.html", context) -def release_archives(request, locale): +def release_archives(request: HttpRequest, locale: str) -> HttpResponse: """This view is used to display archive release stats through Ajax call Only the HTML table is produced """ language = get_object_or_404(Language, locale=locale) context = { "lang": language, - "stats": language.get_release_stats(archives=True), + "stats": get_release_statistics_in_a_given_language(language, archives=True), "show_all_modules_line": False, } return render(request, "languages/language_release_summary.html", context) -def language_release(request, locale, release_name, dtype): +def language_release(request: HttpRequest, locale: str, release_name: str, domain_type: str) -> HttpResponse: if locale == "C": language = None else: language = get_object_or_404(Language, locale=locale) release = get_object_or_404(Release, name=release_name) - stats = Statistics.get_lang_stats_by_type(language, dtype, release) + stats = Statistics.get_lang_stats_by_type(language, domain_type, release) context = { "pageSection": "languages", "language": language, - "language_name": language and language.get_name() or _("Original strings"), + "language_name": language.get_name() if language else _("Original strings"), "release": release, "stats_title": { "ui": _("UI Translations"), "ui-part": _("UI Translations (reduced)"), "doc": _("Documentation"), - }.get(dtype), + }.get(domain_type), "stats": stats, - "dtype": dtype, - "scope": dtype.endswith("-part") and "part" or "full", + "dtype": domain_type, + "scope": "part" if domain_type.endswith("-part") else "full", } return render(request, "languages/language_release.html", context) -def language_release_tar(request, locale, release_name, dtype): +def language_release_tar( + request: HttpRequest, # noqa: ARG001 + locale: str, + release_name: str, + domain_type: str, +) -> HttpResponse: release = get_object_or_404(Release, name=release_name) language = get_object_or_404(Language, locale=locale) - last_modif, file_list = release.get_lang_files(language, dtype) + last_modif, file_list = release.get_lang_files(language, domain_type) - tar_filename = "%s.%s.%s.%s.tar.gz" % (release.name, dtype, language.locale, date.today()) - tar_directory = os.path.join(settings.POT_DIR, "tar") + tar_filename = f"{release.name}.{domain_type}.{language.locale}.{date.today()}.tar.gz" + tar_directory = settings.POT_DIR / "tar" if not os.access(tar_directory, os.R_OK): - os.mkdir(tar_directory) - tar_path = os.path.join(tar_directory, tar_filename) - if not os.access(tar_path, os.R_OK) or last_modif > datetime.fromtimestamp(os.path.getmtime(tar_path), UTC): + tar_directory.mkdir(parents=True, exist_ok=True) + tar_path = tar_directory / tar_filename + if not os.access(tar_path, os.R_OK) or last_modif > datetime.fromtimestamp(tar_path.stat().st_mtime, UTC): with tarfile.open(tar_path, "w:gz") as tar_file: for f in file_list: - tar_file.add(f, os.path.basename(f)) + tar_file.add(f, Path(f).name) return HttpResponseRedirect("/POT/tar/%s" % tar_filename) -def language_release_xml(request, locale, release_name): - """ - This view create the same XML output than the previous Damned-Lies, so as - apps which depend on it (like Vertimus) don't break. - This view may be suppressed when Vertimus will be integrated in D-L. - """ - language = get_object_or_404(Language, locale=locale) - release = get_object_or_404(Release, name=release_name) - stats = release.get_lang_stats(language) - content = '\n' % (locale, release_name) - for catname, categ in stats["ui"]["categs"].items(): - if catname != "default": - content += '' % catname - # totals for category - if catname in stats["doc"]["categs"]: - content += "%s" % stats["doc"]["categs"][catname]["cattrans"] - content += "%s" % stats["doc"]["categs"][catname]["catfuzzy"] - content += "%s" % stats["doc"]["categs"][catname]["catuntrans"] - content += "%s" % categ["cattrans"] - content += "%s" % categ["catfuzzy"] - content += "%s" % categ["catuntrans"] - # Modules - for module, mod_stats in categ["modules"].items(): - branch_name, domains = list(mod_stats.items())[0] - content += '' % (module.name, branch_name) - # DOC domains - if catname in stats["doc"]["categs"] and stats["doc"]["categs"][catname]["modules"]: - for docmod, data in stats["doc"]["categs"][catname]["modules"].items(): - if docmod == module: - content += get_domain_stats(list(data.values())[0], "document") - # UI stats - content += get_domain_stats(domains, "domain") - content += "" - # Add modules who have no ui counterparts - # FIXME: duplicated code fragment. Replace respectively 'dev-tools' and 'desktop' with the catname variable - if catname == "dev-tools": - try: - mod = [m for m in stats["doc"]["categs"]["dev-tools"]["modules"] if m[0] == "gnome-devel-docs"][0][1] - content += '' % mod.keys()[0] - content += get_domain_stats(mod.values()[0], "document") - content += "" - except (KeyError, Exception): # FIXME: which exception is this catching? - pass - if catname == "desktop": - try: - mod = [m for m in stats["doc"]["categs"]["desktop"]["modules"] if m[0] == "gnome-user-docs"][0][1] - content += '' % mod.keys()[0] - content += get_domain_stats(mod.values()[0], "document") - content += "" - except (KeyError, Exception): # FIXME: which exception is this catching? - pass - - if catname != "default": - content += "" - content += "" - return HttpResponse(content, content_type="text/xml") - - -def get_domain_stats(mods, node_name): - """Iterate module domains to get stats""" - content = "" - for dom_key, stat in mods: - if dom_key == " fake": - continue - content += '<%s id="%s">' % (node_name, stat.domain.name) - content += "%s" % stat.translated() - content += "%s" % stat.fuzzy() - content += "%s" % stat.untranslated() - content += "%s" % stat.po_url() - content += "%s" % stat.vcs_web_path() - content += "" % node_name - return content - - -# ********* Utility functions ****************** -def clean_tar_files(): +def clean_tar_files() -> None: """Delete outdated tar.gz files generated by the language_release_tar view""" - tar_directory = os.path.join(settings.POT_DIR, "tar") - if not os.path.exists(tar_directory): - return - for tar_file in os.listdir(tar_directory): - if not tar_file.endswith("%s.tar.gz" % date.today()): - os.remove(os.path.join(tar_directory, tar_file)) + tar_directory = settings.POT_DIR / "tar" + if tar_directory.exists(): + for tar_file in os.listdir(tar_directory): + if not tar_file.endswith(f"{date.today()}.tar.gz"): + (tar_directory / tar_file).unlink() diff --git a/people/admin.py b/people/admin.py index c85b4b0b4e1bffa2f7d8513f66f251a1bcf1653b..234d76277071875bfdd29576e7bf9ca4e0a83be9 100644 --- a/people/admin.py +++ b/people/admin.py @@ -3,7 +3,8 @@ from django.contrib.auth.admin import UserAdmin # pylint: disable=imported-auth-user from django.contrib.auth.models import User -from django.db.models import Q +from django.db.models import Q, QuerySet +from django.http import HttpRequest from django.utils.translation import gettext as _ from common.utils import send_mail @@ -13,7 +14,7 @@ from teams.models import Role class CoordinatorWithoutForgeAccountNotification: - def __init__(self, username): + def __init__(self: "CoordinatorWithoutForgeAccountNotification", username: str) -> None: self.title = _("Your profile is missing your GitLab forge account on Damned Lies") self.body = _( "Dear %(username)s,\n\n" @@ -25,7 +26,11 @@ class CoordinatorWithoutForgeAccountNotification: @admin.action(description=_("Send messages to coordinators that did not set their GitLab profile username.")) -def send_notification_to_coordinators_without_forge_accounts(modeladmin, request, queryset): +def send_notification_to_coordinators_without_forge_accounts( + modeladmin: admin.ModelAdmin, # noqa: ARG001 + request: HttpRequest, # noqa: ARG001 + queryset: QuerySet, +) -> None: users_without_forge_account = queryset.filter(Q(forge_account=None)).values_list("id") coordinators_without_forge_account = [ role.person for role in Role.objects.filter(Q(person__in=users_without_forge_account) & Q(role="coordinator")) @@ -44,8 +49,8 @@ class RoleInTeamInline(admin.TabularInline): class PersonAdmin(admin.ModelAdmin): search_fields = ("username", "first_name", "last_name", "email") list_display = ("username", "first_name", "last_name", "email", "webpage_url", "forge_account") - actions = [send_notification_to_coordinators_without_forge_accounts] - inlines = [RoleInTeamInline] + actions = (send_notification_to_coordinators_without_forge_accounts,) + inlines = (RoleInTeamInline,) UserAdmin.list_display = ("username", "email", "last_name", "first_name", "is_active", "last_login") diff --git a/people/forms.py b/people/forms.py index e22059ae70e08b7ac6a29cf3ec2b18ce8d9835cc..767b5c088410b0e17c8edc6622acc8745d90c7cb 100644 --- a/people/forms.py +++ b/people/forms.py @@ -1,18 +1,22 @@ import hashlib -from http.client import InvalidURL from secrets import token_bytes -from urllib.request import urlopen +from typing import Any +import requests from django import forms from django.conf import settings from django.contrib.auth.forms import AuthenticationForm, UsernameField from django.contrib.auth.validators import UnicodeUsernameValidator from django.core.exceptions import ValidationError +from django.http import HttpRequest from django.urls import reverse from django.utils.encoding import force_bytes from django.utils.translation import gettext as _ from django.utils.translation import gettext_lazy from PIL import ImageFile +from requests import HTTPError +from requests.exceptions import ConnectionError as RequestConnectionError +from requests.exceptions import InvalidSchema, MissingSchema from common.utils import send_mail from people.models import Person @@ -45,15 +49,19 @@ class RegistrationForm(forms.Form): required=False, ) - def clean_username(self): - """Validate the username (correctness and uniqueness)""" + def clean_username(self: "RegistrationForm") -> str: + """ + Validate the username + + :raises ValidationError: when the username is already taken by another user. + """ try: Person.objects.get(username__iexact=self.cleaned_data["username"]) except Person.DoesNotExist: return self.cleaned_data["username"] raise ValidationError(_("This username is already taken. Please choose another.")) - def clean(self): + def clean(self: "RegistrationForm") -> dict[str, Any]: cleaned_data = self.cleaned_data password1 = cleaned_data.get("password1") password2 = cleaned_data.get("password2") @@ -64,7 +72,7 @@ class RegistrationForm(forms.Form): raise ValidationError(_("The passwords do not match")) return cleaned_data - def save(self, request): + def save(self: "RegistrationForm", request: HttpRequest) -> "Person": # noqa: ARG002 """Create the user""" username = self.cleaned_data["username"] email = self.cleaned_data["email"] @@ -78,11 +86,12 @@ class RegistrationForm(forms.Form): ) new_user.save() - self._send_activation_email(activation_key, email) + RegistrationForm._send_activation_email(activation_key, email) return new_user - def _send_activation_email(self, activation_key: str, email: str): + @staticmethod + def _send_activation_email(activation_key: str, email: str) -> None: site_domain = settings.SITE_DOMAIN activation_url = str(reverse("register_activation", kwargs={"key": activation_key})) message = ( @@ -92,13 +101,13 @@ class RegistrationForm(forms.Form): ) % site_domain ) - message += "\n\nhttps://%s%s\n\n" % (site_domain, activation_url) - message += _("Administrators of %s" % site_domain) + message += f"\n\nhttps://{site_domain}{activation_url}\n\n" + message += _("Administrators of %s") % site_domain send_mail(_("Account activation"), message, to=[email]) class LoginForm(AuthenticationForm): - def clean_username(self): + def clean_username(self: "LoginForm") -> str: username = self.cleaned_data["username"] if "@" in username and not Person.objects.filter(username=username).exists(): try: @@ -124,58 +133,71 @@ class DetailForm(forms.ModelForm): "forum_account", ) - def clean_forge_account(self): + def clean_forge_account(self: "DetailForm") -> str: forge_account = self.cleaned_data["forge_account"] if forge_account and forge_account.startswith("@"): forge_account = forge_account.removeprefix("@") return forge_account - def clean_image(self): + def clean_image(self: "DetailForm") -> str: url = self.cleaned_data["image"] + max_width, max_height = Person.MAX_IMAGE_SIZE if url: - size = get_image_size(url) - if size[0] > 100 or size[1] > 100: + width, height = get_image_size_from_url(url) + if width > max_width or height > max_height: raise ValidationError( _("Image too high or too wide (%(width)d×%(height)d, maximum is 100×100 pixels)") - % {"width": size[0], "height": size[1]} + % {"width": width, "height": height} ) return url class TeamChoiceField(forms.ModelChoiceField): - def label_from_instance(self, obj): - return obj.get_description() + def label_from_instance(self: "TeamChoiceField", team: Team) -> str: # noqa: PLR6301 + return team.get_description() class TeamJoinForm(forms.Form): - def __init__(self, *args, **kwargs): + def __init__(self: "TeamJoinForm", *args: list[Any], **kwargs: dict[str, Any]) -> None: super().__init__(*args, **kwargs) # FIXME: exclude team to which user is already member self.fields["teams"] = TeamChoiceField(queryset=Team.objects.all()) -def get_image_size(url): - """Returns width and height (as tuple) of the image pointed at by the url - Code partially copied from http://effbot.org/zone/pil-image-size.htm""" +def get_image_size_from_url(url: str) -> tuple[int, int]: + """ + Returns the width and height (as a tuple) of the image pointed at by the url. + + :raises ValidationError: when there is an error getting the image size + + .. seealso:: + `Pillow ImageFile documentation. `_ + + Code partially copied from http://effbot.org/zone/pil-image-size.htm (dead link) + """ try: - im_file = urlopen(url) - except (OSError, InvalidURL, EOFError, ValueError) as exc: - raise ValidationError(_("The URL you provided is not valid")) from exc + r = requests.get(url, stream=True, timeout=2) + r.raise_for_status() + except (HTTPError, RequestConnectionError, MissingSchema, InvalidSchema) as exc: + raise ValidationError(_("There was an error retrieving the image file (%s)") % exc) from exc + r.raw.decode_content = True + image_file = r.raw + size = None - p = ImageFile.Parser() + image_file_parser = ImageFile.Parser() try: - while 1: - data = im_file.read(1024) + while True: + data = image_file.read(1024) if not data: break - p.feed(data) - if p.image: - size = p.image.size + image_file_parser.feed(data) + if image_file_parser.image: + size = image_file_parser.image.size break except Exception as exc: - raise ValidationError("Sorry, an error occurred while trying to get image size (%s)" % str(exc)) from exc - finally: - im_file.close() + raise ValidationError(_("Sorry, an error occurred while trying to get image size (%s)") % str(exc)) from exc + if not size: raise ValidationError(_("The URL you provided seems not to correspond to a valid image")) + return size diff --git a/people/models.py b/people/models.py index 9d9f5b4105d524eabff88955fbac8e1ddd29248e..f782bfdefa1307448815f8be021e6e3f3dd1f375 100644 --- a/people/models.py +++ b/people/models.py @@ -2,7 +2,7 @@ import binascii import os import re from datetime import timedelta -from typing import TYPE_CHECKING, Union +from typing import TYPE_CHECKING, Any, Union import requests @@ -23,6 +23,8 @@ from damnedlies import settings if TYPE_CHECKING: from languages.models import Language + from stats.models import Module + from teams.models import Team AVATAR_SERVICES = { "gravatar.com": "https://secure.gravatar.com/avatar/{hash}.jpg?{qs}", @@ -45,7 +47,12 @@ class MatrixNetworkField(models.CharField): DOMAIN_REGEX = r":[A-Z0-9a-z.-]+(:[0-9]{2,5})?" MATRIX_USER_IDENTIFIER_REGEX = r"@[A-Z0-9a-z\u0021-\u0039\u003B-\u007F]+" - def validate(self, value, model_instance: "Person"): + def validate(self: "MatrixNetworkField", value: str, model_instance: "Person") -> None: + """ + Validate the Matrix username. + + :raises ValidationError: when the username given is invalid. + """ super().validate(value, model_instance) if not bool( re.match(f"{MatrixNetworkField.MATRIX_USER_IDENTIFIER_REGEX}{MatrixNetworkField.DOMAIN_REGEX}", value) @@ -67,9 +74,9 @@ class FakePerson: that does not really exist in the database. """ - def get_languages(self) -> tuple["Language"]: + def get_languages(self: "FakePerson") -> tuple["Language"]: # noqa: PLR6301 # FIXME: move this function away to avoid cycle imports - from languages.models import Language + from languages.models import Language # noqa: PLC0415 all_person_languages = tuple() languages_for_user = Language.objects.filter(locale=get_language()) @@ -78,9 +85,14 @@ class FakePerson: return all_person_languages -class Person(User, FakePerson): +class Person(User, FakePerson): # noqa: PLR0904 """The User class of D-L.""" + MAX_IMAGE_SIZE: tuple[int, int] = (100, 100) + """ + Maximum width and height of the profile image. + """ + auth_token = models.CharField(_("Authentication Token"), max_length=40, blank=True) forge_account = models.CharField( _("GitLab account"), @@ -141,7 +153,7 @@ class Person(User, FakePerson): ordering = ("username",) @classmethod - def clean_unactivated_accounts(cls): + def clean_unactivated_accounts(cls: "Person") -> None: accounts = cls.objects.filter( activation_key__isnull=False, date_joined__lt=(timezone.now() - timedelta(days=10)) ).exclude(activation_key="") @@ -149,7 +161,7 @@ class Person(User, FakePerson): account.delete() @classmethod - def clean_obsolete_accounts(cls): + def clean_obsolete_accounts(cls: type["Person"]) -> None: """Remove accounts that: - last login is more than 2 years - is not coordinator @@ -172,7 +184,7 @@ class Person(User, FakePerson): account.delete() @classmethod - def get_by_user(cls, user: User) -> Union["Person", "FakePerson"]: + def get_by_user(cls: type["Person"], user: User) -> Union["Person", "FakePerson"]: if user.is_anonymous: return FakePerson() try: @@ -185,7 +197,7 @@ class Person(User, FakePerson): return user.person @classmethod - def get_by_attr(cls, key, val): + def get_by_attr(cls: type["Person"], key: str, val: Any) -> Union[None, "Person"]: # noqa: ANN401 if not val: return None try: @@ -194,72 +206,69 @@ class Person(User, FakePerson): return None @classmethod - def generate_token(cls): + def generate_token(cls: type["Person"]) -> str: return binascii.hexlify(os.urandom(20)).decode() - def save(self, *args, **kwargs): + def save(self: "Person", *args: list[Any], **kwargs: dict[str, Any]) -> None: if not self.password or self.password == "!": # nosec self.password = None self.set_unusable_password() super().save(*args, **kwargs) - def activate(self): + def activate(self: "Person") -> None: self.activation_key = None self.is_active = True self.save() - def no_spam_email(self): + def no_spam_email(self: "Person") -> str: return obfuscate_email(self.email) @property - def name(self): + def name(self: "Person") -> str: if self.first_name or self.last_name: return " ".join([name for name in [self.first_name, self.last_name] if name]) return self.username @property - def forge_account_url(self) -> str: + def forge_account_url(self: "Person") -> str: """ - :raises: - ValueError: when no forge account exist for this user profile + :raises ValueError: when no forge account exist for this user profile """ if self.forge_account: return f"{settings.GNOME_GITLAB_USER_URL}/{self.forge_account}" raise ValueError(_("The forge account name is missing from the user profile.")) @property - def forum_account_url(self) -> str: + def forum_account_url(self: "Person") -> str: """ - :raises: - ValueError: when no forum account exist for this user profile + :raises ValueError: when no forum account exist for this user profile """ if self.forum_account: return f"{settings.GNOME_FORUM_USER_URL}/{self.forum_account}" raise ValueError(_("The username on the GNOME forum is missing from the user profile.")) @property - def forge_account_exists(self): + def forge_account_exists(self: "Person") -> bool: user_api_url = f"https://{settings.GNOME_GITLAB_DOMAIN_NAME}/api/v4/users?username={self.forge_account}" response = requests.get(user_api_url, timeout=1, allow_redirects=False).json() return len(response) == 1 and response[0].get("state", "") == "active" - def __str__(self): + def __str__(self: "Person") -> str: return self.name - def as_author(self): - return "%s <%s>" % (self.name, self.email) + def as_author(self: "Person") -> str: + return f"{self.name} <{self.email}>" - def get_absolute_url(self): + def get_absolute_url(self: "Person") -> str: return reverse("person_detail_username", args=[self.username]) - def coordinates_teams(self): + def coordinates_teams(self: "Person") -> bool: # Class imported here to avoid cyclic import - # pylint: disable=import-outside-toplevel - from teams.models import Team + from teams.models import Team # noqa: PLC0415 return Team.objects.filter(role__person__id=self.id).all() - def is_coordinator(self, team): + def is_coordinator(self: "Person", team: "Team") -> bool: """ If team is a Team instance, tell if current Person is coordinator of that team. If team = 'any', tell if current Person is coordinator of at least one team. @@ -272,31 +281,31 @@ class Person(User, FakePerson): except (ObjectDoesNotExist, AttributeError): return False - def is_committer(self, team): + def is_committer(self: "Person", team: "Team") -> bool: try: self.role_set.get(team__id=team.id, role__in=["committer", "coordinator"]) return True except (ObjectDoesNotExist, AttributeError): return False - def is_reviewer(self, team): + def is_reviewer(self: "Person", team: "Team") -> bool: try: self.role_set.get(team__id=team.id, role__in=["reviewer", "committer", "coordinator"]) return True except (ObjectDoesNotExist, AttributeError): return False - def is_translator(self, team): + def is_translator(self: "Person", team: "Team") -> bool: try: self.role_set.get(team__id=team.id) return True except (ObjectDoesNotExist, AttributeError): return False - def is_maintainer_of(self, module): + def is_maintainer_of(self: "Person", module: "Module") -> bool: return module in self.maintains_modules.all() - def get_languages(self) -> tuple["Language"]: + def get_languages(self: "Person") -> tuple["Language"]: all_person_languages: set["Language"] = set(super().get_languages()) all_teams_for_person = set(role.team for role in self.role_set.select_related("team")) diff --git a/people/templatetags/people.py b/people/templatetags/people.py index 4ef12ab3a1cb6fff45f13bb406b437287e5270ad..2b866b47fe4a3c577fe265c75d83286df2bbae85 100644 --- a/people/templatetags/people.py +++ b/people/templatetags/people.py @@ -12,12 +12,14 @@ register = template.Library() @register.filter -def people_list(lst): +def people_list(list_of_people: list[Person]) -> str: """ Format a people list, with clickable person name. """ # Translators: string used as separator in person list - return format_html_join(_(", "), '{}', ((pers.get_absolute_url(), pers.name) for pers in lst)) + return format_html_join( + _(", "), '{}', ((pers.get_absolute_url(), pers.name) for pers in list_of_people) + ) @register.filter diff --git a/people/tests.py b/people/tests.py index 97b45e6251c0baf68f32fca6e92e997ff3a06eef..36be83f39e849260b7058c279abf023dc3cb9919 100644 --- a/people/tests.py +++ b/people/tests.py @@ -1,4 +1,5 @@ from datetime import timedelta +from pathlib import Path from unittest import mock, skipUnless from unittest.mock import patch @@ -15,6 +16,7 @@ from django.urls import reverse from django.utils import timezone from django.utils.safestring import SafeData from django.utils.translation import gettext as _ +from requests import Response from common.utils import pyicu_present from languages.models import Language @@ -58,6 +60,7 @@ class ForgeAccountFieldTest(TestCase): password="password", # nosec ) p.full_clean() + self.assertEqual(p.forge_account, username) def test_invalid_forge_account_username_special_character(self): invalid_forge_account_names = [ @@ -75,7 +78,8 @@ class ForgeAccountFieldTest(TestCase): class PeopleTestCase(TestCase): - def _create_person(self, seq="", **kwargs): + @staticmethod + def _create_person(seq="", **kwargs): pn = Person(first_name="John", last_name="Nothing", email="jn%s@devnull.com" % seq, username="jn%s" % seq) for key, arg in kwargs.items(): setattr(pn, key, arg) @@ -367,20 +371,26 @@ class PeopleTestCase(TestCase): 'src="https://seccdn.libravatar.org/avatar/618b8b6c1c973c780ec218242c49cbe7?s=80&d=identicon&r=g">', ) - @mock.patch("people.forms.urlopen") - def test_get_image_size(self, m_urlopen): - with open(find("img/nobody.png"), mode="rb") as fh: - m_urlopen.return_value = fh - self.assertEqual(forms.get_image_size("https://l10n.gnome.org/static/img/nobody.png"), (48, 48)) + @mock.patch("people.forms.requests.get") + def test_get_image_size(self, requests_get): + # pylint: disable=unspecified-encoding + with Path.open(find("img/nobody.png"), mode="rb") as fh: + response = Response() + response.status_code = 200 + response.raw = fh + requests_get.return_value = response + + self.assertEqual(forms.get_image_size_from_url("https://l10n.gnome.org/static/img/nobody.png"), (48, 48)) def test_invalid_images(self): invalid_urls = ( "absolutely invalid", "http://hopefullythisurlshouldnotexist.com/grstzqwer.jpg", + "file:///etc/services", ) for url in invalid_urls: with self.assertRaises(ValidationError): - forms.get_image_size(url) + forms.get_image_size_from_url(url) @skipUnless(pyicu_present, "PyICU package is required for this test") def test_all_languages_list_order(self): @@ -435,8 +445,9 @@ class PeopleTestCase(TestCase): person = Person.objects.create(username="john", forge_account="john") self.assertFalse(person.forge_account_exists) + @staticmethod # noinspection PyMethodMayBeStatic - def _test_user_coordinator_with_or_without_forge_account_prepare_data(self) -> tuple["Person", "Team"]: + def _test_user_coordinator_with_or_without_forge_account_prepare_data() -> tuple["Person", "Team"]: person = Person.objects.create(username="john", forge_account=None) language = Language.objects.create(name="French", locale="fr", plurals="plurals=2; plural=(n > 1)") team = Team.objects.create(name="French", use_workflow=True) @@ -501,7 +512,7 @@ class PeopleTestCase(TestCase): class PeopleViewsTestCase(TestCase): - fixtures = ["test_data.json"] + fixtures = ("test_data.json",) def setUp(self) -> None: self.coordinator = Person.objects.get(username="john") @@ -580,7 +591,7 @@ class PeopleViewsTestCase(TestCase): } self.client.post(reverse("admin:people_person_changelist"), data) self.assertEqual(len(mail.outbox), 1) - self.assertIn("Dear %(username)s" % {"username": self.coordinator.username}, mail.outbox[0].body) + self.assertIn(f"Dear {self.coordinator.username}", mail.outbox[0].body) self.assertIn("to inform you that your profile is missing your username on GitLab", mail.outbox[0].body) @override_settings(LANGUAGE_CODE="en") diff --git a/people/views.py b/people/views.py index 9bd16c988f14a6235be2b7ab2c2f95221cf609af..56e0cee93d10228c866f44ebc77fe62e7e0e13fd 100644 --- a/people/views.py +++ b/people/views.py @@ -1,4 +1,5 @@ from operator import itemgetter +from typing import Any from django.conf import settings from django.conf.locale import LANG_INFO @@ -6,7 +7,7 @@ from django.contrib import messages from django.contrib.auth.decorators import login_required from django.contrib.auth.forms import PasswordChangeForm from django.db import IntegrityError, transaction -from django.http import HttpResponseRedirect +from django.http import HttpRequest, HttpResponse, HttpResponseRedirect from django.shortcuts import get_object_or_404, render from django.urls import reverse from django.utils.safestring import mark_safe @@ -26,7 +27,7 @@ class PeopleListView(ListView): model = Person template_name = "people/person_list.html" - def get_context_data(self, **kwargs): + def get_context_data(self: "PeopleListView", **kwargs) -> dict[str, Any]: # noqa: ANN003 context = super().get_context_data(**kwargs) context["pageSection"] = "teams" return context @@ -38,7 +39,7 @@ class PersonDetailView(DetailView): context_object_name = "person" template_name = "people/person_detail.html" - def get_context_data(self, **kwargs): + def get_context_data(self: "PersonDetailView", **kwargs) -> dict[str, Any]: # noqa: ANN003 context = super().get_context_data(**kwargs) states = State.objects.filter(action__person=self.object).distinct() user_on_own_page = self.request.user.is_authenticated and self.object.username == self.request.user.username @@ -55,11 +56,12 @@ class PersonDetailView(DetailView): messages.WARNING, mark_safe( _( - "You did not register a GitLab account in your profile and are the coordinator of a translation team. " - "If you do not have any yet, please register on the GNOME GitLab platform (%s) and indicate your " - "username in your Damned Lies profile." + "You did not register a GitLab account in your profile and are the coordinator of a " + "translation team. If you do not have any yet, please register on the GNOME GitLab platform " + "(%s) and indicate your username in your Damned Lies profile." ) - % f'{settings.GNOME_GITLAB_DOMAIN_NAME}' + % f'' + f"{settings.GNOME_GITLAB_DOMAIN_NAME}" ), # nosec ) @@ -84,11 +86,11 @@ class PersonEditView(UpdateView): form_class = DetailForm template_name = "people/person_detail_change_form.html" - def get_object(self, **kwargs): + def get_object(self: "PersonEditView", **kwargs) -> "Person": # noqa: ANN003, ARG002 self.kwargs["slug"] = self.request.user.username return super().get_object() - def get_context_data(self, **kwargs): + def get_context_data(self: "PersonEditView", **kwargs) -> dict[str, Any]: # noqa: ANN003 context = super().get_context_data(**kwargs) context["pageSection"] = "teams" context["on_own_page"] = self.object.username == self.request.user.username @@ -98,13 +100,13 @@ class PersonEditView(UpdateView): context["all_languages"] = lc_sorted(all_languages, key=itemgetter(1)) return context - def form_invalid(self, form): + def form_invalid(self: "PersonEditView", form: DetailForm) -> HttpResponse: messages.error(self.request, _("Sorry, the form is not valid.")) return super().form_invalid(form) @login_required -def person_team_join(request): +def person_team_join(request: HttpRequest) -> HttpResponse: """Handle the form to join a team""" person = get_object_or_404(Person, username=request.user.username) if request.method == "POST": @@ -120,7 +122,7 @@ def person_team_join(request): team.send_mail_to_coordinator( subject=gettext_lazy("A new person joined your team"), message=gettext_lazy("%(name)s has just joined your translation team on %(site)s"), - messagekw={"name": person.name, "site": settings.SITE_DOMAIN}, + message_kwargs={"name": person.name, "site": settings.SITE_DOMAIN}, ) except IntegrityError: messages.info(request, _("You are already member of this team.")) @@ -139,7 +141,7 @@ def person_team_join(request): @login_required @require_POST -def person_team_leave(request, team_slug): +def person_team_leave(request: HttpRequest, team_slug: str) -> HttpResponseRedirect: person = get_object_or_404(Person, username=request.user.username) team = get_object_or_404(Team, name=team_slug) try: @@ -154,7 +156,7 @@ def person_team_leave(request, team_slug): @login_required -def person_password_change(request): +def person_password_change(request: HttpRequest) -> HttpResponse: """Handle the generic form to change the password""" person = get_object_or_404(Person, username=request.user.username) @@ -177,7 +179,7 @@ def person_password_change(request): @login_required @require_POST -def person_create_token(request): +def person_create_token(request: HttpRequest) -> HttpResponseRedirect: person = get_object_or_404(Person, username=request.user.username) person.auth_token = Person.generate_token() person.save() @@ -186,7 +188,7 @@ def person_create_token(request): @login_required @require_POST -def person_delete_token(request): +def person_delete_token(request: HttpRequest) -> HttpResponseRedirect: person = get_object_or_404(Person, username=request.user.username) person.auth_token = "" # nosec person.save() diff --git a/pyproject.toml b/pyproject.toml index c67b60ec748ef3a88bf2667f494ef4a6c52983e9..88a0628c2f7166b124a652c863d89a1ae55c4834 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -90,7 +90,8 @@ exclude = [ # Damned Lies settings "damnedlies/settings*", # Virtual environment - "*venv*" + "*venv*", + "*tests*" ] [tool.ruff.lint] @@ -120,9 +121,13 @@ select = [ # pep8-naming "N", # isort - "I" + "I", + # flake8-annotation + "ANN", + # arguments + "ARG" ] -ignore = ["RUF001", "RUF002", "RUF003"] +ignore = ["RUF001", "RUF002", "RUF003"] # , "PLR6301", "C901", "PLR0912", "PLR0915", "PLR0904", "PLR0917", "PLR0913"] [tool.ruff.lint.mccabe] max-complexity = 10 @@ -132,7 +137,7 @@ ignore-overlong-task-comments = false max-doc-length = 119 [tool.ruff.lint.pylint] -max-returns = 1 +max-returns = 3 max-statements = 40 [tool.coverage.run] diff --git a/stats/admin.py b/stats/admin.py index 970eb6f5bbc4ac17d6501e5dbb765741bac86d79..c13ffc7db48c4ba062c9127c441ce7912fb8bf26 100644 --- a/stats/admin.py +++ b/stats/admin.py @@ -69,14 +69,14 @@ class DomainInline(admin.StackedInline): def formfield_for_dbfield(self, db_field, **kwargs): if db_field.name == "description": kwargs["widget"] = forms.Textarea(attrs={"rows": "1", "cols": "20"}) - elif db_field.name in ("name", "layout"): + elif db_field.name in {"name", "layout"}: kwargs["widget"] = forms.TextInput(attrs={"size": "20"}) - elif db_field.name in ("red_filter", "extra_its_dirs"): + elif db_field.name in {"red_filter", "extra_its_dirs"}: kwargs["widget"] = forms.Textarea(attrs={"rows": "1", "cols": "40"}) return super().formfield_for_dbfield(db_field, **kwargs) def formfield_for_foreignkey(self, db_field, request, **kwargs): - if db_field.name in ("branch_from", "branch_to") and hasattr(self, "parent_obj") and self.parent_obj: + if db_field.name in {"branch_from", "branch_to"} and hasattr(self, "parent_obj") and self.parent_obj: kwargs["queryset"] = self.parent_obj.branch_set.all() return super().formfield_for_foreignkey(db_field, request, **kwargs) @@ -120,7 +120,7 @@ class ModuleAdmin(admin.ModelAdmin): }, ), ) - inlines = [BranchInline, DomainInline] + inlines = (BranchInline, DomainInline) search_fields = ("name", "homepage", "comment", "vcs_web") list_display = ("__str__", "name", "vcs_root", "ext_platform", "archived") list_filter = ("archived",) @@ -177,8 +177,11 @@ class CategoryAdmin(admin.ModelAdmin): class ReleaseAdmin(admin.ModelAdmin): list_display = ("name", "status", "weight", "string_frozen") list_editable = ("weight",) - inlines = [CategoryInline] - actions = ["copy_release", "delete_release"] + inlines = (CategoryInline,) + actions = ( + "copy_release", + "delete_release", + ) def copy_release(self, request, queryset): """Copy an existing release and use master branches""" @@ -260,7 +263,7 @@ class InformationInline(admin.TabularInline): class StatisticsAdmin(admin.ModelAdmin): search_fields = ("language__name", "branch__module__name") raw_id_fields = ("branch", "domain", "language", "full_po", "part_po") - inlines = [InformationInline] + inlines = (InformationInline,) class PoFileAdmin(admin.ModelAdmin): diff --git a/stats/doap.py b/stats/doap.py index 9337055311129b9805c1ff7c9296a790cbaacb36..c18231e89ab1d95f2f2245f43428e47bfdd38f1a 100644 --- a/stats/doap.py +++ b/stats/doap.py @@ -19,7 +19,7 @@ class DOAPFileParseError(Exception): """ -class DOAPElementNotFound(Exception): +class DOAPElementNotFoundError(Exception): """ When the DOAP element is not found in the DOAP Document. """ @@ -67,7 +67,7 @@ class DOAPParser: maintainers_tags = self.project_root.findall("{%s}maintainer" % DOAPParser.DOAP_PREFIX) if len(maintainers_tags) == 0: - raise DOAPElementNotFound("No maintainer found in this DOAP file.") + raise DOAPElementNotFoundError("No maintainer found in this DOAP file.") person_attributes = [ "{http://xmlns.com/foaf/0.1/}name", @@ -104,14 +104,14 @@ class DOAPParser: homepage_tag = self.project_root.find("{%s}homepage" % DOAPParser.DOAP_PREFIX) if homepage_tag is not None: return homepage_tag.attrib.get("{http://www.w3.org/1999/02/22-rdf-syntax-ns#}resource") - raise DOAPElementNotFound("No homepage in this DOAP file.") + raise DOAPElementNotFoundError("No homepage in this DOAP file.") @property def bugtracker_url(self): bugtracker_tag = self.project_root.find("{%s}bug-database" % DOAPParser.DOAP_PREFIX) if bugtracker_tag is not None: return bugtracker_tag.attrib.get("{http://www.w3.org/1999/02/22-rdf-syntax-ns#}resource") - raise DOAPElementNotFound("No bugtracker URL in this DOAP file.") + raise DOAPElementNotFoundError("No bugtracker URL in this DOAP file.") def _slugify(val): @@ -179,7 +179,7 @@ def _update_doap_maintainers(doap_parser: DOAPParser, module: "Module"): # Drop maintainers not in the DOAP file for maintainer in current_maintainers_to_drop.values(): module.maintainers.remove(maintainer) - except DOAPElementNotFound as doap_element_not_found: + except DOAPElementNotFoundError as doap_element_not_found: logger.error(doap_element_not_found) @@ -189,7 +189,7 @@ def _update_doap_bugtracker_url(doap_parser: DOAPParser, module: "Module"): logger.debug("Update DOAP: bugtracker url found for module ‘%s’: %s", module.name, bugtracker_url) if bugtracker_url and bugtracker_url != module.bugs_base: module.bugs_base = bugtracker_url - except DOAPElementNotFound as doap_element_not_found: + except DOAPElementNotFoundError as doap_element_not_found: logger.error(doap_element_not_found) @@ -199,5 +199,5 @@ def _update_doap_homepage_url(doap_parser: DOAPParser, module: "Module"): logger.debug("Update DOAP: home url for module ‘%s’:: %s", module.name, homepage_url) if homepage_url and homepage_url != module.homepage: module.homepage = homepage_url - except DOAPElementNotFound as doap_element_not_found: + except DOAPElementNotFoundError as doap_element_not_found: logger.error(doap_element_not_found) diff --git a/stats/management/commands/update-stats.py b/stats/management/commands/update-stats.py index 09da1cd5d5ca2d2c1bd043d0a14858ca3575c55a..6c1e36a748db048a9ddc6f8f463f3074cb0b6ec2 100644 --- a/stats/management/commands/update-stats.py +++ b/stats/management/commands/update-stats.py @@ -4,6 +4,7 @@ from django.core.mail import mail_admins from django.core.management.base import BaseCommand, CommandError from django.db.models import Q +from damnedlies import logger from stats.models import Branch, Module, Release @@ -29,8 +30,6 @@ class Command(BaseCommand): parser.add_argument("--debug", action="store_true", default=False, help="activate interactive debug mode") def handle(self, **options): - if options["debug"]: - breakpoint() self.force_stats = options["force"] if options["module"] and options["branch"]: # Update the specific branch(es) of a module @@ -42,7 +41,7 @@ class Command(BaseCommand): self.stderr.write("No modules match `%s`." % options["module"]) for i, module in enumerate(modules): if module.archived and not self.force_stats: - self.stderr.write("The module '%s' is archived. Skipping..." % module.name) + logger.info("The module ‘%s’ is archived. Skipping…", module.name) continue branches = [] for branch_arg in options["branch"]: @@ -52,8 +51,8 @@ class Command(BaseCommand): try: branches.append(module.branch_set.get(name=branch_arg)) except Branch.DoesNotExist: - self.stderr.write( - "Unable to find branch '%s' for module '%s' in the database." % (branch_arg, module.name) + logger.error( + "Unable to find branch ‘%s’ for module ‘%s’ in the database.", branch_arg, module.name ) continue self.update_branches(branches, checkout=(i < 1)) @@ -65,7 +64,7 @@ class Command(BaseCommand): except Module.DoesNotExist as exc: raise CommandError("Unable to find a module named '%s' in the database" % options["module"]) from exc if module.archived and not self.force_stats: - self.stderr.write("The module '%s' is archived. Skipping..." % module.name) + logger.info("The module ‘%s’ is archived. Skipping…", module.name) else: self.update_branches(module.branch_set.all()) @@ -94,10 +93,10 @@ class Command(BaseCommand): def update_branches(self, branches, checkout=True): for branch in branches: - self.stdout.write("Updating stats for %s.%s..." % (branch.module.name, branch.name)) + logger.info("Updating stats for %s.%s", branch.module.name, branch.name) try: branch.update_statistics(self.force_stats, checkout=checkout) except Exception as exc: tbtext = traceback.format_exc() - mail_admins("Error while updating %s %s" % (branch.module.name, branch.name), tbtext) - self.stderr.write("Error while updating stats for %s %s: %s" % (branch.module.name, branch.name, exc)) + mail_admins(f"Error while updating {branch.module.name} {branch.name}", tbtext) + logger.error("Error while updating stats for %s %s: %s", branch.module.name, branch.name, exc) diff --git a/stats/models.py b/stats/models.py index f64af59ee93a7dfc2ef979cbc75b49e99e7aaac0..51b909c5b4ed27f4fa420c7e858ead5cf14ad83a 100644 --- a/stats/models.py +++ b/stats/models.py @@ -33,6 +33,7 @@ from translate.convert import sub2po from common.utils import is_site_admin, run_shell_command from damnedlies import logger from languages.models import Language +from languages.models import Language from people.models import Person from stats import repos, signals, utils from stats.doap import update_doap_infos @@ -74,7 +75,7 @@ BRANCH_HEAD_NAMES = ( ) -class UnableToCommit(Exception): +class UnableToCommitError(Exception): pass @@ -121,9 +122,9 @@ class Module(models.Model): if self.ext_platform: if comment: comment += "
" - comment = "%s%s" % ( - comment, - _( + comment = "{comment}{message_with_link}".format( + comment=comment, + message_with_link=_( 'Translations for this module are externally hosted. Please go to the ' "external platform to see how you can submit your translation." ) @@ -288,7 +289,7 @@ class Branch(models.Model): shutil.rmtree(str(self.output_directory("ui"))) def __str__(self): - return "%s (%s)" % (self.name, self.module) + return f"{self.name} ({self.module})" def __hash__(self): return hash(self.pk) @@ -439,7 +440,8 @@ class Branch(models.Model): self, domain_type: str, mandatory_languages: Iterable[Language] = () ) -> OrderedDict[str, list["StatisticsBase"]]: """ - Get statistics list of type ``domain_type`` (``ui`` or ``doc``), in a dict of lists, key is domain.name (POT in 1st position) + Get statistics list of type ``domain_type`` (``ui`` or ``doc``), in a dict of lists, key is domain.name + (POT in 1st position) :param domain_type: Domain.DOMAIN_TYPE_CHOICES. 'ui' or 'doc'. :type domain_type: str @@ -448,7 +450,6 @@ class Branch(models.Model): :return: statistics list of type ``domain_type`` :rtype: :: - stats = {'po': [potstat, polang1, polang2, ...], 'po-tips': [potstat, polang1, polang2, ...]} """ @@ -607,9 +608,7 @@ class Branch(models.Model): # 4. Compare with old pot files, various checks # ***************************** - previous_pot = self.output_directory(domain.dtype) / ( - "%s.%s.pot" % (domain.pot_base(), self.name_escaped) - ) + previous_pot = self.output_directory(domain.dtype) / (f"{domain.pot_base()}.{self.name_escaped}.pot") if not potfile: logger.error("Can’t generate template file (POT) for %s/%s.", self.module.name, domain.name) if previous_pot.exists(): @@ -659,11 +658,11 @@ class Branch(models.Model): dom_langs = domain.get_lang_files(self.checkout_path) for lang, pofile in dom_langs: outpo = self.output_directory(domain.dtype) / ( - "%s.%s.%s.po" % (domain.pot_base(), self.name_escaped, lang) + f"{domain.pot_base()}.{self.name_escaped}.{lang}.po" ) if ( not force - and changed_status in (utils.NOT_CHANGED, utils.CHANGED_ONLY_FORMATTING) + and changed_status in {utils.NOT_CHANGED, utils.CHANGED_ONLY_FORMATTING} and outpo.exists() and pofile.stat().st_mtime < outpo.stat().st_mtime and lang not in langs_with_ext_errors @@ -797,8 +796,7 @@ class Branch(models.Model): """ with ModuleLock(self.module): self.update_repository() - success = self._repository.cherry_pick(commit_hash) - return success + return self._repository.cherry_pick(commit_hash) class Domain(models.Model): @@ -851,7 +849,7 @@ class Domain(models.Model): ordering = ("-dtype", "name") def __str__(self): - return "%s (%s/%s)" % (self.name, self.module.name, self.get_dtype_display()) + return f"{self.name} ({self.module.name}/{self.get_dtype_display()})" @property def base_directory(self) -> str: @@ -893,7 +891,7 @@ class Domain(models.Model): def can_build_docs(self, branch): try: return self.dtype == "doc" and self.doc_format(branch) - except utils.UndetectableDocFormat: + except utils.UndetectableDocFormatError: return False def get_po_path(self, locale): @@ -1055,7 +1053,7 @@ class Domain(models.Model): pot_method = "gettext" if branch.uses_meson else "intltool" return pot_method - def get_xgettext_command(self, branch) -> list[str]: + def get_xgettext_command(self, branch) -> tuple[list[str], dict[str, str]]: # Note: the command will be run from the po directory. xgettext_args = [] vcs_path = branch.checkout_path / self.base_directory @@ -1068,17 +1066,17 @@ class Domain(models.Model): ("--output", f"{self.pot_base()}.pot"), ]: if opt not in xgettext_args: - xgettext_args.extend([opt, value] if value != "" else [opt]) + xgettext_args.extend([opt, value] if value else [opt]) if (vcs_path / "POTFILES.in").exists(): - xgettext_args = ["--files-from", "POTFILES.in"] + xgettext_args + xgettext_args = ["--files-from", "POTFILES.in", *xgettext_args] elif (vcs_path / "POTFILES").exists(): - xgettext_args = ["--files-from", "POTFILES"] + xgettext_args + xgettext_args = ["--files-from", "POTFILES", *xgettext_args] else: raise RuntimeError(f"No POTFILES file found in {self.base_directory}") - if not os.path.exists(utils.ITS_DATA_DIR): + if not utils.ITS_DATA_DIR.exists(): utils.collect_its_data() - env = {"GETTEXTDATADIRS": os.path.dirname(utils.ITS_DATA_DIR)} + env = {"GETTEXTDATADIRS": utils.ITS_DATA_DIR.parent} if self.extra_its_dirs: env["GETTEXTDATADIRS"] = ":".join( [env["GETTEXTDATADIRS"]] @@ -1107,7 +1105,7 @@ class Domain(models.Model): # Added last as some chars in it may disturb CLI parsing if self.module.bugs_base: xgettext_args.extend(["--msgid-bugs-address", self.module.bugs_base]) - return ["xgettext"] + xgettext_args, env + return ["xgettext", *xgettext_args], env def commit_info(self, branch: "Branch", language: "Language") -> tuple[Path, bool, Path]: """ @@ -1120,7 +1118,7 @@ class Domain(models.Model): :raises UnableToCommit: file cannot be committed. """ if branch.is_vcs_readonly: - raise UnableToCommit(_("The repository is read only")) + raise UnableToCommitError(_("The repository is read only")) absolute_po_path = branch.checkout_path / self.get_po_path(language.locale) po_file_already_exists = absolute_po_path.exists() @@ -1128,7 +1126,7 @@ class Domain(models.Model): if not po_file_already_exists and self.linguas_location != "no": linguas_file_path = branch.checkout_path / self.base_directory / "LINGUAS" if not linguas_file_path.exists(): - raise UnableToCommit( + raise UnableToCommitError( _("Sorry, adding new translations when the LINGUAS file is not known is not supported.") ) return absolute_po_path, po_file_already_exists, linguas_file_path @@ -1140,7 +1138,7 @@ class Domain(models.Model): # Custom (or no) linguas location if self.linguas_location == "no": return {"langs": None, "error": ""} - elif self.linguas_location.rsplit("/", maxsplit=1)[-1] == "LINGUAS": + if self.linguas_location.rsplit("/", maxsplit=1)[-1] == "LINGUAS": return utils.read_linguas_file(base_path / self.linguas_location) variable = "ALL_LINGUAS" @@ -1152,10 +1150,8 @@ class Domain(models.Model): langs = linguas_file.read_variable(variable) return { "langs": langs.split() if langs is not None else None, - "error": gettext_noop( - "Entry for this language is not present in %(var)s variable in %(file)s file." - % {"var": variable, "file": file_path} - ), + "error": _("Entry for this language is not present in %(var)s variable in %(file)s file.") + % {"var": variable, "file": file_path}, } # Standard linguas location if self.dtype == "ui": @@ -1243,9 +1239,9 @@ class Release(models.Model): def compute_percentage(x, y): return int(x / y * 100) - for k in stats: - stats[k]["stats"] = list(map(compute_percentage, stats[k]["stats"], totals)) - stats[k]["diff"] = stats[k]["stats"][-1] - stats[k]["stats"][0] + for key, stat in stats.items(): + stat["stats"] = list(map(compute_percentage, stat["stats"], totals)) + stat["diff"] = stat["stats"][-1] - stat["stats"][0] return stats def total_strings(self): @@ -1498,11 +1494,10 @@ class Release(models.Model): """Get statistics for a specific language, producing the stats data structure Used for displaying the language-release template""" - stats = { + return { "doc": Statistics.get_lang_stats_by_type(lang, "doc", self), "ui": Statistics.get_lang_stats_by_type(lang, "ui", self), } - return stats def get_lang_files(self, lang, dtype): """Return a list of all po files of a lang for this release, preceded by the more recent modification date @@ -1523,7 +1518,7 @@ class Release(models.Model): for stat in pot_stats: if stat.full_po.updated > last_modif_date: last_modif_date = stat.full_po.updated - key = "%s-%s" % (stat.branch_id, stat.domain_id) + key = f"{stat.branch_id}-{stat.domain_id}" lang_stat = po_stats.get(key, stat) file_path = lang_stat.po_path(reduced=partial) if os.access(file_path, os.R_OK): @@ -1552,7 +1547,7 @@ class Category(models.Model): unique_together = ("release", "branch") def __str__(self): - return "%s (%s, %s)" % (self.name, self.release, self.branch) + return f"{self.name} ({self.release}, {self.branch})" class PoFile(models.Model): @@ -1573,7 +1568,7 @@ class PoFile(models.Model): db_table = "pofile" def __str__(self): - return "%s (%s/%s/%s)" % (self.path, self.translated, self.fuzzy, self.untranslated) + return f"{self.path} ({self.translated}/{self.fuzzy}/{self.untranslated})" @property def url(self): @@ -1588,7 +1583,7 @@ class PoFile(models.Model): return self.prefix / self.path.lstrip("/") def filename(self): - return os.path.basename(self.path) + return Path(self.path).name def pot_size(self, words=False): if words: @@ -1687,13 +1682,9 @@ class Statistics(models.Model, StatisticsBase): self.info_list = [] def __str__(self): - """String representation of the object""" - return "%s (%s-%s) %s (%s)" % ( - self.branch.module.name, - self.domain.dtype, - self.domain.name, - self.branch.name, - self.get_lang(), + return ( + f"{self.branch.module.name} ({self.domain.dtype}-{self.domain.name}) " + f"{self.branch.name} ({self.get_lang()})" ) def translated(self, scope="full"): @@ -1786,13 +1777,11 @@ class Statistics(models.Model, StatisticsBase): def filename(self, potfile=False, reduced=False): if not self.is_pot_stats() and not potfile: - return "%s.%s.%s.%spo" % ( - self.domain.pot_base(), - self.branch.name_escaped, - self.language.locale, - reduced and "reduced." or "", + return ( + f"{self.domain.pot_base()}.{self.branch.name_escaped}." + f"{self.language.locale}.{'reduced.' if reduced else ''}po" ) - return "%s.%s.%spot" % (self.domain.pot_base(), self.branch.name_escaped, reduced and "reduced." or "") + return f"{self.domain.pot_base()}.{self.branch.name_escaped}.{'reduced.' if reduced else ''}pot" def pot_stats_summary(self) -> str: """ @@ -1867,9 +1856,8 @@ class Statistics(models.Model, StatisticsBase): stats["total"] += 1 if fig.get("fuzzy", 0): stats["fuzzy"] += 1 - else: - if fig.get("translated", 0): - stats["translated"] += 1 + elif fig.get("translated", 0): + stats["translated"] += 1 stats["untranslated"] = stats["total"] - (stats["translated"] + stats["fuzzy"]) if stats["total"] > 0: stats["prc"] = 100 * stats["translated"] / stats["total"] @@ -1900,7 +1888,7 @@ class Statistics(models.Model, StatisticsBase): if self.domain.dtype == "doc": subdir = "docs/" return utils.url_join( - "/POT/", "%s.%s" % (self.module_name, self.branch.name_escaped), subdir, self.filename(pot_file, reduced) + "/POT/", f"{self.module_name}.{self.branch.name_escaped}", subdir, self.filename(pot_file, reduced) ) def pot_url(self): @@ -2021,8 +2009,8 @@ class Statistics(models.Model, StatisticsBase): for e in self.information_set.all(): if ( not error - or e.type in ("error", "error-ext") - or (e.type in ("warn", "warn-ext") and error.type == "info") + or e.type in {"error", "error-ext"} + or (e.type in {"warn", "warn-ext"} and error.type == "info") ): error = e return error @@ -2371,10 +2359,10 @@ class Information(models.Model): def report_bug_url(self): link = self.statistics.branch.module.get_bugs_enter_url() - link += "&short_desc=%(short)s&content=%(short)s&comment=%(long)s" % { - "short": "Error regenerating template file (POT)", - "long": utils.ellipsize(utils.strip_html(self.get_description()), 1600), - } + link += "&short_desc={short}&content={short}&comment={long}".format( + short="Error regenerating template file (POT)", + long=utils.ellipsize(utils.strip_html(self.get_description()), 1600), + ) return link @@ -2398,3 +2386,15 @@ def try_int(value): def split_name(value): return tuple(try_int(part) for part in re.split(r"[-\.]", value)) + + +def get_release_statistics_in_a_given_language(language: "Language", archives: bool = False) -> list["Statistics"]: + """Get summary stats for all releases""" + if archives: + releases = Release.objects.all().filter(weight__lt=0).order_by("status", "-weight", "-name") + else: + releases = Release.objects.all().filter(weight__gte=0).order_by("status", "-weight", "-name") + stats = [] + for rel in releases: + stats.append(rel.total_for_lang(language)) + return stats diff --git a/stats/repos.py b/stats/repos.py index 4949500e8ba1057c987e8f903748b11754504b8b..fe49e2b710fadff54e0238c5a0c4a9a599ca0e4f 100644 --- a/stats/repos.py +++ b/stats/repos.py @@ -3,10 +3,15 @@ import shutil from abc import ABC, abstractmethod from enum import Enum, auto from pathlib import Path +from typing import TYPE_CHECKING from common.utils import CommandLineError, run_shell_command from damnedlies import logger +if TYPE_CHECKING: + from people.models import Person + from stats.models import Branch + class VCSRepositoryType(Enum): GIT = auto() @@ -26,7 +31,6 @@ class VCSRepository(ABC): def checkout(self) -> None: if self.exists(): self.update() - return else: self.init_checkout() @@ -190,7 +194,7 @@ class GitRepository(VCSRepository): run_shell_command(["git", "cherry-pick", "-x", commit_hash], raise_on_error=True, cwd=working_directory) # Try a push only if the cherry-pick succeeded - git_push_status, _, _ = run_shell_command( + _, _, _ = run_shell_command( ["git", "push", "origin", self.branch.name], raise_on_error=True, cwd=working_directory ) diff --git a/stats/templates/branch_detail.html b/stats/templates/branch_detail.html index 994bffd97aca6f161ede08bb8f7030a4bd335de0..13d77396f62426226a5d06006d29c7f09aa4db7e 100644 --- a/stats/templates/branch_detail.html +++ b/stats/templates/branch_detail.html @@ -8,7 +8,7 @@
{% get_branch_ui_statistics branch mandatory_languages as ui_statistics %} {% if ui_statistics|length %} - {% if not ui_statistics|length_is:"1" %} + {% if not ui_statistics|length == "1" %}

{% trans "Translation" %}

{% endif %} {% include "stats_show.html" with statistics=ui_statistics domain="ui" %} @@ -17,7 +17,7 @@
{% get_branch_documentation_statistics branch mandatory_languages as doc_statistics %} {% if doc_statistics|length %} - {% if not doc_statistics|length_is:"1" %} + {% if not doc_statistics|length == "1" %}

{% trans "Documentation" %}

{% endif %} {% include "stats_show.html" with statistics=doc_statistics domain="doc" %} diff --git a/stats/templatetags/stats_extras.py b/stats/templatetags/stats_extras.py index a36d59c8c590bb2be3a3bdf41b2c6ea624315813..4c51f9b5e9803a0ae54cb99b3150a84001ff9347 100644 --- a/stats/templatetags/stats_extras.py +++ b/stats/templatetags/stats_extras.py @@ -1,5 +1,5 @@ +import markdown as markdown_pkg from django import template -from django.conf import settings from django.utils.html import format_html from django.utils.safestring import mark_safe from django.utils.translation import get_language_bidi @@ -36,7 +36,7 @@ PROGRESS_BAR = ( def linked_with(value, arg): """This filter returns an object (passed in value) enclosed with its absolute url arg is the linked text""" - return "%s" % (value.get_absolute_url(), arg) + return f"{arg}" @register.filter @@ -45,7 +45,7 @@ def support_class(value): Value is a translation percentage""" if value >= 80: return "supported" - elif value >= 50: + if value >= 50: return "partially" return "not_supported" @@ -57,11 +57,9 @@ def support_class_total(stats): if stats["diff"] >= 0: return support_class(actual) - else: - if actual >= 80: - return "partially" - else: - return "not_supported" + if actual >= 80: + return "partially" + return "not_supported" @register.filter @@ -77,8 +75,8 @@ def browse_bugs(module, content): @register.simple_tag def num_stats_for_statistic(statistics: Statistics, scope: str = "full", for_words: bool = False) -> str: - if not isinstance(statistics, (Statistics, FakeLangStatistics, FakeSummaryStatistics)): - logger.error("The given statistic is not a Statistics object (is %s). Skipping." % type(statistics)) + if not isinstance(statistics, Statistics | FakeLangStatistics | FakeSummaryStatistics): + logger.error("The given statistic is not a Statistics object (is %s). Skipping.", type(statistics)) return _render_error_num_stats_template() if for_words: @@ -101,7 +99,7 @@ def num_stats_for_statistic(statistics: Statistics, scope: str = "full", for_wor @register.simple_tag def num_stats_for_pofile(statistics: PoFile, scope: str = "full", for_words: bool = False) -> str: if not isinstance(statistics, PoFile): - logger.error("The given statistic is not a PoFile (is %s). Skipping." % type(statistics)) + logger.error("The given statistic is not a PoFile (is %s). Skipping.", type(statistics)) return _render_error_num_stats_template() if for_words: @@ -127,7 +125,7 @@ def num_stats_for_pofile(statistics: PoFile, scope: str = "full", for_words: boo @register.simple_tag def num_stats_for_dict_of_stats(statistics: dict, scope: str = "full"): if not isinstance(statistics, dict): - logger.error("The given statistic is not a dictionary (is %s). Skipping." % type(statistics)) + logger.error("The given statistic is not a dictionary (is %s). Skipping.", type(statistics)) return _render_error_num_stats_template() translation_statistics = statistics @@ -159,14 +157,14 @@ def _render_statistics_template(translation_statistics: dict, show_zeros: bool) def _render_error_num_stats_template() -> str: - return mark_safe("""""" + _("Error") + "") + return mark_safe("""""" + _("Error") + "") # nosec @register.filter def vis_stats(stat, scope="full"): """Produce visual stats with green/red bar""" - bidi = get_language_bidi() and "right" or "left" - if isinstance(stat, (Statistics, FakeLangStatistics, FakeSummaryStatistics)): + bidi = "right" if get_language_bidi() else "left" + if isinstance(stat, Statistics | FakeLangStatistics | FakeSummaryStatistics): trans = stat.tr_percentage(scope) fuzzy = stat.fu_percentage(scope) untrans = stat.un_percentage(scope) @@ -197,7 +195,7 @@ def vis_stats(stat, scope="full"): @register.filter def vis_word_stats(stat, scope="full"): """Produce visual stats with green/red bar""" - if isinstance(stat, (Statistics, FakeLangStatistics, FakeSummaryStatistics)): + if isinstance(stat, Statistics | FakeLangStatistics | FakeSummaryStatistics): trans = stat.tr_word_percentage(scope) fuzzy = stat.fu_word_percentage(scope) untrans = stat.un_word_percentage(scope) @@ -213,7 +211,7 @@ def vis_word_stats(stat, scope="full"): return format_html( PROGRESS_BAR, **{ - "dir": get_language_bidi() and "right" or "left", + "dir": "right" if get_language_bidi() else "left", "trans": trans, "fuzzy": fuzzy, "tr_fu": trans + fuzzy, @@ -238,14 +236,5 @@ def markdown(value, arg=""): {{ value|markdown:"extension1_name,extension2_name..." }} """ - try: - import markdown - except ImportError: - if settings.DEBUG: - raise template.TemplateSyntaxError( - "Error in 'markdown' filter: The Python markdown library isn't installed." - ) - return value - else: - extensions = [e for e in arg.split(",") if e] - return mark_safe(markdown.markdown(value, extensions=extensions)) + extensions = [e for e in arg.split(",") if e] + return mark_safe(markdown_pkg.markdown(value, extensions=extensions)) diff --git a/stats/templatetags/translation_percentage.py b/stats/templatetags/translation_percentage.py index c5c97a0ffa72e42c1ff385dcf1b631d4c660b37b..2f969fbbfbac0579d20596894f53c85216b5decb 100644 --- a/stats/templatetags/translation_percentage.py +++ b/stats/templatetags/translation_percentage.py @@ -3,7 +3,7 @@ from typing import TYPE_CHECKING from django import template from django.db.models import QuerySet - +from damnedlies import logger from stats.models import Statistics register = template.Library() @@ -51,9 +51,9 @@ def translation_percentage_in_language_for_module( @register.simple_tag() def translation_percentage_color_type(translation_percentage: int): - if translation_percentage == 100: + full_translation, almost_full_translation = 100, 96 + if translation_percentage == full_translation: return "success" - elif 96 <= translation_percentage < 100: + if almost_full_translation <= translation_percentage < full_translation: return "warning" - else: - return "danger" + return "danger" diff --git a/stats/tests/tests.py b/stats/tests/tests.py index e0b17c8ffd1395c27285558d0b35a59f48a31b25..0fe18682e49e78df3b497b6b860241b1abaf1b3f 100644 --- a/stats/tests/tests.py +++ b/stats/tests/tests.py @@ -34,7 +34,7 @@ from stats.models import ( PoFile, Release, Statistics, - UnableToCommit, + UnableToCommitError, ) from ..repos import GitRepository, VCSRepository, VCSRepositoryType @@ -49,7 +49,7 @@ except ImportError: class ModuleTestCase(TestCase): - fixtures = ["test_data.json"] + fixtures = ("test_data.json",) SYS_DEPENDENCIES = ( ("gettext", "msgfmt"), ("intltool", "intltool-update"), @@ -276,7 +276,7 @@ class ModuleTestCase(TestCase): coord.save() self.client.login(username="john", password="john") response = self.client.get(reverse("module_edit_branches", args=[self.mod]), follow=True) - self.assertContains(response, "you're not allowed to edit") + self.assertContains(response, "you’re not allowed to edit") # Test now with appropriate permissions coord.is_superuser = True coord.save() @@ -419,16 +419,16 @@ class ModuleTestCase(TestCase): domain = self.mod.domain_set.get(name="po") fr_lang = Language.objects.get(locale="fr") - with self.assertRaises(UnableToCommit): + with self.assertRaises(UnableToCommitError): # read-only VCS domain.commit_info(branch, fr_lang) # Setup as a writable repo self.mod.vcs_root = "git@gitlab.gnome.org:GNOME/%s.git" % self.mod.name linguas = branch.checkout_path / "po" / "LINGUAS" - os.remove(str(linguas)) + linguas.unlink() bem_lang = Language.objects.get(locale="bem") - with self.assertRaises(UnableToCommit): + with self.assertRaises(UnableToCommitError): # unaccessible LINGUAS file for a new lang domain.commit_info(branch, bem_lang) @@ -444,7 +444,7 @@ class ModuleTestCase(TestCase): domain = self.mod.domain_set.get(name="po") fr_lang = Language.objects.get(locale="fr") with PatchShellCommand(): - with self.assertRaisesRegex(UnableToCommit, "read only"): + with self.assertRaisesRegex(UnableToCommitError, "read only"): with self.assertLogs("stats.models", level="ERROR"): branch.commit_po(po_file, domain, fr_lang, Person.objects.get(username="john")) # Setup as a writable repo @@ -459,7 +459,8 @@ class ModuleTestCase(TestCase): "if [ -e .gitmodules ]; then git submodule update --init; fi", ) # User interface (existing language) - git_ops = update_repo_sequence + ( + git_ops = ( + *update_repo_sequence, "git add po/fr.po", # Quoting is done at the Popen level "git commit -m Update French translation --author John Coordinator ", @@ -473,7 +474,8 @@ class ModuleTestCase(TestCase): # User interface (new language) bem_lang = Language.objects.get(locale="bem") - git_ops = update_repo_sequence + ( + git_ops = ( + *update_repo_sequence, "git add po/bem.po", "git add po/LINGUAS", "git commit -m Add Bemba translation --author John Coordinator ", @@ -492,7 +494,8 @@ class ModuleTestCase(TestCase): # Documentation domain = self.mod.domain_set.get(name="help") - git_ops = update_repo_sequence + ( + git_ops = ( + *update_repo_sequence, "git add help/fr/fr.po", "git commit -m Update French translation --author John Coordinator ", "git log -n1 --format=oneline", @@ -547,7 +550,8 @@ class ModuleTestCase(TestCase): "git clean -dfq", "if [ -e .gitmodules ]; then git submodule update --init; fi", ) - git_ops = update_repo_sequence + ( + git_ops = ( + *update_repo_sequence, "git cherry-pick -x", "git push origin master", ) @@ -558,7 +562,7 @@ class ModuleTestCase(TestCase): class TestDOAPFiles(TestCase): - fixtures = ["test_data.json"] + fixtures = ("test_data.json",) def setUp(self): self.module = Module.objects.get(name="gnome-hello") @@ -702,7 +706,7 @@ class DomainTests(TestModuleBase): "--msgid-bugs-address", "https://gitlab.gnome.org/GNOME/testmodule/issues", ], - {"GETTEXTDATADIRS": os.path.dirname(utils.ITS_DATA_DIR)}, + {"GETTEXTDATADIRS": utils.ITS_DATA_DIR.parent}, ), ) @@ -713,7 +717,7 @@ class DomainTests(TestModuleBase): domain = Domain.objects.create( module=self.mod, name="release-notes", dtype="doc", layout="help_docbook/{lang}/{lang}.po" ) - potfile, errs = utils.generate_doc_pot_file(self.branch, domain) + _, errs = utils.generate_doc_pot_file(self.branch, domain) self.assertEqual(errs, []) doc_format = domain.doc_format(self.branch) self.assertEqual(doc_format.format, "docbook") @@ -754,9 +758,9 @@ class DomainTests(TestModuleBase): pot_params="https://l10n.gnome.org/POT/damned-lies.master/damned-lies.master.pot", ) with patch("stats.models.request.urlopen") as mock_urlopen: - with open(Path(__file__).parent / "empty.pot", mode="rb") as fh: + with Path.open(Path(__file__).parent / "empty.pot", mode="rb") as fh: mock_urlopen.return_value = fh - potfile, errs = dom.generate_pot_file(self.branch) + potfile, _ = dom.generate_pot_file(self.branch) self.addCleanup(os.remove, str(potfile)) self.assertTrue(potfile.exists()) @@ -764,7 +768,7 @@ class DomainTests(TestModuleBase): dom = Domain.objects.create( module=self.mod, name="shell-po", dtype="ui", pot_method="shell", pot_params="touch some.pot" ) - potfile, errs = dom.generate_pot_file(self.branch) + potfile, _ = dom.generate_pot_file(self.branch) self.addCleanup(os.remove, str(potfile)) self.assertTrue(potfile.exists()) @@ -777,7 +781,7 @@ class DomainTests(TestModuleBase): layout="subtitles/po/{lang}.po", pot_method="subtitles", ) - potfile, errs = dom.generate_pot_file(self.branch) + potfile, _ = dom.generate_pot_file(self.branch) self.addCleanup(os.remove, str(potfile)) self.assertTrue(potfile.exists()) self.assertEqual( @@ -799,7 +803,7 @@ class DomainTests(TestModuleBase): class StatisticsTests(TestCase): - fixtures = ["test_data.json"] + fixtures = ("test_data.json",) def test_get_global_stats(self): rel = Release.objects.get(name="gnome-3-8") @@ -948,7 +952,7 @@ class StatisticsTests(TestCase): class FigureTests(TestCase): - fixtures = ["test_data.json"] + fixtures = ("test_data.json",) def test_figure_view(self): url = reverse("docimages", args=["gnome-hello", "help", "master", "fr"]) @@ -1078,7 +1082,7 @@ class UtilsTests(TestModuleBase): temp_file = tempfile.NamedTemporaryFile(delete=False) self.addCleanup(os.remove, temp_file.name) utils.po_grep(it_path, temp_file.name, "locations|desktop.in.h") - with open(temp_file.name) as fh: + with Path.open(temp_file.name, encoding="utf-8") as fh: content = fh.read() self.assertGreater(len(content), 0) self.assertNotIn("desktop.in.h", content) @@ -1117,7 +1121,7 @@ class OtherTests(TestCase): class VCSRepositoryTest(TestCase): - fixtures = ["test_data.json"] + fixtures = ("test_data.json",) def test_create_new_vcs_repository(self): branch = Branch.objects.get(pk=1) diff --git a/stats/tests/tests_views.py b/stats/tests/tests_views.py index 5681f792bc775599993766d57666e9e23c25dc57..5104cc90375a8a8022a16ea3602008e5f7ec475a 100644 --- a/stats/tests/tests_views.py +++ b/stats/tests/tests_views.py @@ -11,7 +11,7 @@ from teams.models import Role, Team class TestModuleViews(TestCase): - fixtures = ["test_data"] + fixtures = ("test_data",) @test_scratchdir def test_module_branch_view_shows_all_mandatory_languages_for_authenticated_user(self): diff --git a/stats/tests/utils.py b/stats/tests/utils.py index 1b3816015bc7c2548fc814249e0395e33d745a52..d0fdd8510c66de220a915e7705a98367c0b40817 100644 --- a/stats/tests/utils.py +++ b/stats/tests/utils.py @@ -1,4 +1,3 @@ -import os import shutil import tarfile import tempfile @@ -36,9 +35,8 @@ class PatchShellCommand: from common.utils import run_shell_command return run_shell_command(cmd, *args, **kwargs) - else: - # Pretend the command was successful - return 0, "", "" + # Pretend the command was successful + return 0, "", "" def __exit__(self, *args): self.patcher1.stop() @@ -58,7 +56,7 @@ def test_scratchdir(test_func): settings.POT_DIR.mkdir(parents=True, exist_ok=True) settings.LOG_DIR = settings.SCRATCHDIR / "logs" settings.LOG_DIR.mkdir(parents=True, exist_ok=True) - tar_path = os.path.join(os.path.dirname(os.path.abspath(__file__)), "gnome-hello.tar.gz") + tar_path = Path(__file__).resolve().parent / "gnome-hello.tar.gz" with tarfile.open(tar_path) as gnome_hello_tar: gnome_hello_tar.extractall(settings.SCRATCHDIR / "git") try: diff --git a/stats/utils.py b/stats/utils.py index 8c6e47b540f222276724ac529dcc00da2aec002f..25c9a4eb507a441defd7f37e0e6d219b73c3dda1 100644 --- a/stats/utils.py +++ b/stats/utils.py @@ -6,6 +6,7 @@ import shutil import time from itertools import islice from pathlib import Path +from typing import TYPE_CHECKING, ClassVar from unittest.mock import MagicMock from django.conf import settings @@ -36,6 +37,9 @@ ITS_DATA_DIR = settings.SCRATCHDIR / "data" / "its" # monkey-patch ttk (https://github.com/translate/translate/issues/2129) orig_addunit = TranslationStore.addunit +if TYPE_CHECKING: + from stats.models import Branch, Domain + def patched_add_unit(self, unit): # Prevent two header units in the same store @@ -49,7 +53,7 @@ def patched_add_unit(self, unit): TranslationStore.addunit = patched_add_unit -class UndetectableDocFormat(Exception): +class UndetectableDocFormatError(Exception): pass @@ -61,13 +65,15 @@ class DocFormat: self.vcs_path = branch.domain_path(domain) self.makefile = MakefileWrapper.find_file(branch, [self.vcs_path]) if self.makefile is None: - raise UndetectableDocFormat(gettext_noop("Unable to find a makefile for module %s") % branch.module.name) + raise UndetectableDocFormatError( + gettext_noop("Unable to find a makefile for module %s") % branch.module.name + ) has_page_files = any(f.suffix == ".page" for f in self.list_c_files()) self.format = "mallard" if has_page_files else "docbook" self.tool = "itstool" def __repr__(self): - return "%s format=%s, tool=%s>" % (self.__class__, self.format, self.tool) + return f"{self.__class__} format={self.format}, tool={self.tool}>" @property def use_meson(self): @@ -90,7 +96,7 @@ class DocFormat: break if not sources: # Last try: only one xml/docbook file in C/... - xml_files = [f for f in self.list_c_files() if f.suffix in (".xml", ".docbook")] + xml_files = [f for f in self.list_c_files() if f.suffix in {".xml", ".docbook"}] if len(xml_files) == 1: sources.append(xml_files[0].name) else: @@ -105,7 +111,7 @@ class DocFormat: sources += source_list.split() elif source_list: sources += source_list - return [Path("C", src) for src in sources if src not in ("", "$(NULL)")] + return [Path("C", src) for src in sources if src not in {"", "$(NULL)"}] def command(self, potfile, files): cmd = ["%sitstool" % ITSTOOL_PATH, "-o", str(potfile)] @@ -139,7 +145,11 @@ class DocFormat: class MakefileWrapper: - default_makefiles = ["Makefile.am", "meson.build", "CMakeLists.txt"] + default_makefiles = ( + "Makefile.am", + "meson.build", + "CMakeLists.txt", + ) @classmethod def find_file(cls, branch, vcs_paths, file_name=None): @@ -151,10 +161,9 @@ class MakefileWrapper: if file_path.exists(): if file_name == "meson.build": return MesonfileWrapper(branch, file_path) - elif file_name == "CMakeLists.txt": + if file_name == "CMakeLists.txt": return CMakefileWrapper(branch, file_path) - else: - return MakefileWrapper(branch, file_path) + return MakefileWrapper(branch, file_path) def __init__(self, branch, path): self.branch = branch @@ -209,9 +218,9 @@ class MakefileWrapper: class MesonfileWrapper(MakefileWrapper): - i18n_gettext_kwargs = {"po_dir", "data_dirs", "type", "languages", "args", "preset"} - gnome_yelp_kwargs = {"sources", "media", "symlink_media", "languages"} - ignorable_kwargs = {"install_dir"} + i18n_gettext_kwargs: ClassVar[set[str]] = {"po_dir", "data_dirs", "type", "languages", "args", "preset"} + gnome_yelp_kwargs: ClassVar[set[str]] = {"sources", "media", "symlink_media", "languages"} + ignorable_kwargs: ClassVar[set[str]] = {"install_dir"} readable = True @cached_property @@ -222,13 +231,12 @@ class MesonfileWrapper(MakefileWrapper): content = re.sub(r"(" + "|".join(possible_kwargs) + ") ?:", r"\1=", content) # ignore if/endif sections content = re.sub(r"^if .*endif$", "", content, flags=re.M | re.S) - content = ( + return ( content.replace("true", "True") .replace("false", "False") .replace("i18n = import('i18n')", "") .replace("gnome = import('gnome')", "") ) - return content @cached_property def _parsed_variables(self): @@ -292,8 +300,7 @@ class MesonfileWrapper(MakefileWrapper): def strip_mock(value): if isinstance(value, list): return [v for v in value if not isinstance(v, MagicMock)] - else: - return value if not isinstance(value, MagicMock) else None + return value if not isinstance(value, MagicMock) else None for var in variables: if var in parsed_vars: @@ -354,7 +361,7 @@ def ellipsize(val, length): def check_program_presence(prog_name): """Test if prog_name is an available command on the system""" - status, output, err = run_shell_command(["which", prog_name]) + status, _, _ = run_shell_command(["which", prog_name]) return status == 0 @@ -375,15 +382,6 @@ def po_grep(in_file, out_file, filter_): pogrep.rungrep(str(in_file), out, None, grepfilter) except Exception: pass - # command-line variant: - """ - cmd = 'pogrep --invert-match --header --search=locations --regexp ' \ - '"gschema\\.xml\\.in|schemas\\.in\" %(full_po)s %(part_po)s' % { - 'full_po': in_file, - 'part_po': out_file, - } - run_shell_command(cmd) - """ def check_potfiles(po_path): @@ -392,7 +390,7 @@ def check_potfiles(po_path): errors = [] run_shell_command(["rm", "-f", "missing", "notexist"], cwd=po_path) - status, output, errs = run_shell_command(["intltool-update", "-m"], cwd=po_path) + status, _, _ = run_shell_command(["intltool-update", "-m"], cwd=po_path) if status != STATUS_OK: errors.append(("error", gettext_noop("Errors while running “intltool-update -m” check."))) @@ -433,7 +431,7 @@ def generate_doc_pot_file(branch: "Branch", domain: "Domain") -> tuple[str, list potbase = domain.pot_base() potfile = doc_format.vcs_path / "C" / (potbase + ".pot") command = doc_format.command(potfile, files) - status, output, errs = run_shell_command(command, cwd=doc_format.vcs_path) + status, _, errs = run_shell_command(command, cwd=doc_format.vcs_path) if status != STATUS_OK: return "", [ @@ -450,23 +448,21 @@ def generate_doc_pot_file(branch: "Branch", domain: "Domain") -> tuple[str, list if not potfile.exists(): return "", [] - else: - return potfile, [] + return potfile, [] def pot_diff_status(pota, potb): - (status, output, errs) = run_shell_command('diff "%s" "%s"|wc -l' % (pota, potb)) + _, output, _ = run_shell_command(f'diff "{pota}" "{potb}"|wc -l') # POT generation date always change and produce a 4 line diff if int(output) <= 4: return NOT_CHANGED, "" result_all, result_add_only = potdiff.diff(str(pota), str(potb)) - if not len(result_all) and not len(result_add_only): + if not result_all and not result_add_only: return CHANGED_ONLY_FORMATTING, "" - elif len(result_add_only): + if result_add_only: return CHANGED_WITH_ADDITIONS, result_add_only - else: - return CHANGED_NO_ADDITIONS, result_all + return CHANGED_NO_ADDITIONS, result_all def remove_diff_markup_on_list_of_additions(differences: list[str]) -> list[str]: @@ -508,7 +504,7 @@ def check_po_conformity(pofile): # Check if PO file is in UTF-8 if input_file != "-": - command = 'msgconv -t UTF-8 "%s" | diff -i -I \'^#~\' -u "%s" - >/dev/null' % (pofile, pofile) + command = f'msgconv -t UTF-8 "{pofile}" | diff -i -I \'^#~\' -u "{pofile}" - >/dev/null' status, _, _ = run_shell_command(command, env=C_ENV) if status != STATUS_OK: errors.append(("warn", gettext_noop("PO file “%s” is not UTF-8 encoded.") % pofile.name)) @@ -520,15 +516,14 @@ def check_po_quality(pofile, filters): Check po file quality with the translate-toolkit pofilter tool. http://docs.translatehouse.org/projects/translate-toolkit/en/latest/commands/pofilter.html """ - if not os.path.exists(pofile): + if not Path(pofile).exists(): return _("The file “%s” does not exist") % pofile status, out, errs = run_shell_command( ["pofilter", "--progress=none", "--gnome", "-t", "accelerators", "-t"] + list(filters) + [pofile] ) if status == STATUS_OK: return out - else: - return _("Error running pofilter: %s") % errs + return _("Error running pofilter: %s") % errs def po_file_statistics(pofile): @@ -602,9 +597,7 @@ def get_ui_linguas(branch, subdir): if linguas.exists(): return read_linguas_file(linguas) # AS_ALL_LINGUAS is a macro that takes all po files by default - status, output, errs = run_shell_command( - "grep -qs AS_ALL_LINGUAS %s%sconfigure.*" % (branch.checkout_path, os.sep) - ) + status, _, _ = run_shell_command(f"grep -qs AS_ALL_LINGUAS {branch.checkout_path}{os.sep}configure.*") if status == 0: return {"langs": None, "error": gettext_noop("No need to edit LINGUAS file or variable for this module")} @@ -664,7 +657,7 @@ def collect_its_data(): branch.checkout() for file_path in files: src = branch.checkout_path / file_path - dest = ITS_DATA_DIR / os.path.basename(file_path) + dest = ITS_DATA_DIR / Path(file_path).parent shutil.copyfile(str(src), dest) @@ -718,19 +711,19 @@ def check_identical_figures(fig_stats, base_path, lang): def add_custom_header(po_path, header, value): """Add a custom po file header""" - grep_cmd = """grep "%s" %s""" % (header, po_path) + grep_cmd = f"""grep "{header}" {po_path}""" status = 1 last_headers = ["Content-Transfer-Encoding", "Plural-Forms"] - while status != 0 and last_headers != []: - (status, output, errs) = run_shell_command(grep_cmd) + while status != 0 and last_headers: + status, output, _ = run_shell_command(grep_cmd) if status != 0: # Try to add header - cmd = """sed -i '/^\"%s/ a\\"%s: %s\\\\n"' %s""" % (last_headers.pop(), header, value, po_path) - (stat, out, err) = run_shell_command(cmd) - if status == 0 and "%s: %s" % (header, value) not in output: + cmd = f"""sed -i '/^\"{last_headers.pop()}/ a\\"{header}: {value}\\\\n"' {po_path}""" + run_shell_command(cmd) + if status == 0 and f"{header}: {value}" not in output: # Set header value - cmd = """sed -i '/^\"%s/ c\\"%s: %s\\\\n"' %s""" % (header, header, value, po_path) - (stat, out, err) = run_shell_command(cmd) + cmd = f"""sed -i '/^\"{header}/ c\\"{header}: {value}\\\\n"' {po_path}""" + run_shell_command(cmd) def exclude_untrans_messages(potfile): @@ -755,7 +748,7 @@ def exclude_untrans_messages(potfile): def is_po_reduced(file_path): - status, output, errs = run_shell_command(["grep", "X-DamnedLies-Scope: partial", file_path]) + status, _, _ = run_shell_command(["grep", "X-DamnedLies-Scope: partial", file_path]) return status == 0 diff --git a/stats/views.py b/stats/views.py index be2fde9e32f623a61841f450d9594bac564fee96..b15594a97c96522fbfcc4228ff01d587c72a9817 100644 --- a/stats/views.py +++ b/stats/views.py @@ -1,5 +1,6 @@ import os from datetime import date +from itertools import chain from django.contrib import messages from django.contrib.auth.decorators import login_required @@ -16,7 +17,8 @@ from django.urls import reverse from django.utils.translation import get_language, get_language_info from django.utils.translation import gettext as _ -from common.utils import MIME_TYPES, get_user_locale, run_shell_command +from common.utils import MIME_TYPES, run_shell_command +from languages.models import get_user_language_from_request from languages.models import Language from people.models import Person from stats.forms import ModuleBranchForm @@ -34,11 +36,13 @@ from stats.models import ( def modules(request, return_format="html"): - if return_format in ("json", "xml"): + if return_format in {"json", "xml"}: data = serializers.serialize(return_format, Module.objects.all()) return HttpResponse(data, content_type=MIME_TYPES[return_format]) - if return_format in ("html",): + if return_format in { + "html", + }: user_language = get_language_info(get_language()) # Looks like a complex query bug actually extracts all Statistics in the language for the HEAD branches. @@ -70,7 +74,7 @@ def modules(request, return_format="html"): } return render(request, "module_list.html", context) - raise HttpResponseBadRequest( + return HttpResponseBadRequest( _("The return format type you indicated is not allowed. Allowed values are: html, json, xml.") ) @@ -85,7 +89,7 @@ def module(request, module_name: str): "branches": branches, "can_edit_branches": mod.can_edit_branches(request.user), "can_refresh": can_refresh_branch(request.user), - "user_language": get_user_locale(request), + "user_language": get_user_language_from_request(request), # Will allow to display the language of the user, even if no statistic exist in # the user language "mandatory_languages": Person.get_by_user(request.user).get_languages(), @@ -116,7 +120,7 @@ def module_branch(request, module_name, branch_name): def module_edit_branches(request, module_name): mod = get_object_or_404(Module, name=module_name) if not mod.can_edit_branches(request.user): - messages.error(request, "Sorry, you're not allowed to edit this module's branches") + messages.error(request, _("Sorry, you’re not allowed to edit this module’s branches")) return HttpResponseRedirect(mod.get_absolute_url()) if request.method == "POST": @@ -160,18 +164,18 @@ def module_edit_branches(request, module_name): try: branch = Branch(module=mod, name=branch_name) branch.save() - messages.success(request, "The new branch %s has been added" % branch_name) + messages.success(request, _("The new branch %s has been added") % branch_name) updated = True # Send message to gnome-i18n? except Exception as e: - messages.error(request, "Error adding the branch '%s': %s" % (branch_name, str(e))) + messages.error(request, _("Error adding the branch '%s': %s") % (branch_name, str(e))) branch = None else: - messages.success(request, "Branches updated") + messages.success(request, _("Branches updated")) if updated: form = ModuleBranchForm(mod) else: - messages.error(request, "Sorry, the form is not valid") + messages.error(request, _("Sorry, the form is not valid")) else: form = ModuleBranchForm(mod) context = { @@ -189,7 +193,8 @@ def branch_refresh(request, branch_id): if ModuleLock(branch.module).is_locked: messages.info( request, - "A statistics update is already running for branch %s of module %s" % (branch.name, branch.module.name), + _("A statistics update is already running for branch %s of module %s.") + % (branch.name, branch.module.name), ) else: try: @@ -197,8 +202,8 @@ def branch_refresh(request, branch_id): except Exception as exc: messages.error( request, - "An error occurred while updating statistics for branch %s of module %s:
%s" - % (branch.name, branch.module.name, exc), + f"An error occurred while updating statistics for branch " + f"{branch.name} of module {branch.module.name}: {exc}", ) else: messages.success( @@ -233,46 +238,38 @@ def docimages(request, module_name, potbase, branch_name, langcode): def dynamic_po(request, module_name, branch_name, domain_name, filename): """Generates a dynamic po file from the POT file of a branch""" try: - locale, ext = filename.split(".") + locale, _ = filename.split(".") if locale.endswith("-reduced"): locale, reduced = locale[:-8], True else: reduced = False - except Exception: - raise Http404 + except Exception as exc: + raise Http404 from exc language = get_object_or_404(Language.objects.select_related("team"), locale=locale) branch = get_object_or_404(Branch, module__name=module_name, name=branch_name) try: domain = branch.connected_domains[domain_name] - except KeyError: - raise Http404 + except KeyError as ke: + raise Http404 from ke potfile = get_object_or_404(Statistics, branch=branch, domain=domain, language=None) file_path = potfile.po_path(reduced=reduced) if not os.access(file_path, os.R_OK): raise Http404 - dyn_content = """# %(lang)s translation for %(pack)s. -# Copyright (C) %(year)s %(pack)s's COPYRIGHT HOLDER -# This file is distributed under the same license as the %(pack)s package.\n""" % { - "lang": language.name, - "pack": module_name, - "year": date.today().year, - } + dyn_content = f"""# {language.name} translation for {module_name}. +# Copyright (C) {date.today().year} {module_name}'s COPYRIGHT HOLDER +# This file is distributed under the same license as the {module_name} package.\n""" if request.user.is_authenticated: person = Person.get_by_user(request.user) - dyn_content += "# %(name)s <%(email)s>, %(year)s.\n#\n" % { - "name": person.name, - "email": person.email, - "year": date.today().year, - } + dyn_content += f"# {person.name} <{person.email}>, {date.today().year}.\n#\n" else: dyn_content += "# FIRST AUTHOR , YEAR.\n#\n" command = f"msginit --locale={locale} --no-translator --input={file_path} --output-file=-" try: - status, output, err = run_shell_command(command, raise_on_error=True) + _, output, _ = run_shell_command(command, raise_on_error=True) except OSError: return HttpResponse(f"Unable to produce a new .po file for language {locale}, msginit command failed.") lines = output.split("\n") @@ -284,18 +281,18 @@ def dynamic_po(request, module_name, branch_name, domain_name, filename): continue # Transformations new_line = { - '"Project-Id-': '"Project-Id-Version: %s %s\\n"' % (module_name, branch_name), + '"Project-Id-': f'"Project-Id-Version: {module_name} {branch_name}\\n"', '"Last-Transl': '"Last-Translator: FULL NAME \\n"', - '"Language-Te': '"Language-Team: %s <%s>\\n"' - % (language.name, language.team and language.team.mailing_list or "%s@li.org" % locale), + '"Language-Te': f'"Language-Team: {language.name} ' + f'<{language.team if language.team and language.team.mailing_list else "%s@li.org" % locale}>\\n"', '"Content-Typ': '"Content-Type: text/plain; charset=UTF-8\\n"', - '"Plural-Form': '"Plural-Forms: %s;\\n"' % (language.plurals or "nplurals=INTEGER; plural=EXPRESSION"), + '"Plural-Form': f'"Plural-Forms: {language.plurals or "nplurals=INTEGER; plural=EXPRESSION"};\\n"', }.get(line[:12], line) if line != new_line and line[-3:] != '\\n"': # Skip the wrapped part of the replaced line skip_next_line = True dyn_content += new_line + "\n" - if line == "": + if not line: # Output the remaining part of the file untouched dyn_content += "\n".join(lines[i + 1 :]) break @@ -309,9 +306,7 @@ def dynamic_po(request, module_name, branch_name, domain_name, filename): def releases(request, format="html"): active_releases = Release.objects.filter(weight__gte=0).order_by("status", "-weight", "-name") old_releases = Release.objects.filter(weight__lt=0).order_by("status", "-weight", "-name") - if format in ("json", "xml"): - from itertools import chain - + if format in {"json", "xml"}: data = serializers.serialize(format, chain(active_releases, old_releases)) return HttpResponse(data, content_type=MIME_TYPES[format]) context = { @@ -326,12 +321,12 @@ def release(request, release_name, format="html"): release = get_object_or_404(Release, name=release_name) if format == "xml": return render(request, "release_detail.xml", {"release": release}, content_type=MIME_TYPES[format]) - context = {"pageSection": "releases", "user_language": get_user_locale(request), "release": release} + context = {"pageSection": "releases", "user_language": get_user_language_from_request(request), "release": release} return render(request, "release_detail.html", context) def compare_by_releases(request, dtype, rels_to_compare): - if dtype not in dict(Domain.DOMAIN_TYPE_CHOICES).keys(): + if dtype not in dict(Domain.DOMAIN_TYPE_CHOICES): raise Http404("Wrong domain type") releases = list( Release.objects.in_bulk( @@ -348,14 +343,12 @@ def compare_by_releases(request, dtype, rels_to_compare): return render(request, "release_compare.html", context) -# ********** Utility function ********** -def can_refresh_branch(user): - """Return True if user is authorized to force statistics refresh""" - if not user.is_authenticated: - return False - if user.has_perm("stats.change_module"): +def can_refresh_branch(user: Person) -> bool: + if user.is_authenticated and user.has_perm("stats.change_module"): return True + + # Otherwise, if the user is coordinator or any team person = Person.get_by_user(user) - if person.is_coordinator(team="any"): + if isinstance(person, Person) and person.is_coordinator(team="any"): return True return False diff --git a/teams/admin.py b/teams/admin.py index b5dcd0088f1683725fe97196060f59bbc5e347c5..9e43a338ea1bb3847492e2f7263eaa3336b9d103 100644 --- a/teams/admin.py +++ b/teams/admin.py @@ -1,4 +1,5 @@ from django.contrib import admin +from django.forms import Field from languages.models import Language from teams.models import Role, Team @@ -18,10 +19,10 @@ class RoleInline(admin.TabularInline): class TeamAdmin(admin.ModelAdmin): search_fields = ("name", "description") list_display = ("description", "use_workflow", "webpage_url") - list_filter = ["use_workflow"] - inlines = [LanguageInline, RoleInline] + list_filter = ("use_workflow",) + inlines = (LanguageInline, RoleInline) - def formfield_for_dbfield(self, db_field, **kwargs): + def formfield_for_dbfield(self: "TeamAdmin", db_field: type["Field"], **kwargs) -> type["Field"]: # noqa ANN003 # Reduced text area for aliases field = super().formfield_for_dbfield(db_field, **kwargs) if db_field.name == "description": diff --git a/teams/forms.py b/teams/forms.py index d8d451fdb4356ccec974ad7cfc03406cd5754f1f..43a2d0a3ab419e6a44be81595e304d76fdaf487c 100644 --- a/teams/forms.py +++ b/teams/forms.py @@ -1,23 +1,30 @@ +from collections.abc import Generator +from typing import TYPE_CHECKING, Any, ClassVar + from django import forms from django.conf import settings +from django.http import HttpRequest from django.utils.translation import gettext as _ from common.utils import is_site_admin, send_mail from teams.models import ROLE_CHOICES, Role, Team +if TYPE_CHECKING: + from models import Person + class EditTeamDetailsForm(forms.ModelForm): class Meta: model = Team fields = ("webpage_url", "mailing_list", "mailing_list_subscribe", "use_workflow", "presentation") - widgets = { + widgets: ClassVar[dict[str, Any]] = { "webpage_url": forms.TextInput(attrs={"size": 60, "class": "form-control"}), "mailing_list": forms.TextInput(attrs={"size": 60, "class": "form-control"}), "mailing_list_subscribe": forms.TextInput(attrs={"size": 60, "class": "form-control"}), "presentation": forms.Textarea(attrs={"cols": 60, "class": "form-control"}), } - def __init__(self, user, *args, **kwargs): + def __init__(self: "EditTeamDetailsForm", user: "Person", *args, **kwargs): # noqa ANN003 super().__init__(*args, **kwargs) self.user = user if is_site_admin(user): @@ -37,7 +44,7 @@ class EditTeamDetailsForm(forms.ModelForm): initial=current_coord_pk, ) - def save(self, *args, **kwargs): + def save(self: "EditTeamDetailsForm", *args, **kwargs): # noqa ANN003 super().save(*args, **kwargs) if "coordinatorship" in self.changed_data and is_site_admin(self.user): # Change coordinator @@ -55,25 +62,27 @@ class EditTeamDetailsForm(forms.ModelForm): class EditMemberRoleForm(forms.Form): - def __init__(self, roles, *args, **kwargs): + def __init__(self: "EditMemberRoleForm", roles: list, *args, **kwargs): # noqa ANN003 super().__init__(*args, **kwargs) choices = [x for x in ROLE_CHOICES if x[0] != "coordinator"] choices.extend([("inactivate", _("Mark as Inactive")), ("remove", _("Remove From Team"))]) for role in roles: self.fields[str(role.pk)] = forms.ChoiceField( choices=choices, - label='%s' % (role.person.get_absolute_url(), role.person.name), + label=f'{role.person.name}', initial=role.role, ) if roles: self.fields["form_type"] = forms.CharField(widget=forms.HiddenInput, initial=roles[0].role) - def get_fields(self): + def get_fields(self: "EditMemberRoleForm") -> Generator[Any, None, None]: for key in self.fields: - if key not in ("form_type",): + if key not in { + "form_type", + }: yield self[key] - def save(self, request): + def save(self: "EditMemberRoleForm", request: HttpRequest) -> None: # noqa: ARG002 for key, field in self.fields.items(): form_value = self.cleaned_data[key] if field.initial != form_value: diff --git a/teams/models.py b/teams/models.py index 71a6518fa503e3c967092bb2db126fa4610fa05a..58ace0c959dd44230e3eeca211c94bc53e29c86b 100644 --- a/teams/models.py +++ b/teams/models.py @@ -1,8 +1,9 @@ from datetime import timedelta -from typing import TYPE_CHECKING +from typing import TYPE_CHECKING, Any from django.conf import settings from django.db import models +from django.db.models import QuerySet from django.urls import reverse from django.utils import timezone, translation from django.utils.translation import gettext as _ @@ -16,7 +17,7 @@ if TYPE_CHECKING: class TeamManager(models.Manager): - def all_with_coordinator(self): + def all_with_coordinator(self: "TeamManager") -> QuerySet: """ Returns all teams with the coordinator already prefilled. Use that function to reduce the size of extracted data and the numbers of objects @@ -39,7 +40,7 @@ class TeamManager(models.Manager): pass return teams - def all_with_roles(self): + def all_with_roles(self: "TeamManager") -> QuerySet: """ This method prefills team.coordinator/committer/reviewer/translator to reduce subsequent database access. @@ -82,35 +83,35 @@ class Team(models.Model): db_table = "team" ordering = ("description",) - def __init__(self, *args, **kwargs): + def __init__(self: "Team", *args, **kwargs) -> None: # noqa ANN003 models.Model.__init__(self, *args, **kwargs) - self.roles = {} + self.roles: dict[str, list[Person]] = {} - def __str__(self): + def __str__(self: "Team") -> str: return self.description - def get_absolute_url(self): + def get_absolute_url(self: "Team") -> str: return reverse("team_slug", args=[self.name]) - def can_edit(self, user): + def can_edit(self: "Team", user: "Person") -> bool: """Return True if user is allowed to edit this team user is a User (from request.user), not a Person """ return user.is_authenticated and user.username in [p.username for p in self.get_coordinators()] - def fill_role(self, role, person): + def fill_role(self: "Team", role: str, person: "Person") -> None: """Used by TeamManager to prefill roles in team""" if not self.roles: self.roles = {"coordinator": [], "committer": [], "reviewer": [], "translator": []} self.roles[role].append(person) - def get_description(self): + def get_description(self: "Team") -> str: return _(self.description) - def get_languages(self) -> tuple["Language"]: + def get_languages(self: "Team") -> tuple["Language"]: return tuple(self.language_set.all()) - def get_members_by_role_exact(self, role, only_active=True): + def get_members_by_role_exact(self: "Team", role: str, only_active: bool = True) -> list[Person]: """Return a list of active members""" try: return self.roles[role] @@ -121,19 +122,19 @@ class Team(models.Model): members = Person.objects.filter(role__team__id=self.id, role__role=role) return list(members) - def get_coordinators(self): + def get_coordinators(self: "Team") -> list[Person]: return self.get_members_by_role_exact("coordinator", only_active=False) - def get_committers_exact(self): + def get_committers_exact(self: "Team") -> list[Person]: return self.get_members_by_role_exact("committer") - def get_reviewers_exact(self): + def get_reviewers_exact(self: "Team") -> list[Person]: return self.get_members_by_role_exact("reviewer") - def get_translators_exact(self): + def get_translators_exact(self: "Team") -> list[Person]: return self.get_members_by_role_exact("translator") - def get_members_by_roles(self, roles, only_active=True): + def get_members_by_roles(self: "Team", roles: list[str], only_active: bool = True) -> list[Person]: """Requires a list of roles in argument""" try: members = [] @@ -146,13 +147,13 @@ class Team(models.Model): members = Person.objects.filter(role__team__id=self.id, role__role__in=roles) return list(members) - def get_committers(self): + def get_committers(self: "Team") -> list[Person]: return self.get_members_by_roles(["coordinator", "committer"]) - def get_reviewers(self): + def get_reviewers(self: "Team") -> list[Person]: return self.get_members_by_roles(["coordinator", "committer", "reviewer"]) - def get_translators(self): + def get_translators(self: "Team") -> list[Person]: """Don't use get_members_by_roles to provide an optimization""" try: members = [] @@ -163,19 +164,20 @@ class Team(models.Model): members = list(self.members.all()) return members - def get_inactive_members(self): + def get_inactive_members(self: "Team") -> list[Person]: """Return the inactive members""" - members = list(Person.objects.filter(role__team__id=self.id, role__is_active=False)) - return members + return list(Person.objects.filter(role__team__id=self.id, role__is_active=False)) - def send_mail_to_coordinator(self, subject, message, messagekw=None): + def send_mail_to_coordinator( + self: "Team", subject: str, message: str, message_kwargs: dict[str, Any] | None = None + ) -> None: """Send a message to the coordinator, in her language if available and if subject and message are lazy strings""" recipients = [pers.email for pers in self.get_coordinators() if pers.email] if not recipients: return with translation.override(self.language_set.first().locale): - message = "%s\n--\n" % (message % (messagekw or {}),) + message = "%s\n--\n" % (message % (message_kwargs or {}),) # noqa: UP031 message += _("This is an automated message sent from %s.") % settings.SITE_DOMAIN send_mail(str(subject), message, to=recipients, headers={settings.EMAIL_HEADER_NAME: "coordinator-mail"}) @@ -188,23 +190,25 @@ class FakeTeam: fake = 1 - def __init__(self, language): + def __init__(self: "FakeTeam", language: "Language") -> None: self.language = language self.description = _("No team for locale %s") % self.language.locale - def get_absolute_url(self): + def get_absolute_url(self: "FakeTeam") -> str: return reverse("team_slug", args=[self.language.locale]) - def can_edit(self, user): + def can_edit(self: "FakeTeam", _: "Person") -> bool: # noqa PLR6301 return False - def get_description(self): + def get_description(self: "FakeTeam") -> str: return self.language.locale - def get_languages(self): - return (self.language,) + def get_languages(self: "FakeTeam") -> str: + return _( + self.language, + ) - def get_coordinators(self): + def get_coordinators(self: "FakeTeam") -> list[Person]: # noqa PLR6301 return [] @@ -231,11 +235,11 @@ class Role(models.Model): db_table = "role" unique_together = ("team", "person") - def __str__(self): - return "%s is %s in %s team" % (self.person.name, self.role, self.team.description) + def __str__(self: "Role") -> str: + return f"{self.person.name} is {self.role} in {self.team.description} team" @classmethod - def inactivate_unused_roles(cls): + def inactivate_unused_roles(cls: "Role") -> None: """Inactivate the roles when login older than 180 days""" last_login = timezone.now() - timedelta(days=30 * 6) cls.objects.filter(person__last_login__lt=last_login, is_active=True).update(is_active=False) diff --git a/teams/templates/teams/team_detail.html b/teams/templates/teams/team_detail.html index 46f9ddba92f43a14019fc80c22b0574e0c406b97..3f266ac5a4c89b85e6c7138fbdfe75de2c63996e 100644 --- a/teams/templates/teams/team_detail.html +++ b/teams/templates/teams/team_detail.html @@ -113,7 +113,7 @@

{{ lang.get_name }} ({{ lang.locale }})

{% with 1 as show_all_modules_line %} - {% with lang.get_release_stats as stats %} + {% with lang.get_release_statistics_in_a_given_language as stats %} {% include "languages/language_release_summary.html" %} {% endwith %} {% endwith %} diff --git a/teams/tests.py b/teams/tests.py index 0000d51403b34f4ba0d7c714e956e2a0f6abaf70..c00a5b6c1d9a47b29f51e2a6a273fe9df9147cde 100644 --- a/teams/tests.py +++ b/teams/tests.py @@ -121,17 +121,17 @@ class TeamTests(TeamsAndRolesMixin, TestCase): members = team.get_committers() self.assertEqual(len(members), 2) for pc in members: - self.assertTrue(pc in [self.coordinator, self.committer]) + self.assertTrue(pc in {self.coordinator, self.committer}) members = team.get_reviewers() self.assertEqual(len(members), 3) for pc in members: - self.assertTrue(pc in [self.coordinator, self.committer, self.reviewer]) + self.assertTrue(pc in {self.coordinator, self.committer, self.reviewer}) members = team.get_translators() self.assertEqual(len(members), 4) for pc in members: - self.assertTrue(pc in [self.coordinator, self.committer, self.reviewer, self.translator]) + self.assertTrue(pc in {self.coordinator, self.committer, self.reviewer, self.translator}) def test_roles(self): self.run_roles_test(self.team_french) diff --git a/teams/views.py b/teams/views.py index b14e0a1c5c45864e8da27fa491fdbc7c6b7ecd14..fbd2327a982736d8e452465f2b164b80cc66c07e 100644 --- a/teams/views.py +++ b/teams/views.py @@ -3,6 +3,7 @@ from typing import Any from django.contrib.auth.decorators import login_required from django.http import ( HttpRequest, + HttpResponse, HttpResponseBadRequest, HttpResponseForbidden, HttpResponseRedirect, @@ -52,10 +53,10 @@ def context_data_for_view_get_person_teams_from_request(request: HttpRequest) -> } -def teams(request, format_requested="html"): +def teams(request: HttpRequest, format_requested: str = "html") -> HttpResponse: all_teams_with_coordinator = Team.objects.all_with_coordinator() - if format_requested in ("xml", "json"): + if format_requested in {"xml", "json"}: return render( request, "teams/team_list.%s" % format_requested, @@ -63,7 +64,9 @@ def teams(request, format_requested="html"): content_type=utils.MIME_TYPES[format_requested], ) - if format_requested in ("html",): + if format_requested in { + "html", + }: context = { "pageSection": "teams", "teams": utils.trans_sort_object_list(all_teams_with_coordinator, "description"), @@ -76,7 +79,7 @@ def teams(request, format_requested="html"): ) -def team(request, team_slug): +def team(request: HttpRequest, team_slug: str) -> HttpResponse: try: team = Team.objects.get(name=team_slug) mem_groups = ( @@ -149,7 +152,7 @@ def team(request, team_slug): return render(request, "teams/team_detail.html", context) -def team_edit(request, team_slug): +def team_edit(request: HttpRequest, team_slug: str) -> HttpResponse: team = get_object_or_404(Team, name=team_slug) if not (team.can_edit(request.user) or utils.is_site_admin(request.user)): return HttpResponseForbidden("You are not allowed to edit this team.") @@ -164,7 +167,7 @@ def team_edit(request, team_slug): @require_http_methods(["POST"]) @login_required -def connected_user_join_team(request, team_slug: str): +def connected_user_join_team(request: HttpRequest, team_slug: str) -> HttpResponseRedirect: person = get_object_or_404(Person, username=request.user.username) target_team = get_object_or_404(Team, name=team_slug) @@ -180,7 +183,7 @@ def connected_user_join_team(request, team_slug: str): @require_http_methods(["POST"]) @login_required -def connected_user_leave_team(request, team_slug: str): +def connected_user_leave_team(request: HttpRequest, team_slug: str) -> HttpResponseRedirect: person = get_object_or_404(Person, username=request.user.username) target_team = get_object_or_404(Team, name=team_slug) role = get_object_or_404(Role, team=target_team, person=person) diff --git a/vertimus/forms.py b/vertimus/forms.py index 1f0f5ff3aa0a552d62cea7f5dc1e2227c1148afe..7965b41fc7a1ddb2ab4149b13c5d3547677325a6 100644 --- a/vertimus/forms.py +++ b/vertimus/forms.py @@ -1,4 +1,6 @@ -import os +from collections.abc import Collection +from pathlib import Path +from typing import TYPE_CHECKING, Any from django import forms from django.core.exceptions import ValidationError @@ -10,6 +12,9 @@ from stats.models import Person from stats.utils import check_po_conformity from vertimus.models import Action, ActionCI, ActionSeparator +if TYPE_CHECKING: + from vertimus.models import State + class DisabledLabel(str): """String subclass to mark some label as disabled.""" @@ -18,7 +23,7 @@ class DisabledLabel(str): class DisablableSelect(forms.Select): """Custom widget to allow a Select option to be disabled.""" - def create_option(self, *args, **kwargs): + def create_option(self: "DisablableSelect", *args, **kwargs) -> dict[str, Any]: # noqa: ANN002, ANN003 context = super().create_option(*args, **kwargs) if isinstance(context["label"], DisabledLabel): context["attrs"]["disabled"] = True @@ -31,12 +36,12 @@ class DisablableSelect(forms.Select): class AuthorChoiceField(forms.ModelChoiceField): widget = DisablableSelect - def label_from_instance(self, obj): - if str(obj) == obj.username: - return DisabledLabel(gettext("%(name)s (full name missing)") % {"name": obj.username}) - if not obj.email: - return DisabledLabel(gettext("%(name)s (email missing)") % {"name": str(obj)}) - return str(obj) + def label_from_instance(self: "AuthorChoiceField", person: Person) -> str: # noqa: PLR6301 + if str(person) == person.username: + return DisabledLabel(gettext("%(name)s (full name missing)") % {"name": person.username}) + if not person.email: + return DisabledLabel(gettext("%(name)s (email missing)") % {"name": str(person)}) + return str(person) class ActionForm(forms.Form): @@ -52,7 +57,14 @@ class ActionForm(forms.Form): file = forms.FileField(label=_("File"), required=False, help_text=_("Upload a .po, .gz, .bz2, .xz or .png file")) send_to_ml = forms.BooleanField(label=_("Send message to the team mailing list"), required=False) - def __init__(self, current_user, state, actions, *args, **kwargs): + def __init__( + self: "ActionForm", + current_user: "Person", + state: "State", + actions: Collection[Action], + *args, # noqa: ANN002 + **kwargs, # noqa: ANN003 + ) -> None: super().__init__(*args, **kwargs) self.actions = actions self.current_user = current_user @@ -78,11 +90,11 @@ class ActionForm(forms.Form): "name": main_branch.name } - def clean_file(self): + def clean_file(self: "ActionForm") -> dict[str, Any]: data = self.cleaned_data["file"] if data: - ext = os.path.splitext(data.name)[1] - if ext not in (".po", ".gz", ".bz2", ".xz", ".png"): + ext = Path(data.name).suffix + if ext not in {".po", ".gz", ".bz2", ".xz", ".png"}: raise ValidationError(_("Only files with extension .po, .gz, .bz2, .xz or .png are admitted.")) # If this is a .po file, check validity (msgfmt) if ext == ".po": @@ -92,7 +104,7 @@ class ActionForm(forms.Form): ) return data - def clean(self): + def clean(self: "ActionForm") -> dict[str, Any]: cleaned_data = self.cleaned_data action_code = cleaned_data.get("action") if action_code is None: diff --git a/vertimus/models.py b/vertimus/models.py index 586489d3432ef3def6105f4678555367ba678022..b6d3017f130908e7112abea0fc9bbee3142a9905 100644 --- a/vertimus/models.py +++ b/vertimus/models.py @@ -1,8 +1,10 @@ +import abc import os import shutil import sys from datetime import timedelta from pathlib import Path +from typing import TYPE_CHECKING, Any, Optional from django.conf import settings from django.db import models @@ -17,19 +19,19 @@ from django.utils.translation import gettext_lazy as _ from common.utils import run_shell_command, send_mail from languages.models import Language from people.models import Person -from stats.models import Branch, Domain, PoFile, Statistics, UnableToCommit +from stats.models import Branch, Domain, PoFile, Statistics, UnableToCommitError from stats.signals import pot_has_changed from stats.utils import is_po_reduced, po_grep from teams.models import Role +if TYPE_CHECKING: + from django.db.models import QuerySet -class SendMailFailed(Exception): + +class SendMailFailedError(Exception): """Something went wrong while sending message""" -# -# States -# class State(models.Model): """State of a module translation""" @@ -46,7 +48,7 @@ class State(models.Model): verbose_name = "state" unique_together = ("branch", "domain", "language") - def __init__(self, *args, **kwargs): + def __init__(self: "State", *args, **kwargs) -> None: # noqa: ANN002, ANN003 super().__init__(*args, **kwargs) if self.name == "None" and getattr(self.__class__, "_name", "None") != "None": self.name = self.__class__._name @@ -62,39 +64,33 @@ class State(models.Model): "Committed": StateCommitted, }.get(self.name, State) - def __str__(self): - return "%s: %s %s (%s - %s)" % ( - self.name, - self.branch.module.name, - self.branch.name, - self.language.name, - self.domain.name, - ) + def __str__(self: "State") -> str: + return f"{self.name}: {self.branch.module.name} {self.branch.name} ({self.language.name} - {self.domain.name})" - def get_absolute_url(self): + def get_absolute_url(self: "State") -> str: return reverse("vertimus_by_ids", args=[self.branch_id, self.domain_id, self.language_id]) @property - def stats(self): + def stats(self: "State") -> Statistics | None: try: return Statistics.objects.get(branch=self.branch, domain=self.domain, language=self.language) except Statistics.DoesNotExist: return None - def able_to_commit(self): + def able_to_commit(self: "State") -> bool: try: self.domain.commit_info(self.branch, self.language) - except UnableToCommit: + except UnableToCommitError: return False return self.get_latest_po_file_action() is not None - def change_state(self, state_class, person=None): + def change_state(self: "State", state_class: type["State"], person: Person | None = None) -> None: self.name = state_class._name self.person = person self.__class__ = state_class self.save() - def _get_available_actions(self, person, action_names): + def _get_available_actions(self: "State", person: Person, action_names: list[str]) -> list["Action"]: # For archived modules, the only possible action is to archive the state (by the coordinator only) if self.branch.module.archived: if self.action_set.count() > 0 and person.is_coordinator(self.language.team): @@ -109,7 +105,11 @@ class State(models.Model): action_names.extend(("Separator", "IC", "AA")) return [eval("Action" + action_name)() for action_name in action_names] # FIXME: eval is unsafe - def get_action_sequence_from_level(self, level): + @abc.abstractmethod + def get_available_actions(self: "State", person: Person) -> list["Action"]: + raise NotImplementedError() + + def get_action_sequence_from_level(self: "State", level: int) -> int: """Get the sequence corresponding to the requested level. The first level is 1.""" if level < 0: @@ -126,7 +126,7 @@ class State(models.Model): sequence = query[0]["sequence"] return sequence - def involved_persons(self, extra_user=None): + def involved_persons(self: "State", extra_user: Person | None = None) -> "QuerySet[Person]": """ Return all persons having posted any action on the current state. """ @@ -134,7 +134,7 @@ class State(models.Model): return Person.objects.filter(Q(action__state_db=self) | Q(pk=extra_user.pk)).distinct() return Person.objects.filter(action__state_db=self).distinct() - def get_latest_po_file_action(self): + def get_latest_po_file_action(self: "State") -> Optional["Action"]: try: return Action.objects.filter(file__endswith=".po", state_db=self).latest("id") except Action.DoesNotExist: @@ -148,7 +148,7 @@ class StateNone(State): class Meta: proxy = True - def get_available_actions(self, person): + def get_available_actions(self: "StateNone", person: Person) -> list["Action"]: action_names = [] if (self.language.team and person.is_translator(self.language.team)) or person.is_maintainer_of( @@ -166,7 +166,7 @@ class StateTranslating(State): class Meta: proxy = True - def get_available_actions(self, person): + def get_available_actions(self: "StateTranslating", person: Person) -> list["Action"]: action_names = [] if self.person == person: @@ -182,7 +182,7 @@ class StateTranslated(State): class Meta: proxy = True - def get_available_actions(self, person): + def get_available_actions(self: "StateTranslated", person: Person) -> list["Action"]: action_names = [] if person.is_reviewer(self.language.team): @@ -208,7 +208,7 @@ class StateProofreading(State): class Meta: proxy = True - def get_available_actions(self, person): + def get_available_actions(self: "StateProofreading", person: Person) -> list["Action"]: action_names = [] if person.is_reviewer(self.language.team): @@ -226,7 +226,7 @@ class StateProofread(State): class Meta: proxy = True - def get_available_actions(self, person): + def get_available_actions(self: "StateProofread", person: "Person") -> list["Action"]: if person.is_reviewer(self.language.team): action_names = ["TC", "RP", "TR"] else: @@ -246,7 +246,7 @@ class StateToReview(State): class Meta: proxy = True - def get_available_actions(self, person): + def get_available_actions(self: "StateToReview", person: Person) -> list["Action"]: action_names = [] if person.is_translator(self.language.team): action_names.append("RT") @@ -261,7 +261,7 @@ class StateToCommit(State): class Meta: proxy = True - def get_available_actions(self, person): + def get_available_actions(self: "StateToCommit", person: Person) -> list["Action"]: if person.is_committer(self.language.team): action_names = ["RC", "TR", "UNDO"] if self.able_to_commit(): @@ -279,7 +279,7 @@ class StateCommitting(State): class Meta: proxy = True - def get_available_actions(self, person): + def get_available_actions(self: "StateCommitting", person: Person) -> list["Action"]: action_names = [] if person.is_committer(self.language.team): @@ -298,7 +298,7 @@ class StateCommitted(State): class Meta: proxy = True - def get_available_actions(self, person): + def get_available_actions(self: "StateCommitted", person: Person) -> list["Action"]: if person.is_committer(self.language.team): action_names = ["AA"] else: @@ -331,23 +331,25 @@ ACTION_NAMES = ( ) -def generate_upload_filename(instance, filename): +def generate_upload_filename(instance: type["ActionAbstract"], filename: str) -> str: if isinstance(instance, ActionArchived): - return "%s/%s" % (settings.UPLOAD_ARCHIVED_DIR, os.path.basename(filename)) + return f"{settings.UPLOAD_ARCHIVED_DIR}/{Path(filename).name}" + # Extract the first extension (with the point) - root, ext = os.path.splitext(filename) + filepath = Path(filename) + root, ext = Path(filepath.name), filepath.suffix # Check if a second extension is present - if os.path.splitext(root)[1] == ".tar": + if root.suffix == ".tar": ext = ".tar" + ext - new_filename = "%s-%s-%s-%s-%s%s" % ( - instance.state_db.branch.module.name, - instance.state_db.branch.name, - instance.state_db.domain.name, - instance.state_db.language.locale, - instance.state_db.id, - ext, + new_filename = ( + f"{instance.state_db.branch.module.name}-" + f"{instance.state_db.branch.name}-" + f"{instance.state_db.domain.name}-" + f"{instance.state_db.language.locale}-" + f"{instance.state_db.id}" + f"{ext}" ) - return "%s/%s" % (settings.UPLOAD_DIR, new_filename) + return f"{settings.UPLOAD_DIR}/{new_filename}" class MergedPoFile(PoFile): @@ -355,7 +357,7 @@ class MergedPoFile(PoFile): proxy = True @property - def prefix(self): + def prefix(self: "MergedPoFile") -> str: return settings.MEDIA_ROOT @@ -384,44 +386,46 @@ class ActionAbstract(models.Model): class Meta: abstract = True - def __str__(self): - return "%s (%s) - %s" % (self.name, self.description, self.id) + def __str__(self: "ActionAbstract") -> str: + return f"{self.name} ({self.description}) - {self.id}" @property - def description(self): + def description(self: "ActionAbstract") -> str: return dict(ACTION_NAMES).get(self.name, None) @property - def person_name(self): + def person_name(self: "ActionAbstract") -> str: return self.person.name if self.person else gettext("deleted account") @property - def most_uptodate_file(self): + def most_uptodate_file(self: "ActionAbstract") -> PoFile: return self.merged_file if self.merged_file else self.file @property - def most_uptodate_filepath(self): + def most_uptodate_filepath(self: "ActionAbstract") -> str: return self.merged_file.full_path if self.merged_file else self.file.path @property - def can_build(self): + def can_build(self: "ActionAbstract") -> bool: return not isinstance(self, ActionArchived) and self.state_db.domain.can_build_docs(self.state_db.branch) @property - def build_url(self): + def build_url(self: "ActionAbstract") -> str: path = settings.SCRATCHDIR / "HTML" / str(self.pk) / "index.html" return "/" + str(path.relative_to(settings.SCRATCHDIR)) if path.exists() else None - def get_filename(self): + def get_filename(self: "ActionAbstract") -> str | None: if self.file: - return os.path.basename(self.file.name) + return Path(self.file.name).name return None - def has_po_file(self): + def has_po_file(self: "ActionAbstract") -> bool: return self.file and self.file.name[-3:] == ".po" @classmethod - def get_action_history(cls, state=None, sequence=None): + def get_action_history( + cls: "ActionAbstract", state: State | None = None, sequence: int | None = None + ) -> list[tuple[Any, list[dict[str, Any]]]]: """ Return action history as a list of tuples (action, file_history), file_history is a list of previous po files, used in vertimus view to @@ -459,24 +463,23 @@ class Action(ActionAbstract): db_table = "action" verbose_name = "action" - def __init__(self, *args, **kwargs): + def __init__(self: "Action", *args, **kwargs) -> None: # noqa: ANN002, ANN003 super().__init__(*args, **kwargs) if self.name: self.__class__ = eval("Action" + self.name) - else: - if getattr(self.__class__, "name"): - self.name = self.__class__.name + elif getattr(self.__class__, "name"): + self.name = self.__class__.name @classmethod - def new_by_name(cls, action_name, **kwargs): + def new_by_name(cls: "Action", action_name: str, **kwargs) -> type["Action"]: # noqa: ANN003 return eval("Action" + action_name)(**kwargs) # FIXME: eval is unsafe - def save(self, *args, **kwargs): + def save(self: "Action", *args, **kwargs) -> None: # noqa: ANN002, ANN003 if not self.id and not self.created: self.created = timezone.now() super().save(*args, **kwargs) - def update_state(self): + def update_state(self: "Action") -> None: if self.target_state is not None: # All actions change state except Writing a comment self.state_db.change_state(self.target_state, self.person) @@ -489,14 +492,14 @@ class Action(ActionAbstract): arch_action = self.new_by_name("AA", person=self.person) arch_action.apply_on(self.state_db, {}) - def apply_on(self, state, form_data): + def apply_on(self: "Action", state: "State", form_data: dict[str, Any]) -> None: if self.name not in [a.name for a in state.get_available_actions(self.person)]: raise Exception("Action not allowed") if state.pk is None: state.save() self.state_db = state if self.file: - self.file.save(os.path.basename(self.file.name), self.file, save=False) + self.file.save(Path(self.file.name).name, self.file, save=False) if form_data.get("comment"): self.comment = form_data["comment"] self.sent_to_ml = form_data.get("send_to_ml", False) @@ -514,7 +517,7 @@ class Action(ActionAbstract): recipients = [pers.email for pers in state.language.team.get_coordinators() if pers.email] self.send_mail_new_state(state, recipients) - def get_previous_action_with_po(self): + def get_previous_action_with_po(self: "Action") -> Optional["Action"]: """ Return the previous Action with an uploaded file related to the same state. @@ -527,9 +530,9 @@ class Action(ActionAbstract): except Action.DoesNotExist: return None - def merge_file_with_pot(self, pot_file): + def merge_file_with_pot(self: "Action", pot_file: str) -> None: """Merge the uploaded translated file with current pot.""" - if not self.file or not os.path.exists(str(pot_file)): + if not self.file or not Path(pot_file).exists(): return if not self.merged_file: merged_path = "%s.merged.po" % self.file.path[:-3] @@ -544,12 +547,12 @@ class Action(ActionAbstract): temp_path = "%s.temp.po" % self.file.path[:-3] shutil.copy(merged_path, temp_path) po_grep(temp_path, merged_path, self.state_db.domain.red_filter) - os.remove(temp_path) + Path(temp_path).unlink() self.merged_file.update_statistics() - def send_mail_new_state(self, state, recipient_list): + def send_mail_new_state(self: "Action", state: "State", recipient_list: list[str]) -> None: """ - Prepare and send an email to recipient_list, informing about the action. + Prepare and email recipient_list, informing about the action. """ # Remove None and empty string items from the list recipient_list = filter(lambda x: x and x is not None, recipient_list) @@ -557,18 +560,16 @@ class Action(ActionAbstract): if not recipient_list: return - url = "https://%s%s" % ( - settings.SITE_DOMAIN, - reverse( - "vertimus_by_names", - args=(state.branch.module.name, state.branch.name, state.domain.name, state.language.locale), - ), - ) + url = f"""https://{settings.SITE_DOMAIN}" + f"{reverse( + 'vertimus_by_names', + args=(state.branch.module.name, state.branch.name, state.domain.name, state.language.locale) + )}""" subject = state.branch.module.name + " - " + state.branch.name # to_language may be unnecessary after https://code.djangoproject.com/ticket/32581 is solved with override(to_language(Language.django_locale(state.language.locale))): message = gettext("Hello,") + "\n\n" + gettext(self.default_message) + "\n%(url)s\n\n" - message = message % { + message %= { "module": state.branch.module.name, "branch": state.branch.name, "domain": state.domain.name, @@ -582,11 +583,7 @@ class Action(ActionAbstract): try: send_mail(subject, message, to=recipient_list, headers={settings.EMAIL_HEADER_NAME: state.description}) except Exception as exc: - raise SendMailFailed("Sending message failed: %r" % exc) from exc - - -def generate_archive_filename(instance, original_filename): - return "%s/%s" % (settings.UPLOAD_ARCHIVED_DIR, os.path.basename(original_filename)) + raise SendMailFailedError("Sending message failed: %r" % exc) from exc class ActionArchived(ActionAbstract): @@ -598,7 +595,7 @@ class ActionArchived(ActionAbstract): db_table = "action_archived" @classmethod - def clean_old_actions(cls, days): + def clean_old_actions(cls: "ActionArchived", days: int) -> None: """Delete old archived actions after some (now-days) time""" # In each sequence, test date of the latest action, to delete whole sequences instead of individual actions for action in ( @@ -667,9 +664,9 @@ class ActionTC(Action): class Meta: proxy = True - def apply_on(self, state, form_data): + def apply_on(self: "ActionTC", state: "State", form_data: dict[str, Any]) -> None: super().apply_on(state, form_data) - # Send an email to all committers of the team + # Email all committers of the team committers = [c.email for c in state.language.team.get_committers()] self.send_mail_new_state(state, committers) @@ -683,7 +680,7 @@ class ActionCI(Action): class Meta: proxy = True - def apply_on(self, state, form_data): + def apply_on(self: "ActionCI", state: "State", form_data: dict[str, Any]) -> str: self.state_db = state action_with_po = self.get_previous_action_with_po() author = form_data["author"] if form_data["author"] else None @@ -745,7 +742,7 @@ class ActionAA(Action): class Meta: proxy = True - def apply_on(self, state, form_data): + def apply_on(self: "ActionAA", state: "State", form_data: dict[str, Any]) -> None: super().apply_on(state, form_data) all_actions = Action.objects.filter(state_db=state).order_by("id").all() @@ -766,7 +763,7 @@ class ActionAA(Action): file=file_to_archive, ) if file_to_archive: - action_archived.file.save(os.path.basename(action.file.name), file_to_archive, save=False) + action_archived.file.save(Path(action.file.name).name, file_to_archive, save=False) if sequence is None: # The ID is available after the save() @@ -785,7 +782,7 @@ class ActionUNDO(Action): class Meta: proxy = True - def update_state(self): + def update_state(self: "ActionUNDO") -> None: # Look for the action to revert, excluding WC because this action is a noop on State actions = Action.objects.filter(state_db__id=self.state_db.id).exclude(name="WC").order_by("-id") skip_next = False @@ -812,12 +809,11 @@ class ActionSeparator: description = "--------" -# -# Signal actions -# @receiver(pot_has_changed) -def update_uploaded_files(sender, **kwargs): - """Callback to handle pot_file_changed signal""" +def update_uploaded_files(sender, **kwargs) -> None: # noqa: ANN003, ANN001, ARG001 + """ + Updates all the PO files that have ongoing actions using the latest POT file that is available. + """ actions = Action.objects.filter( state_db__branch=kwargs["branch"], state_db__domain=kwargs["domain"], file__endswith=".po" ) @@ -826,10 +822,9 @@ def update_uploaded_files(sender, **kwargs): @receiver(post_save) -def merge_uploaded_file(sender, instance, **kwargs): +def merge_uploaded_file(sender, instance, **kwargs) -> None: # noqa: ANN003, ANN001, ARG001 """ - post_save callback for Action that automatically merge uploaded file - with latest pot file. + Callback for Action that automatically merge uploaded file with the latest pot file. """ if not isinstance(instance, Action): return @@ -845,9 +840,9 @@ def merge_uploaded_file(sender, instance, **kwargs): @receiver(pre_delete) -def delete_action_files(sender, instance, **kwargs): +def delete_action_files(sender, instance, **kwargs) -> None: # noqa: ANN003, ANN001, ARG001 """ - pre_delete callback for Action that deletes: + Callback for Action that deletes: - the uploaded file - the merged file - the html dir where docs are built @@ -857,21 +852,21 @@ def delete_action_files(sender, instance, **kwargs): if instance.file.path.endswith(".po"): if instance.merged_file: if os.access(instance.merged_file.full_path, os.W_OK): - os.remove(instance.merged_file.full_path) + Path(instance.merged_file.full_path).unlink() if os.access(instance.file.path, os.W_OK): - os.remove(instance.file.path) + Path(instance.file.path).unlink() html_dir = settings.SCRATCHDIR / "HTML" / str(instance.pk) if html_dir.exists(): shutil.rmtree(str(html_dir)) @receiver(pre_delete, sender=Statistics) -def clean_dangling_states(sender, instance, **kwargs): +def clean_dangling_states(sender: object, instance: Statistics, **kwargs) -> None: # noqa: ANN003, ARG001 State.objects.filter(branch=instance.branch, domain=instance.domain, language=instance.language).delete() @receiver(post_save) -def reactivate_role(sender, instance, **kwargs): +def reactivate_role(instance, **kwargs) -> None: # noqa: ANN003, ARG001, ANN001 # Reactivating the role if needed if not isinstance(instance, Action): return diff --git a/vertimus/templatetags/vertimus.py b/vertimus/templatetags/vertimus.py index 6ea494202071f852e13b9abbde45c8501f869c6d..3d603cc30e6acaee798c6ecf1e726b172599d4fb 100644 --- a/vertimus/templatetags/vertimus.py +++ b/vertimus/templatetags/vertimus.py @@ -1,22 +1,17 @@ +from typing import TYPE_CHECKING + from django import template from django.templatetags.static import static from django.utils.html import format_html -from vertimus.models import ( - State, - StateCommitting, - StateProofread, - StateProofreading, - StateToReview, - StateTranslated, - StateTranslating, -) +if TYPE_CHECKING: + pass register = template.Library() @register.filter -def as_tr(field): +def as_tr(field) -> str: help_html = "" if field.help_text: help_html = format_html('
{}', field.help_text) @@ -49,7 +44,7 @@ def as_tr(field): @register.filter -def as_tr_cb(field): +def as_tr_cb(field) -> str: label = field.label if field.help_text: label = format_html('{} ({})', label, field.help_text) @@ -63,7 +58,16 @@ def as_tr_cb(field): @register.simple_tag -def get_badge_bg_from_vertimus_state(vertimus_state: State) -> str: +def get_badge_bg_from_vertimus_state(vertimus_state) -> str: + from vertimus.models import ( + StateCommitting, + StateProofread, + StateProofreading, + StateToReview, + StateTranslated, + StateTranslating, + ) + vertimus_state_badge_color = { StateTranslating.description: "text-bg-primary", StateProofreading.description: "text-bg-primary", diff --git a/vertimus/tests/tests.py b/vertimus/tests/tests.py index f15333e4dc3f15ab73e10f25c086412ada5567ff..d5e2b31e43284ecf2fde46f13019ddef1c3c2d3d 100644 --- a/vertimus/tests/tests.py +++ b/vertimus/tests/tests.py @@ -1,4 +1,3 @@ -import os import shutil import tempfile from functools import cache @@ -102,8 +101,8 @@ class VertimusTest(TeamsAndRolesMixin, TestCase): def tearDown(self): for path in self.files_to_clean: - if os.path.exists(path): - os.remove(path) + if Path(path).exists(): + Path(path).unlink() def upload_file(self, to_state, action_code="UP", pers=None): """Test utility to add an uploaded file to a state""" @@ -734,8 +733,8 @@ class VertimusTest(TeamsAndRolesMixin, TestCase): state = StateNone(branch=self.b, domain=self.d, language=self.language, person=self.reviewer) state.save() # Ensure pot file exists (used for merged file and by the diff view) - potfile, errs = self.d.generate_pot_file(self.b) - pot_location = self.b.output_directory(self.d.dtype) / ("%s.%s.pot" % (self.d.pot_base(), self.b.name)) + potfile, _ = self.d.generate_pot_file(self.b) + pot_location = self.b.output_directory(self.d.dtype) / (f"{self.d.pot_base()}.{self.b.name}.pot") shutil.copyfile(str(potfile), str(pot_location)) action = Action.new_by_name("RT", person=self.translator) @@ -784,7 +783,7 @@ class VertimusTest(TeamsAndRolesMixin, TestCase): self.assertEqual(doc.childNodes[0].getAttribute("xmlns:atom"), "http://www.w3.org/2005/Atom") self.assertEqual( doc.getElementsByTagName("title")[1].toxml(), - """po (gnome-hello/User Interface) - gnome-hello (gnome-2-24) - Reserve for translation\n""", + "po (gnome-hello/User Interface) - gnome-hello (gnome-2-24) - Reserve for translation\n", ) self.assertEqual( doc.getElementsByTagName("guid")[0].toxml(), @@ -805,15 +804,15 @@ class VertimusTest(TeamsAndRolesMixin, TestCase): with valid_path.open() as test_file: action = Action.new_by_name("UT", person=self.translator, file=File(test_file)) action.state_db = state - action.file.save(os.path.basename(action.file.name), action.file, save=False) + action.file.save(Path(action.file.name).name, action.file, save=False) action.merged_file = None action.save() response = self.client.get(reverse("action-quality-check", args=[action.pk])) self.assertContains(response, "The po file looks good!") po_stat = Statistics.objects.create(branch=self.b, domain=self.d, language=self.language) - os.mkdir(os.path.dirname(po_stat.po_path())) - with valid_path.open() as valid_file, open(po_stat.po_path(), "w") as fh: + Path(po_stat.po_path()).parent.mkdir(parents=True, exist_ok=True) + with valid_path.open() as valid_file, Path.open(po_stat.po_path(), "w", encoding="utf-8") as fh: fh.write(valid_file.read()) response = self.client.get(reverse("stats-quality-check", args=[po_stat.pk])) self.assertContains(response, "The po file looks good!") diff --git a/vertimus/views.py b/vertimus/views.py index b6ccfbfea94f81a17b02f2a68317c94e884f075b..4121daf438913e547874a3bc6449bbc962f8b1fb 100644 --- a/vertimus/views.py +++ b/vertimus/views.py @@ -5,6 +5,9 @@ import re import shutil import subprocess import tempfile +from collections.abc import Generator +from pathlib import Path +from typing import TYPE_CHECKING, Any from defusedxml.minidom import parse from django.conf import settings @@ -33,22 +36,27 @@ from stats.models import ( ) from stats.utils import ( DocFormat, - UndetectableDocFormat, + UndetectableDocFormatError, check_po_quality, is_po_reduced, ) from vertimus.forms import ActionForm -from vertimus.models import Action, ActionArchived, SendMailFailed, State +from vertimus.models import Action, ActionArchived, SendMailFailedError, State +if TYPE_CHECKING: + from django.http import HttpRequest, HttpResponse -def vertimus_by_stats_id(request, stats_id, lang_id): +from damnedlies import logger + + +def vertimus_by_stats_id(request: "HttpRequest", stats_id: int, lang_id: int) -> "HttpResponse": """Access to Vertimus view by a Statistics ID""" stats = get_object_or_404(Statistics, pk=stats_id) lang = get_object_or_404(Language, pk=lang_id) return vertimus(request, stats.branch, stats.domain, lang, stats) -def vertimus_by_ids(request, branch_id, domain_id, language_id): +def vertimus_by_ids(request: "HttpRequest", branch_id: int, domain_id: int, language_id: int) -> "HttpResponse": """Access to Vertimus view by Branch, Domain and language IDs""" branch = get_object_or_404(Branch, pk=branch_id) domain = get_object_or_404(Domain, pk=domain_id) @@ -56,7 +64,9 @@ def vertimus_by_ids(request, branch_id, domain_id, language_id): return vertimus(request, branch, domain, language) -def vertimus_by_names(request, module_name, branch_name, domain_name, locale_name, level="0"): +def vertimus_by_names( + request: "HttpRequest", module_name: str, branch_name: str, domain_name: str, locale_name: str, level: int = 0 +) -> "HttpResponse": """Access to Vertimus view by Branch, Domain and Language names""" module = get_object_or_404(Module, name=module_name) branch = get_object_or_404(Branch.objects.select_related("module"), name=branch_name, module__id=module.id) @@ -83,7 +93,14 @@ def _exists_a_domain_type_in_this_language_for_branch( return at_least_one_stat_exists_for_the_branch_domain_type -def vertimus(request, branch, domain, language, stats=None, level="0"): +def vertimus( + request: "HttpRequest", + branch: "Branch", + domain: "Domain", + language: "Language", + stats: Statistics | None = None, + level: int = 0, +) -> "HttpResponse": """The Vertimus view and form management. Level argument is used to access to the previous action history, first level (1) is the grandparent, second (2) is the parent of the grandparent, etc.""" @@ -114,7 +131,7 @@ def vertimus(request, branch, domain, language, stats=None, level="0"): action_form = None person = request.user.person if request.user.is_authenticated else None if person and level == 0: - # Only authenticated user can act on the translation and it's not + # Only authenticated user can act on the translation, and it's not # possible to edit an archived workflow available_actions = state.get_available_actions(person) if request.method == "POST": @@ -127,7 +144,7 @@ def vertimus(request, branch, domain, language, stats=None, level="0"): action = Action.new_by_name(action, person=person, file=request.FILES.get("file", None)) try: msg = action.apply_on(state, action_form.cleaned_data) - except SendMailFailed: + except SendMailFailedError: messages.error(request, _("A problem occurred while sending mail, no mail have been sent")) except Exception as e: messages.error(request, _("An error occurred during applying your action: %s") % e) @@ -150,7 +167,7 @@ def vertimus(request, branch, domain, language, stats=None, level="0"): "stats": stats, "pot_stats": pot_stats, "po_url": stats.po_url(), - "po_url_reduced": stats.has_reducedstat() and stats.po_url(reduced=True) or "", + "po_url_reduced": stats.po_url(reduced=True) if stats.has_reducedstat() else "", "branch": branch, "other_states": other_branch_states, "domain": domain, @@ -173,7 +190,9 @@ def vertimus(request, branch, domain, language, stats=None, level="0"): return render(request, "vertimus/vertimus_detail.html", context) -def get_vertimus_state(branch, domain, language, stats=None): +def get_vertimus_state( + branch: "Branch", domain: "Domain", language: "Language", stats: Statistics | None = None +) -> tuple[Statistics, Statistics, State]: pot_stats = get_object_or_404(Statistics, branch=branch, domain=domain, language=None) if not stats: try: @@ -190,7 +209,7 @@ def get_vertimus_state(branch, domain, language, stats=None): return pot_stats, stats, state -def vertimus_diff(request, action_id_1, action_id_2, level): +def vertimus_diff(request: "HttpRequest", action_id_1: int, action_id_2: int, level: int = 0) -> "HttpResponse": """Show a diff between current action po file and previous file""" if int(level) != 0: action_real = ActionArchived @@ -201,7 +220,7 @@ def vertimus_diff(request, action_id_1, action_id_2, level): file_path_1 = action_1.most_uptodate_filepath reduced = is_po_reduced(file_path_1) - if not os.path.exists(file_path_1): + if not Path(file_path_1).exists(): raise Http404("File not found") descr_1 = _('Uploaded file by %(name)s on %(date)s') % { @@ -210,7 +229,7 @@ def vertimus_diff(request, action_id_1, action_id_2, level): "date": action_1.created, } - if action_id_2 not in (None, 0): + if action_id_2 not in {None, 0}: # 1) id_2 specified in URL action_2 = get_object_or_404(action_real, pk=action_id_2) file_path_2 = action_2.most_uptodate_filepath @@ -242,16 +261,15 @@ def vertimus_diff(request, action_id_1, action_id_2, level): } except Statistics.DoesNotExist: stats = get_object_or_404(Statistics, branch=state.branch, domain=state.domain, language=None) - descr_2 = '%(text)s' % { - "url": stats.pot_url(), + descr_2 = f'%(text)s' % { "text": _("Latest POT file"), } file_path_2 = stats.po_path(reduced=reduced) - if not os.path.exists(file_path_2): + if not Path(file_path_2).exists(): raise Http404("File not found") d = difflib.HtmlDiff(wrapcolumn=80) - with open(file_path_1, encoding="utf-8", errors="replace") as fh1, open( + with Path.open(file_path_1, encoding="utf-8", errors="replace") as fh1, Path.open( file_path_2, encoding="utf-8", errors="replace" ) as fh2: diff_content = d.make_table(fh2.readlines(), fh1.readlines(), descr_2, descr_1, context=True) @@ -263,7 +281,13 @@ def vertimus_diff(request, action_id_1, action_id_2, level): return render(request, "vertimus/vertimus_diff.html", context) -def latest_uploaded_po(request, module_name, branch_name, domain_name, locale_name): +def latest_uploaded_po( + request: "HttpRequest", # noqa: ARG001 + module_name: str, + branch_name: str, + domain_name: str, + locale_name: str, +) -> "HttpResponse": """Redirect to the latest uploaded po for a module/branch/language""" branch = get_object_or_404(Branch, module__name=module_name, name=branch_name) domain = get_object_or_404(Domain, module__name=module_name, name=domain_name) @@ -276,7 +300,7 @@ def latest_uploaded_po(request, module_name, branch_name, domain_name, locale_na return HttpResponseRedirect(latest_upload[0].merged_file.url) -def activity_by_language(request, locale): +def activity_by_language(request: "HttpRequest", locale: str) -> "HttpResponse": language = get_object_or_404(Language, locale=locale) states = State.objects.filter(language=language).exclude(name="None") context = { @@ -288,12 +312,12 @@ def activity_by_language(request, locale): class PoFileActionBase(View): - def get(self, request, *args, **kwargs): + def get(self: "PoFileActionBase", request: "HttpRequest", *args, **kwargs) -> "HttpResponse": # noqa: ANN002, ARG002, ANN003 self.pofile = self.get_po_file() context = self.get_context_data(**kwargs) return render(request, self.template_name, context) - def get_po_file(self): + def get_po_file(self: "PoFileActionBase") -> Path: pofile = None if self.kwargs.get("action_pk"): self.action = get_object_or_404(Action, pk=self.kwargs["action_pk"]) @@ -310,7 +334,7 @@ class PoFileActionBase(View): class QualityCheckView(PoFileActionBase): template_name = "vertimus/quality-check.html" - def get_context_data(self, **kwargs): + def get_context_data(self: "QualityCheckView", **kwargs) -> dict[str, Any]: # noqa: ARG002, ANN003 context = {"base": "base_modal.html"} if self.pofile is None: context["results"] = _("No po file to check") @@ -361,15 +385,15 @@ class MsgiddiffView(PoFileActionBase): ) FOOTER = "" - def streamed_file(self, po_file): - def strip(line): + def streamed_file(self: "MsgiddiffView", po_file: Path) -> Generator[str, None, None]: + def strip(line: str) -> str: if line.startswith("#| "): line = line[3:] if line.startswith("msgid "): line = line[6:] return line.rstrip("\n").strip('"') - def line_fmt(no, line): + def line_fmt(no: int, line: str) -> str: return '%d ' % no + html.escape(line) + "
" yield self.HEADER @@ -377,7 +401,7 @@ class MsgiddiffView(PoFileActionBase): no_wrap = False stored_comments = [] in_fuzzy = False - with open(po_file, encoding="utf-8") as fh: + with Path.open(po_file, encoding="utf-8") as fh: for noline, line in enumerate(fh.readlines(), start=1): if line.startswith(("#.", "#:")): stored_comments.append((noline, line)) @@ -425,16 +449,26 @@ class MsgiddiffView(PoFileActionBase): yield self.FOOTER - def get(self, request, *args, **kwargs): + def get( + self: "MsgiddiffView", + request: "HttpRequest", # noqa: ARG002 + *args, # noqa: ARG002, ANN002, + **kwargs, # noqa: ARG002, ANN003 + ) -> StreamingHttpResponse: stats = get_object_or_404(Statistics, pk=self.kwargs["stats_pk"]) pofile = stats.po_path() return StreamingHttpResponse(self.streamed_file(pofile)) class BuildTranslatedDocsView(PoFileActionBase): - http_method_names = ["post"] - - def post(self, request, *args, **kwargs): + http_method_names = ("post",) + + def post( + self: "BuildTranslatedDocsView", + request: "HttpRequest", + *args, # noqa: ARG002, ANN002 + **kwargs, # noqa: ARG002, ANN003 + ) -> "HttpResponse": pofile = self.get_po_file() if pofile is None: raise Http404("No target po file for this action") @@ -458,45 +492,50 @@ class BuildTranslatedDocsView(PoFileActionBase): return HttpResponseRedirect(state.get_absolute_url()) return HttpResponseRedirect(self.action.build_url) - def build_docs(self, state, po_file, html_dir): + def build_docs(self: "BuildTranslatedDocsView", state: "State", po_file: Path, html_dir: Path) -> str: """ Try building translated docs, return an error message or an empty string on success. """ try: doc_format = DocFormat(state.domain, state.branch) - except UndetectableDocFormat as err: + except UndetectableDocFormatError as err: return str(err) build_error = _("Build failed (%(program)s): %(err)s") with tempfile.NamedTemporaryFile(suffix=".gmo") as gmo, tempfile.TemporaryDirectory() as build_dir: - result = subprocess.run(["msgfmt", po_file, "-o", os.path.join(gmo.name)], stderr=subprocess.PIPE) - if result.returncode != 0: + try: + subprocess.run(["msgfmt", po_file, "-o", os.path.join(gmo.name)], capture_output=True, check=True) + except subprocess.CalledProcessError as called_process_error: + logger.error("Building docs: %s", called_process_error.stderr.decode(errors="replace")) return build_error % { "program": "msgfmt", - "err": result.stderr.decode(errors="replace"), + "err": called_process_error.stderr.decode(errors="replace"), } sources = doc_format.source_files() - result = subprocess.run( - [ - "itstool", - "-m", - gmo.name, - "-l", - state.language.locale, - "-o", - str(build_dir), - "--strict", - *[str(s) for s in sources], - ], - cwd=str(doc_format.vcs_path), - stderr=subprocess.PIPE, - ) - if result.returncode != 0: + try: + subprocess.run( + [ + "itstool", + "-m", + gmo.name, + "-l", + state.language.locale, + "-o", + str(build_dir), + "--strict", + *[str(s) for s in sources], + ], + cwd=str(doc_format.vcs_path), + capture_output=True, + check=True, + ) + except subprocess.CalledProcessError as called_process_error: + logger.error("Building docs: %s", called_process_error.stderr.decode(errors="replace")) return build_error % { "program": "itstool", - "err": result.stderr.decode(errors="replace"), + "err": called_process_error.stderr.decode(errors="replace"), } # Now build the html version @@ -507,17 +546,23 @@ class BuildTranslatedDocsView(PoFileActionBase): build_ref = [str(build_dir)] else: build_ref = [os.path.join(build_dir, s.name) for s in sources] - cmd = ["yelp-build", "html", "-o", str(html_dir) + "/", "-p", str(doc_format.vcs_path / "C"), *build_ref] - result = subprocess.run(cmd, cwd=str(build_dir), stderr=subprocess.PIPE) + index_html = html_dir / "index.html" - if result.returncode != 0 or (not index_html.exists() and len(result.stderr) > 0): - shutil.rmtree(str(html_dir)) + cmd = ["yelp-build", "html", "-o", str(html_dir) + "/", "-p", str(doc_format.vcs_path / "C"), *build_ref] + try: + result = subprocess.run(cmd, cwd=str(build_dir), capture_output=True, check=True) + except subprocess.CalledProcessError as called_process_error: + html_directory_should_be_removed_because_of_failure = ( + not index_html.exists() and len(result.stderr) > 0 + ) + if html_directory_should_be_removed_because_of_failure: + shutil.rmtree(html_dir) return build_error % { "program": "yelp-build", - "err": result.stderr.decode(errors="replace"), + "err": called_process_error.stderr.decode(errors="replace"), } - if not index_html.exists() and os.path.isfile(build_ref[0]): + if not index_html.exists() and Path(build_ref[0]).is_file(): # Create an index.html symlink to the base html doc if needed try: # FIXME: forbid_entities=False is a trick that should be removed if possible @@ -531,7 +576,7 @@ class BuildTranslatedDocsView(PoFileActionBase): return "" -def diff_strings(previous, current): +def diff_strings(previous: str, current: str) -> str: """ Compute a diff between two strings, with inline differences. http://stackoverflow.com/questions/774316/python-difflib-highlighting-differences-inline