Pylint compliance on cotisations

cotisations/__init__.py

@@ -20,5 +20,8 @@
 # You should have received a copy of the GNU General Public License along
 # with this program; if not, write to the Free Software Foundation, Inc.,
 # 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+"""cotisations
+The app in charge of all the members's cotisations
+"""
 
 from .acl import *

cotisations/admin.py

@@ -20,6 +20,9 @@
 # You should have received a copy of the GNU General Public License along
 # with this program; if not, write to the Free Software Foundation, Inc.,
 # 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+"""cotisations.admin
+The objects, fields and datastructures visible in the Django admin view
+"""
 
 from __future__ import unicode_literals
 

cotisations/forms.py

@@ -38,16 +38,14 @@ from __future__ import unicode_literals
 from django import forms
 from django.db.models import Q
 from django.forms import ModelForm, Form
-from django.core.validators import MinValueValidator, MaxValueValidator
+from django.core.validators import MinValueValidator
 from django.utils.translation import ugettext as _
 from django.utils.translation import ugettext_lazy as _l
 
-from .models import Article, Paiement, Facture, Banque
 from preferences.models import OptionalUser
-from users.models import User
-
 from re2o.field_permissions import FieldPermissionFormMixin
 from re2o.mixins import FormRevMixin
+from .models import Article, Paiement, Facture, Banque
 
 
 class NewFactureForm(FormRevMixin, ModelForm):
@@ -313,6 +311,11 @@ class NewFactureSoldeForm(NewFactureForm):
     """
     def __init__(self, *args, **kwargs):
         prefix = kwargs.pop('prefix', self.Meta.model.__name__)
+        super(NewFactureSoldeForm, self).__init__(
+            *args,
+            prefix=prefix,
+            **kwargs
+        )
         self.fields['cheque'].required = False
         self.fields['banque'].required = False
         self.fields['cheque'].label = _('Cheque number')
@@ -367,6 +370,10 @@ class RechargeForm(FormRevMixin, Form):
         super(RechargeForm, self).__init__(*args, **kwargs)
 
     def clean_value(self):
+        """
+        Returns a cleaned vlaue from the received form by validating
+        the value is well inside the possible limits
+        """
         value = self.cleaned_data['value']
         if value < OptionalUser.get_cached_value('min_online_payment'):
             raise forms.ValidationError(

cotisations/models.py

@@ -34,17 +34,16 @@ from __future__ import unicode_literals
 from dateutil.relativedelta import relativedelta
 
 from django.db import models
-from django.db.models import Q
+from django.db.models import Q, Max
 from django.db.models.signals import post_save, post_delete
 from django.dispatch import receiver
 from django.forms import ValidationError
 from django.core.validators import MinValueValidator
-from django.db.models import Max
 from django.utils import timezone
-from machines.models import regen
 from django.utils.translation import ugettext as _
 from django.utils.translation import ugettext_lazy as _l
 
+from machines.models import regen
 from re2o.field_permissions import FieldPermissionModelMixin
 from re2o.mixins import AclMixin, RevMixin
 
@@ -106,15 +105,15 @@ class Facture(RevMixin, AclMixin, FieldPermissionModelMixin, models.Model):
         permissions = (
             # TODO : change facture to invoice
             ('change_facture_control',
-                _l("Can change the \"controlled\" state")),
+             _l("Can change the \"controlled\" state")),
             # TODO : seems more likely to be call create_facture_pdf
             # or create_invoice_pdf
             ('change_facture_pdf',
-                _l("Can create a custom PDF invoice")),
+             _l("Can create a custom PDF invoice")),
             ('view_facture',
-                _l("Can see an invoice's details")),
+             _l("Can see an invoice's details")),
             ('change_all_facture',
-                _l("Can edit all the previous invoices")),
+             _l("Can edit all the previous invoices")),
         )
         verbose_name = _l("Invoice")
         verbose_name_plural = _l("Invoices")
@@ -187,7 +186,7 @@ class Facture(RevMixin, AclMixin, FieldPermissionModelMixin, models.Model):
         else:
             return True, None
 
