[damned-lies] refactor: fix few linter issues in vertimus

[Claude Paroz <claudep src gnome org>]

commit a0ad8ca2a31633ff83318fce05986d8abe3cc79b
Author: Guillaume Bernard <associations guillaume-bernard fr>
Date:   Sun Apr 18 18:37:33 2021 +0200

    refactor: fix few linter issues in vertimus

 vertimus/admin.py                 |  3 ++
 vertimus/forms.py                 |  8 ++++--
 vertimus/models.py                | 59 +++++++++++++++++++++------------------
 vertimus/templatetags/vertimus.py |  1 -
 vertimus/views.py                 | 13 ++++-----
 5 files changed, 46 insertions(+), 38 deletions(-)
---
diff --git a/vertimus/admin.py b/vertimus/admin.py
index 91eb590c..4fec3f3b 100644
--- a/vertimus/admin.py
+++ b/vertimus/admin.py
@@ -2,15 +2,18 @@ from django.contrib import admin
 
 from vertimus.models import State, Action
 
+
 class StateAdmin(admin.ModelAdmin):
     list_filter = ('name', 'language')
     raw_id_fields = ('branch', 'domain', 'person',)
     search_fields = ('branch__module__name',)
 
+
 class ActionAdmin(admin.ModelAdmin):
     list_display = ('__str__', 'state_db', 'merged_file')
     raw_id_fields = ('state_db', 'person', 'merged_file')
     search_fields = ('comment',)
 
+
 admin.site.register(State, StateAdmin)
 admin.site.register(Action, ActionAdmin)
diff --git a/vertimus/forms.py b/vertimus/forms.py
index 8396b982..9f8f5d31 100644
--- a/vertimus/forms.py
+++ b/vertimus/forms.py
@@ -32,7 +32,7 @@ class AuthorChoiceField(forms.ModelChoiceField):
     def label_from_instance(self, obj):
         if str(obj) == obj.username:
             return DisabledLabel(gettext("%(name)s (full name missing)") % {'name': obj.username})
-        elif not obj.email:
+        if not obj.email:
             return DisabledLabel(gettext("%(name)s (email missing)") % {'name': str(obj)})
         return str(obj)
 
@@ -86,7 +86,9 @@ class ActionForm(forms.Form):
             # If this is a .po file, check validity (msgfmt)
             if ext == '.po':
                 if check_po_conformity(data):
