From f2c91199d12288ae31372e338e8307af7de08bc3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ma=C3=ABl=20Kervella?= Date: Sat, 14 Apr 2018 13:39:51 +0000 Subject: [PATCH] Pylint compliance on cotisations --- cotisations/__init__.py | 3 + cotisations/admin.py | 3 + cotisations/forms.py | 15 +++- cotisations/models.py | 58 ++++++------ cotisations/payment.py | 5 +- cotisations/payment_utils/comnpay.py | 47 +++++----- cotisations/tests.py | 5 +- cotisations/tex.py | 25 +++--- cotisations/urls.py | 128 ++++++++++++++++----------- cotisations/views.py | 28 +++--- 10 files changed, 185 insertions(+), 132 deletions(-) diff --git a/cotisations/__init__.py b/cotisations/__init__.py index df6e4256..ee5a305d 100644 --- a/cotisations/__init__.py +++ b/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 * diff --git a/cotisations/admin.py b/cotisations/admin.py index 8186e4e3..587bc066 100644 --- a/cotisations/admin.py +++ b/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 diff --git a/cotisations/forms.py b/cotisations/forms.py index 4414de3e..a6647a45 100644 --- a/cotisations/forms.py +++ b/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( diff --git a/cotisations/models.py b/cotisations/models.py index c7b940d6..fa3156f0 100644 --- a/cotisations/models.py +++ b/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 \ - not self.facture.user.can_edit( - user_request, *args, **kwargs)[0]: + elif (not user_request.has_perm('cotisations.change_all_facture') and + not self.facture.user.can_edit( + 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 \ - (self.facture.control or not self.facture.valid): + elif (not user_request.has_perm('cotisations.change_all_vente') and + (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): - if not user_request.has_perm('cotisations.view_vente') and \ - self.facture.user != user_request: + def can_view(self, user_request, *_args, **_kwargs): + if (not user_request.has_perm('cotisations.view_vente') and + 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') diff --git a/cotisations/payment.py b/cotisations/payment.py index 8efdd344..b03e1c55 100644 --- a/cotisations/payment.py +++ b/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 diff --git a/cotisations/payment_utils/comnpay.py b/cotisations/payment_utils/comnpay.py index ad0d8c77..6c73463f 100644 --- a/cotisations/payment_utils/comnpay.py +++ b/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(): - - vad_number = "" - secret_key = "" - urlRetourOK = "" - urlRetourNOK = "" - urlIPN = "" - source = "" - typeTr = "D" + """ The class representing a transaction with all the functions + used during the negociation + """ 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), - idTPE=self.vad_number, - idTransaction=self.idTransaction, - devise="EUR", - lang='fr', - nom_produit=produit, - source=self.source, - urlRetourOK=self.urlRetourOK, - urlRetourNOK=self.urlRetourNOK, - typeTr=str(self.typeTr) + montant=str(montant), + idTPE=self.vad_number, + idTransaction=self.idTransaction, + devise="EUR", + lang='fr', + nom_produit=produit, + source=self.source, + urlRetourOK=self.urlRetourOK, + urlRetourNOK=self.urlRetourNOK, + typeTr=str(self.typeTr) ) if self.urlIPN != "": @@ -63,12 +66,14 @@ class Payment(): for key in array_tpe: ret += ''.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"] diff --git a/cotisations/tests.py b/cotisations/tests.py index 21fa6d24..46b9d708 100644 --- a/cotisations/tests.py +++ b/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. diff --git a/cotisations/tex.py b/cotisations/tex.py index 95f12b93..f456fe8a 100644 --- a/cotisations/tex.py +++ b/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['recipient_name']), - str(ctx['DATE'].year), - str(ctx['DATE'].month), - str(ctx['DATE'].day), + slugify(ctx.get('asso_name', "")), + slugify(ctx.get('recipient_name', "")), + str(ctx.get('DATE', datetime.now()).year), + str(ctx.get('DATE', datetime.now()).month), + 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 diff --git a/cotisations/urls.py b/cotisations/urls.py index 05b1a7a3..6eafe721 100644 --- a/cotisations/urls.py +++ b/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[0-9]+)$', + url( + r'^new_facture/(?P[0-9]+)$', views.new_facture, name='new-facture' - ), - url(r'^edit_facture/(?P[0-9]+)$', + ), + url( + r'^edit_facture/(?P[0-9]+)$', views.edit_facture, name='edit-facture' - ), - url(r'^del_facture/(?P[0-9]+)$', + ), + url( + r'^del_facture/(?P[0-9]+)$', views.del_facture, name='del-facture' - ), - url(r'^facture_pdf/(?P[0-9]+)$', + ), + url( + r'^facture_pdf/(?P[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[0-9]+)$', + ), + url( + r'^credit_solde/(?P[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[0-9]+)$', + ), + url( + r'^edit_article/(?P[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[0-9]+)$', + ), + url( + r'^edit_paiement/(?P[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[0-9]+)$', + ), + url( + r'^edit_banque/(?P[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\w+)/(?P[0-9]+)$', + ), + url( + r'history/(?P\w+)/(?P[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[0-9]+)$', + ), + url( + r'^new_facture_solde/(?P[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[0-9]+)$', + ), + url( + r'^payment/accept/(?P[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'), ] diff --git a/cotisations/views.py b/cotisations/views.py index c8c68c19..acf8c404 100644 --- a/cotisations/views.py +++ b/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, permission_required +from django.contrib.auth.decorators import login_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. """