-    def can_view(self, user_request, *args, **kwargs):
+    def can_view(self, user_request, *_args, **_kwargs):
         if not user_request.has_perm('cotisations.view_facture') and \
                 self.user != user_request:
             return False, _("You don't have the right to see someone else's "
@@ -198,14 +197,17 @@ class Facture(RevMixin, AclMixin, FieldPermissionModelMixin, models.Model):
             return True, None
 
     @staticmethod
-    def can_change_control(user_request, *args, **kwargs):
+    def can_change_control(user_request, *_args, **_kwargs):
+        """ Returns True if the user can change the 'controlled' status of
+        this invoice """
         return (
             user_request.has_perm('cotisations.change_facture_control'),
             _("You don't have the right to edit the \"controlled\" state.")
         )
 
     @staticmethod
-    def can_change_pdf(user_request, *args, **kwargs):
+    def can_change_pdf(user_request, *_args, **_kwargs):
+        """ Returns True if the user can change this invoice """
         return (
             user_request.has_perm('cotisations.change_facture_pdf'),
             _("You don't have the right to edit an invoice.")
@@ -222,7 +224,7 @@ class Facture(RevMixin, AclMixin, FieldPermissionModelMixin, models.Model):
 
 
 @receiver(post_save, sender=Facture)
-def facture_post_save(sender, **kwargs):
+def facture_post_save(_sender, **kwargs):
     """
     Synchronise the LDAP user after an invoice has been saved.
     """
@@ -232,7 +234,7 @@ def facture_post_save(sender, **kwargs):
 
 
 @receiver(post_delete, sender=Facture)
-def facture_post_delete(sender, **kwargs):
+def facture_post_delete(_sender, **kwargs):
     """
     Synchronise the LDAP user after an invoice has been deleted.
     """
@@ -375,13 +377,14 @@ class Vente(RevMixin, AclMixin, models.Model):
     def can_edit(self, user_request, *args, **kwargs):
         if not user_request.has_perm('cotisations.change_vente'):
             return False, _("You don't have the right to edit the purchases.")
-        elif not user_request.has_perm('cotisations.change_all_facture') and \
+        elif (not user_request.has_perm('cotisations.change_all_facture') and
-                not self.facture.user.can_edit(
+              not self.facture.user.can_edit(
-                    user_request, *args, **kwargs)[0]:
+                  user_request, *args, **kwargs
+              )[0]):
             return False, _("You don't have the right to edit this user's "
                             "purchases.")
-        elif not user_request.has_perm('cotisations.change_all_vente') and \
+        elif (not user_request.has_perm('cotisations.change_all_vente') and
-                (self.facture.control or not self.facture.valid):
+              (self.facture.control or not self.facture.valid)):
             return False, _("You don't have the right to edit a purchase "
                             "already controlled or invalidated.")
         else:
@@ -399,9 +402,9 @@ class Vente(RevMixin, AclMixin, models.Model):
         else:
             return True, None
 
-    def can_view(self, user_request, *args, **kwargs):
+    def can_view(self, user_request, *_args, **_kwargs):
-        if not user_request.has_perm('cotisations.view_vente') and \
+        if (not user_request.has_perm('cotisations.view_vente') and
-                self.facture.user != user_request:
+                self.facture.user != user_request):
             return False, _("You don't have the right to see someone "
                             "else's purchase history.")
         else:
@@ -413,7 +416,7 @@ class Vente(RevMixin, AclMixin, models.Model):
 
 # TODO : change vente to purchase
 @receiver(post_save, sender=Vente)
-def vente_post_save(sender, **kwargs):
+def vente_post_save(_sender, **kwargs):
     """
     Creates a 'cotisation' related object if needed and synchronise the
     LDAP user when a purchase has been saved.
@@ -431,7 +434,7 @@ def vente_post_save(sender, **kwargs):
 
 # TODO : change vente to purchase
 @receiver(post_delete, sender=Vente)
-def vente_post_delete(sender, **kwargs):
+def vente_post_delete(_sender, **kwargs):
     """
     Synchronise the LDAP user after a purchase has been deleted.
     """
@@ -646,7 +649,7 @@ class Cotisation(RevMixin, AclMixin, models.Model):
             ('change_all_cotisation', _l("Can edit the previous cotisations")),
         )
 
-    def can_edit(self, user_request, *args, **kwargs):
+    def can_edit(self, user_request, *_args, **_kwargs):
         if not user_request.has_perm('cotisations.change_cotisation'):
             return False, _("You don't have the right to edit a cotisation.")
         elif not user_request.has_perm('cotisations.change_all_cotisation') \
@@ -657,7 +660,7 @@ class Cotisation(RevMixin, AclMixin, models.Model):
         else:
             return True, None
 
-    def can_delete(self, user_request, *args, **kwargs):
+    def can_delete(self, user_request, *_args, **_kwargs):
         if not user_request.has_perm('cotisations.delete_cotisation'):
             return False, _("You don't have the right to delete a "
                             "cotisation.")
@@ -667,7 +670,7 @@ class Cotisation(RevMixin, AclMixin, models.Model):
         else:
             return True, None
 
-    def can_view(self, user_request, *args, **kwargs):
+    def can_view(self, user_request, *_args, **_kwargs):
         if not user_request.has_perm('cotisations.view_cotisation') and\
                 self.vente.facture.user != user_request:
             return False, _("You don't have the right to see someone else's "
@@ -680,7 +683,7 @@ class Cotisation(RevMixin, AclMixin, models.Model):
 
 
 @receiver(post_save, sender=Cotisation)
-def cotisation_post_save(sender, **kwargs):
+def cotisation_post_save(_sender, **_kwargs):
     """
     Mark some services as needing a regeneration after the edition of a
     cotisation. Indeed the membership status may have changed.
@@ -692,11 +695,10 @@ def cotisation_post_save(sender, **kwargs):
 
 
 @receiver(post_delete, sender=Cotisation)
-def cotisation_post_delete(sender, **kwargs):
+def cotisation_post_delete(_sender, **_kwargs):
     """
     Mark some services as needing a regeneration after the deletion of a
     cotisation. Indeed the membership status may have changed.
     """
-    cotisation = kwargs['instance']
     regen('mac_ip_list')
     regen('mailing')

cotisations/payment.py

@@ -2,6 +2,9 @@
 
 Here are defined some views dedicated to online payement.
 """
+
+from collections import OrderedDict
+
 from django.urls import reverse
 from django.shortcuts import redirect, get_object_or_404
 from django.contrib.auth.decorators import login_required
@@ -11,8 +14,6 @@ from django.utils.datastructures import MultiValueDictKeyError
 from django.utils.translation import ugettext as _
 from django.http import HttpResponse, HttpResponseBadRequest
 
-from collections import OrderedDict
-
 from preferences.models import AssoOption
 from .models import Facture
 from .payment_utils.comnpay import Payment as ComnpayPayment

cotisations/payment_utils/comnpay.py

@@ -1,20 +1,19 @@
+"""cotisations.payment_utils.comnpay
+The module in charge of handling the negociation with Comnpay
+for online payment
+"""
+
 import time
 from random import randrange
 import base64
 import hashlib
 from collections import OrderedDict
-from itertools import chain
 
 
 class Payment():
-
+    """ The class representing a transaction with all the functions
-    vad_number = ""
+    used during the negociation
-    secret_key = ""
+    """
-    urlRetourOK = ""
-    urlRetourNOK = ""
-    urlIPN = ""
-    source = ""
-    typeTr = "D"
 
     def __init__(self, vad_number="", secret_key="", urlRetourOK="",
                  urlRetourNOK="", urlIPN="", source="", typeTr="D"):
@@ -25,9 +24,13 @@ class Payment():
         self.urlIPN = urlIPN
         self.source = source
         self.typeTr = typeTr
+        self.idTransaction = ""
 
     def buildSecretHTML(self, produit="Produit", montant="0.00",
                         idTransaction=""):
+        """ Build an HTML hidden form with the different parameters for the
+        transaction
+        """
         if idTransaction == "":
             self.idTransaction = str(time.time())
             self.idTransaction += self.vad_number
@@ -36,16 +39,16 @@ class Payment():
             self.idTransaction = idTransaction
 
         array_tpe = OrderedDict(
-                montant=str(montant),
+            montant=str(montant),
-                idTPE=self.vad_number,
+            idTPE=self.vad_number,
-                idTransaction=self.idTransaction,
+            idTransaction=self.idTransaction,
-                devise="EUR",
+            devise="EUR",
-                lang='fr',
+            lang='fr',
-                nom_produit=produit,
+            nom_produit=produit,
-                source=self.source,
+            source=self.source,
-                urlRetourOK=self.urlRetourOK,
+            urlRetourOK=self.urlRetourOK,
-                urlRetourNOK=self.urlRetourNOK,
+            urlRetourNOK=self.urlRetourNOK,
-                typeTr=str(self.typeTr)
+            typeTr=str(self.typeTr)
         )
 
         if self.urlIPN != "":
@@ -63,12 +66,14 @@ class Payment():
         for key in array_tpe:
             ret += '<input type="hidden" name="{k}" value="{v}"/>'.format(
                 k=key,
-                v=array_type[key]
+                v=array_tpe[key]
             )
 
         return ret
 
-    def validSec(self, values, secret_key):
+    @staticmethod
+    def validSec(values, secret_key):
+        """ Check if the secret value is correct """
         if "sec" in values:
             sec = values['sec']
             del values["sec"]

cotisations/tests.py

@@ -19,7 +19,10 @@
 # You should have received a copy of the GNU General Public License along
 # with this program; if not, write to the Free Software Foundation, Inc.,
 # 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+"""cotisations.tests
+The tests for the Cotisations module.
+"""
 
-from django.test import TestCase
+# from django.test import TestCase
 
 # Create your tests here.

cotisations/tex.py

@@ -24,43 +24,44 @@ Module in charge of rendering some LaTex templates.
 Used to generated PDF invoice.
 """
 
+import tempfile
+from subprocess import Popen, PIPE
+import os
+from datetime import datetime
+
 from django.template.loader import get_template
 from django.template import Context
 from django.http import HttpResponse
 from django.conf import settings
 from django.utils.text import slugify
 
-import tempfile
-from subprocess import Popen, PIPE
-import os
-
 
 TEMP_PREFIX = getattr(settings, 'TEX_TEMP_PREFIX', 'render_tex-')
 CACHE_PREFIX = getattr(settings, 'TEX_CACHE_PREFIX', 'render-tex')
 CACHE_TIMEOUT = getattr(settings, 'TEX_CACHE_TIMEOUT', 86400)  # 1 day
 
 
-def render_invoice(request, ctx={}):
+def render_invoice(_request, ctx={}):
     """
     Render an invoice using some available information such as the current
     date, the user, the articles, the prices, ...
     """
     filename = '_'.join([
         'invoice',
-        slugify(ctx['asso_name']),
+        slugify(ctx.get('asso_name', "")),
-        slugify(ctx['recipient_name']),
+        slugify(ctx.get('recipient_name', "")),
-        str(ctx['DATE'].year),
+        str(ctx.get('DATE', datetime.now()).year),
-        str(ctx['DATE'].month),
+        str(ctx.get('DATE', datetime.now()).month),
-        str(ctx['DATE'].day),
+        str(ctx.get('DATE', datetime.now()).day),
     ])
-    r = render_tex(request, 'cotisations/factures.tex', ctx)
+    r = render_tex(_request, 'cotisations/factures.tex', ctx)
     r['Content-Disposition'] = 'attachment; filename="{name}.pdf"'.format(
         name=filename
     )
     return r
 
 
-def render_tex(request, template, ctx={}):
+def render_tex(_request, template, ctx={}):
     """
     Creates a PDF from a LaTex templates using pdflatex.
     Writes it in a temporary directory and send back an HTTP response for

cotisations/urls.py

@@ -19,6 +19,9 @@
 # You should have received a copy of the GNU General Public License along
 # with this program; if not, write to the Free Software Foundation, Inc.,
 # 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+"""cotisations.urls
+The defined URLs for the Cotisations app
+"""
 
 from __future__ import unicode_literals
 
@@ -29,106 +32,131 @@ from . import views
 from . import payment
 
 urlpatterns = [
-    url(r'^new_facture/(?P<userid>[0-9]+)$',
+    url(
+        r'^new_facture/(?P<userid>[0-9]+)$',
         views.new_facture,
         name='new-facture'
-        ),
+    ),
-    url(r'^edit_facture/(?P<factureid>[0-9]+)$',
+    url(
+        r'^edit_facture/(?P<factureid>[0-9]+)$',
         views.edit_facture,
         name='edit-facture'
-        ),
+    ),
-    url(r'^del_facture/(?P<factureid>[0-9]+)$',
+    url(
+        r'^del_facture/(?P<factureid>[0-9]+)$',
         views.del_facture,
         name='del-facture'
-        ),
+    ),
-    url(r'^facture_pdf/(?P<factureid>[0-9]+)$',
+    url(
+        r'^facture_pdf/(?P<factureid>[0-9]+)$',
         views.facture_pdf,
         name='facture-pdf'
-        ),
+    ),
-    url(r'^new_facture_pdf/$',
+    url(
+        r'^new_facture_pdf/$',
         views.new_facture_pdf,
         name='new-facture-pdf'
-        ),
+    ),
-    url(r'^credit_solde/(?P<userid>[0-9]+)$',
+    url(
+        r'^credit_solde/(?P<userid>[0-9]+)$',
         views.credit_solde,
         name='credit-solde'
-        ),
+    ),
-    url(r'^add_article/$',
+    url(
+        r'^add_article/$',
         views.add_article,
         name='add-article'
-        ),
+    ),
-    url(r'^edit_article/(?P<articleid>[0-9]+)$',
+    url(
+        r'^edit_article/(?P<articleid>[0-9]+)$',
         views.edit_article,
         name='edit-article'
-        ),
+    ),
-    url(r'^del_article/$',
+    url(
+        r'^del_article/$',
         views.del_article,
         name='del-article'
-        ),
+    ),
-    url(r'^add_paiement/$',
+    url(
+        r'^add_paiement/$',
         views.add_paiement,
         name='add-paiement'
-        ),
+    ),
-    url(r'^edit_paiement/(?P<paiementid>[0-9]+)$',
+    url(
+        r'^edit_paiement/(?P<paiementid>[0-9]+)$',
         views.edit_paiement,
         name='edit-paiement'
-        ),
+    ),
-    url(r'^del_paiement/$',
+    url(
+        r'^del_paiement/$',
         views.del_paiement,
         name='del-paiement'
-        ),
+    ),
-    url(r'^add_banque/$',
+    url(
+        r'^add_banque/$',
         views.add_banque,
         name='add-banque'
-        ),
+    ),
-    url(r'^edit_banque/(?P<banqueid>[0-9]+)$',
+    url(
+        r'^edit_banque/(?P<banqueid>[0-9]+)$',
         views.edit_banque,
         name='edit-banque'
-        ),
+    ),
-    url(r'^del_banque/$',
+    url(
+        r'^del_banque/$',
         views.del_banque,
         name='del-banque'
-        ),
+    ),
-    url(r'^index_article/$',
+    url(
+        r'^index_article/$',
         views.index_article,
         name='index-article'
-        ),
+    ),
-    url(r'^index_banque/$',
+    url(
+        r'^index_banque/$',
         views.index_banque,
         name='index-banque'
-        ),
+    ),
-    url(r'^index_paiement/$',
+    url(
+        r'^index_paiement/$',
         views.index_paiement,
         name='index-paiement'
-        ),
+    ),
-    url(r'history/(?P<object_name>\w+)/(?P<object_id>[0-9]+)$',
+    url(
+        r'history/(?P<object_name>\w+)/(?P<object_id>[0-9]+)$',
         re2o.views.history,
         name='history',
         kwargs={'application': 'cotisations'},
-        ),
+    ),
-    url(r'^control/$',
+    url(
+        r'^control/$',
         views.control,
         name='control'
-        ),
+    ),
-    url(r'^new_facture_solde/(?P<userid>[0-9]+)$',
+    url(
+        r'^new_facture_solde/(?P<userid>[0-9]+)$',
         views.new_facture_solde,
         name='new_facture_solde'
-        ),
+    ),
-    url(r'^recharge/$',
+    url(
+        r'^recharge/$',
         views.recharge,
         name='recharge'
-        ),
+    ),
-    url(r'^payment/accept/(?P<factureid>[0-9]+)$',
+    url(
+        r'^payment/accept/(?P<factureid>[0-9]+)$',
         payment.accept_payment,
         name='accept_payment'
-        ),
+    ),
-    url(r'^payment/refuse/$',
+    url(
+        r'^payment/refuse/$',
         payment.refuse_payment,
         name='refuse_payment'
-        ),
+    ),
-    url(r'^payment/ipn/$',
+    url(
+        r'^payment/ipn/$',
         payment.ipn,
         name='ipn'
-        ),
+    ),
     url(r'^$', views.index, name='index'),
 ]

cotisations/views.py

@@ -23,22 +23,23 @@
 # App de gestion des users pour re2o
 # Goulven Kermarec, Gabriel Détraz
 # Gplv2
+"""cotisations.views
+The different views used in the Cotisations module
+"""
+
 from __future__ import unicode_literals
 import os
 
 from django.urls import reverse
 from django.shortcuts import render, redirect
-from django.core.validators import MaxValueValidator
+from django.contrib.auth.decorators import login_required
-from django.contrib.auth.decorators import login_required, permission_required
 from django.contrib import messages
 from django.db.models import ProtectedError
-from django.db import transaction
 from django.db.models import Q
 from django.forms import modelformset_factory, formset_factory
 from django.utils import timezone
 from django.utils.translation import ugettext as _
-from django.views.decorators.csrf import csrf_exempt
+
-from django.views.decorators.debug import sensitive_variables
 # Import des models, forms et fonctions re2o
 from reversion import revisions as reversion
 from users.models import User
@@ -70,7 +71,6 @@ from .forms import (
     SelectUserArticleForm,
     SelectClubArticleForm,
     CreditSoldeForm,
-    NewFactureSoldeForm,
     RechargeForm
 )
 from . import payment as online_payment
@@ -122,7 +122,7 @@ def new_facture(request, user, userid):
             if user_balance:
                 # TODO : change Paiement to Payment
                 if new_invoice_instance.paiement == (
-                    Paiement.objects.get_or_create(moyen='solde')[0]
+                        Paiement.objects.get_or_create(moyen='solde')[0]
                 ):
                     total_price = 0
                     for art_item in articles:
@@ -263,7 +263,7 @@ def new_facture_pdf(request):
 # TODO : change facture to invoice
 @login_required
 @can_view(Facture)
-def facture_pdf(request, facture, factureid):
+def facture_pdf(request, facture, _factureid):
     """
     View used to generate a PDF file from  an existing invoice in database
     Creates a line for each Purchase (thus article sold) and generate the
@@ -306,7 +306,7 @@ def facture_pdf(request, facture, factureid):
 # TODO : change facture to invoice
 @login_required
 @can_edit(Facture)
-def edit_facture(request, facture, factureid):
+def edit_facture(request, facture, _factureid):
     """
     View used to edit an existing invoice.
     Articles can be added or remove to the invoice and quantity
@@ -347,7 +347,7 @@ def edit_facture(request, facture, factureid):
 # TODO : change facture to invoice
 @login_required
 @can_delete(Facture)
-def del_facture(request, facture, factureid):
+def del_facture(request, facture, _factureid):
     """
     View used to delete an existing invocie.
     """
@@ -368,7 +368,7 @@ def del_facture(request, facture, factureid):
 @login_required
 @can_create(Facture)
 @can_edit(User)
-def credit_solde(request, user, userid):
+def credit_solde(request, user, _userid):
     """
     View used to edit the balance of a user.
     Can be use either to increase or decrease a user's balance.
@@ -425,7 +425,7 @@ def add_article(request):
 
 @login_required
 @can_edit(Article)
-def edit_article(request, article_instance, articleid):
+def edit_article(request, article_instance, _articleid):
     """
     View used to edit an article.
     """
@@ -489,7 +489,7 @@ def add_paiement(request):
 # TODO : chnage paiement to Payment
 @login_required
 @can_edit(Paiement)
-def edit_paiement(request, paiement_instance, paiementid):
+def edit_paiement(request, paiement_instance, _paiementid):
     """
     View used to edit a payment method.
     """
@@ -567,7 +567,7 @@ def add_banque(request):
 # TODO : change banque to bank
 @login_required
 @can_edit(Banque)
-def edit_banque(request, banque_instance, banqueid):
+def edit_banque(request, banque_instance, _bbanqueid):
     """
     View used to edit a bank.
     """