-                    raise ValidationError(_(".po file does not pass “msgfmt -vc”. Please correct the file 
and try again."))
+                    raise ValidationError(
+                        _(".po file does not pass “msgfmt -vc”. Please correct the file and try again.")
+                    )
         return data
 
     def clean(self):
@@ -94,7 +96,7 @@ class ActionForm(forms.Form):
         action_code = cleaned_data.get('action')
         if action_code is None:
             raise ValidationError(_("Invalid action. Someone probably posted another action just before 
you."))
-        elif action_code == 'CI':
+        if action_code == 'CI':
             if not cleaned_data['author']:
                 raise ValidationError(_("Committing a file requires a commit author."))
             if 'Token' in getattr(self.current_user, 'backend', ''):
diff --git a/vertimus/models.py b/vertimus/models.py
index d1c983f3..4b2c0c62 100644
--- a/vertimus/models.py
+++ b/vertimus/models.py
@@ -1,7 +1,7 @@
-import os, sys
+import os
+import sys
 import shutil
 from datetime import datetime, timedelta
-from pathlib import Path
 
 from django.conf import settings
 from django.db import models
@@ -25,12 +25,11 @@ from teams.models import Role
 
 class SendMailFailed(Exception):
     """Something went wrong while sending message"""
-    pass
+
 
 #
 # States
 #
-
 class State(models.Model):
     """State of a module translation"""
     branch = models.ForeignKey(Branch, on_delete=models.CASCADE)
@@ -94,8 +93,7 @@ class State(models.Model):
         if self.branch.module.archived:
             if self.action_set.count() > 0 and person.is_coordinator(self.language.team):
                 return [ActionAA()]
-            else:
-                return []
+            return []
 
         action_names.append('WC')
         # Allow the coordinator to cancel current reserved state
@@ -104,7 +102,7 @@ class State(models.Model):
             action_names.append('UNDO')
         if person.is_committer(self.language.team) and 'IC' not in action_names:
             action_names.extend(('Separator', 'IC', 'AA'))
-        return [eval('Action' + action_name)() for action_name in action_names]
+        return [eval('Action' + action_name)() for action_name in action_names]  # FIXME: eval is unsafe
 
     def get_action_sequence_from_level(self, level):
         """Get the sequence corresponding to the requested level.
@@ -399,14 +397,10 @@ class ActionAbstract(models.Model):
     def get_filename(self):
         if self.file:
             return os.path.basename(self.file.name)
-        else:
-            return None
+        return None
 
     def has_po_file(self):
-        try:
-            return self.file.name[-3:] == ".po"
-        except:
-            return False
+        return self.file and self.file.name[-3:] == ".po"
 
     @classmethod
     def get_action_history(cls, state=None, sequence=None):
@@ -455,7 +449,7 @@ class Action(ActionAbstract):
 
     @classmethod
     def new_by_name(cls, action_name, **kwargs):
-         return eval('Action' + action_name)(**kwargs)
+        return eval('Action' + action_name)(**kwargs)  # FIXME: eval is unsafe
 
     def save(self, *args, **kwargs):
         if not self.id and not self.created:
@@ -476,7 +470,7 @@ class Action(ActionAbstract):
             arch_action.apply_on(self.state_db, {})
 
     def apply_on(self, state, form_data):
-        if not self.name in [a.name for a in state.get_available_actions(self.person)]:
+        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()
@@ -545,13 +539,17 @@ 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 = "https://%s%s"; % (
+            settings.SITE_DOMAIN, 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))):
@@ -579,6 +577,7 @@ class Action(ActionAbstract):
 def generate_archive_filename(instance, original_filename):
     return "%s/%s" % (settings.UPLOAD_ARCHIVED_DIR, os.path.basename(original_filename))
 
+
 class ActionArchived(ActionAbstract):
     # The first element of each cycle is null at creation time (and defined
     # afterward).
@@ -739,7 +738,7 @@ class ActionAA(Action):
             file_to_archive = None
             if action.file:
                 try:
-                    file_to_archive = action.file.file # get a file object, not a filefield
+                    file_to_archive = action.file.file  # get a file object, not a filefield
                 except IOError:
                     pass
             action_archived = ActionArchived(
@@ -760,7 +759,7 @@ class ActionAA(Action):
             action_archived.sequence = sequence
             action_archived.save()
 
-            action.delete() # The file is also automatically deleted, if it is not referenced elsewhere
+            action.delete()  # The file is also automatically deleted, if it is not referenced elsewhere
 
 
 class ActionUNDO(Action):
@@ -794,6 +793,7 @@ class ActionSeparator:
     name = None
     description = "--------"
 
+
 #
 # Signal actions
 #
@@ -806,6 +806,7 @@ def update_uploaded_files(sender, **kwargs):
     for action in actions:
         action.merge_file_with_pot(kwargs['potfile'])
 
+
 @receiver(post_save)
 def merge_uploaded_file(sender, instance, **kwargs):
     """
@@ -816,12 +817,15 @@ def merge_uploaded_file(sender, instance, **kwargs):
         return
     if instance.file and instance.file.path.endswith('.po'):
         try:
-            stat = Statistics.objects.get(branch=instance.state_db.branch, domain=instance.state_db.domain, 
language=None)
+            stat = Statistics.objects.get(
+                branch=instance.state_db.branch, domain=instance.state_db.domain, language=None
+            )
         except Statistics.DoesNotExist:
             return
         potfile = stat.po_path()
         instance.merge_file_with_pot(potfile)
 
+
 @receiver(pre_delete)
 def delete_action_files(sender, instance, **kwargs):
     """
@@ -835,9 +839,9 @@ 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)
+                os.remove(instance.merged_file.full_path)
     if os.access(instance.file.path, os.W_OK):
-         os.remove(instance.file.path)
+        os.remove(instance.file.path)
     html_dir = settings.SCRATCHDIR / 'HTML' / str(instance.pk)
     if html_dir.exists():
         shutil.rmtree(str(html_dir))
@@ -847,6 +851,7 @@ def delete_action_files(sender, instance, **kwargs):
 def clean_dangling_states(sender, instance, **kwargs):
     State.objects.filter(branch=instance.branch, domain=instance.domain, language=instance.language).delete()
 
+
 @receiver(post_save)
 def reactivate_role(sender, instance, **kwargs):
     # Reactivating the role if needed
diff --git a/vertimus/templatetags/vertimus.py b/vertimus/templatetags/vertimus.py
index f34b4b6e..f08619ec 100644
--- a/vertimus/templatetags/vertimus.py
+++ b/vertimus/templatetags/vertimus.py
@@ -1,5 +1,4 @@
 from django import template
-from django.conf import settings
 from django.templatetags.static import static
 from django.utils.html import format_html
 
diff --git a/vertimus/views.py b/vertimus/views.py
index d9025c25..c13e221a 100644
--- a/vertimus/views.py
+++ b/vertimus/views.py
@@ -5,7 +5,6 @@ import re
 import shutil
 import subprocess
 import tempfile
-from pathlib import Path
 from xml.dom.minidom import parse
 
 from django.conf import settings
@@ -89,7 +88,7 @@ def vertimus(request, branch, domain, language, stats=None, level="0"):
         person = request.user.person
 
         available_actions = state.get_available_actions(person)
-        has_ml = language.team and bool(language.team.mailing_list) or False
+        has_ml = bool(language.team.mailing_list) if language.team else False
         if request.method == 'POST':
             action_form = ActionForm(
                 request.user, state, available_actions, has_ml, request.POST, request.FILES
@@ -367,19 +366,19 @@ class MsgiddiffView(PoFileActionBase):
                     if line.startswith('#|'):
                         prev_id.append(line)
                         continue
-                    elif line.startswith('msgstr "'):
+                    if line.startswith('msgstr "'):
                         # Compute and display inline diff
                         sep = '\n' if no_wrap else ''
                         yield (
                             '<div class="diff%s">' % (' nowrap' if no_wrap else '') + diff_strings(
-                                html.escape(sep.join(strip(l) for l in prev_id)),
-                                html.escape(sep.join(strip(l) for _, l in curr_id))
+                                html.escape(sep.join(strip(_line) for _line in prev_id)),
+                                html.escape(sep.join(strip(_linel) for _, _line in curr_id))
                             ) + '</div>'
                         )
                         for no, _line in curr_id:
                             yield line_fmt(no, _line)
                         diff_printed = True
-                    elif not diff_printed:
+                    if not diff_printed:
                         curr_id.append((noline, line))
                         continue
                     yield line_fmt(noline, line)
@@ -464,7 +463,7 @@ class BuildTranslatedDocsView(PoFileActionBase):
             ]
             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)):
+            if result.returncode != 0 or (not index_html.exists() and len(result.stderr) > 0):
                 shutil.rmtree(str(html_dir))
                 return build_error % {
                     'program': 'yelp-build',