From a17e8d9b8caf8e9c59eff2a2bb4b3e3d266f1b63 Mon Sep 17 00:00:00 2001 From: Marc Aymerich Date: Tue, 2 Jun 2015 12:59:49 +0000 Subject: [PATCH] Imporvements on payments, bills and upgraded domain label validation to RFC 1123 --- TODO.md | 11 ++-- .../project_template/project_name/settings.py | 3 + .../project_template/project_name/urls.py | 1 + orchestra/contrib/bills/admin.py | 10 +-- orchestra/contrib/bills/filters.py | 64 ++++++++++++++++--- orchestra/contrib/bills/models.py | 35 ++++++---- .../bills/templates/bills/microspective.html | 2 +- orchestra/contrib/domains/validators.py | 18 +++--- orchestra/contrib/mailer/engine.py | 2 - orchestra/contrib/mailer/tasks.py | 1 + orchestra/contrib/payments/actions.py | 23 ++----- orchestra/contrib/payments/admin.py | 11 +++- orchestra/contrib/payments/helpers.py | 37 +++++++++++ .../payments/methods/sepadirectdebit.py | 6 +- orchestra/contrib/payments/models.py | 3 +- .../payments/transaction/get_processes.html | 2 +- orchestra/models/fields.py | 37 +++++++++-- orchestra/urls.py | 9 ++- orchestra/views.py | 20 ++++++ 19 files changed, 215 insertions(+), 80 deletions(-) create mode 100644 orchestra/contrib/payments/helpers.py create mode 100644 orchestra/views.py diff --git a/TODO.md b/TODO.md index 79391137..67d4336a 100644 --- a/TODO.md +++ b/TODO.md @@ -407,20 +407,17 @@ uwsgi --reload /tmp/project-master.pid touch /tmp/somefile - # Change zone ttl # batch zone edditing # inherit registers from parent? -# Disable pagination on membership fees (allways one page) - # datetime metric storage granularity: otherwise innacurate detection of billed metric on order.billed_on # Serializers.validation migration to DRF3: grep -r 'attrs, source' *|grep -v '~' serailzer self.instance on create. -# generate Direct debit q19 on a protected path, or store it on the transaction.proc -# regenerate direct debit q19 -# add transproc.method for regeneration +# set_password serializer: "just-the-password" not {"password": "password"} -# TODO wrapp admin delete: delete proc undo processing on related transactions +# use namedtuples! + +# Negative transactionsx diff --git a/orchestra/conf/project_template/project_name/settings.py b/orchestra/conf/project_template/project_name/settings.py index 6c940c63..7ddeafbc 100644 --- a/orchestra/conf/project_template/project_name/settings.py +++ b/orchestra/conf/project_template/project_name/settings.py @@ -161,6 +161,9 @@ STATIC_URL = '/static/' # Example: "/home/media/media.lawrence.com/static/" STATIC_ROOT = os.path.join(BASE_DIR, 'static') +# Absolute filesystem path to the directory that will hold user-uploaded files. +MEDIA_ROOT = os.path.join(BASE_DIR, 'media') + # Path used for database translations files LOCALE_PATHS = ( diff --git a/orchestra/conf/project_template/project_name/urls.py b/orchestra/conf/project_template/project_name/urls.py index 8dbfad58..3ffcec45 100644 --- a/orchestra/conf/project_template/project_name/urls.py +++ b/orchestra/conf/project_template/project_name/urls.py @@ -1,5 +1,6 @@ from django.conf.urls import include, url + urlpatterns = [ url(r'', include('orchestra.urls')), ] diff --git a/orchestra/contrib/bills/admin.py b/orchestra/contrib/bills/admin.py index eb431fd2..b1d84684 100644 --- a/orchestra/contrib/bills/admin.py +++ b/orchestra/contrib/bills/admin.py @@ -17,7 +17,7 @@ from orchestra.contrib.accounts.admin import AccountAdminMixin, AccountAdmin from orchestra.forms.widgets import paddingCheckboxSelectMultiple from . import settings, actions -from .filters import BillTypeListFilter, HasBillContactListFilter, PositivePriceListFilter +from .filters import BillTypeListFilter, HasBillContactListFilter, TotalListFilter, PaymentStateListFilter from .models import Bill, Invoice, AmendmentInvoice, Fee, AmendmentFee, ProForma, BillLine, BillContact @@ -39,7 +39,7 @@ class BillLineInline(admin.TabularInline): order_link = admin_link('order', display='pk') def display_total(self, line): - total = line.get_total() + total = line.compute_total() sublines = line.sublines.all() if sublines: content = '\n'.join(['%s: %s' % (sub.description, sub.total) for sub in sublines]) @@ -89,7 +89,7 @@ class ClosedBillLineInline(BillLineInline): display_subtotal.allow_tags = True def display_total(self, line): - return line.get_total() + return line.compute_total() display_total.short_description = _("Total") display_total.allow_tags = True @@ -180,7 +180,7 @@ class BillAdmin(AccountAdminMixin, ExtendedModelAdmin): 'number', 'type_link', 'account_link', 'created_on_display', 'num_lines', 'display_total', 'display_payment_state', 'is_open', 'is_sent' ) - list_filter = (BillTypeListFilter, 'is_open', 'is_sent', PositivePriceListFilter) + list_filter = (BillTypeListFilter, 'is_open', 'is_sent', TotalListFilter, PaymentStateListFilter) add_fields = ('account', 'type', 'is_open', 'due_on', 'comments') fieldsets = ( (None, { @@ -238,7 +238,7 @@ class BillAdmin(AccountAdminMixin, ExtendedModelAdmin): state = bill.get_payment_state_display().upper() color = PAYMENT_STATE_COLORS.get(bill.payment_state, 'grey') return '{name}'.format( - url=url, color=color, name=state) + url=url, color=color, name=state) display_payment_state.allow_tags = True display_payment_state.short_description = _("Payment") diff --git a/orchestra/contrib/bills/filters.py b/orchestra/contrib/bills/filters.py index b7fdd990..646ea2fe 100644 --- a/orchestra/contrib/bills/filters.py +++ b/orchestra/contrib/bills/filters.py @@ -1,5 +1,7 @@ from django.contrib.admin import SimpleListFilter from django.core.urlresolvers import reverse +from django.db.models import Q +from django.utils.safestring import mark_safe from django.utils.translation import ugettext_lazy as _ @@ -37,21 +39,25 @@ class BillTypeListFilter(SimpleListFilter): } -class PositivePriceListFilter(SimpleListFilter): - title = _("positive price") - parameter_name = 'positive_price' +class TotalListFilter(SimpleListFilter): + title = _("total") + parameter_name = 'total' def lookups(self, request, model_admin): return ( - ('True', _("Yes")), - ('False', _("No")), + ('gt', mark_safe("total > 0")), + ('eq', "total = 0"), + ('lt', mark_safe("total < 0")), ) def queryset(self, request, queryset): - if self.value() == 'True': + if self.value() == 'gt': return queryset.filter(computed_total__gt=0) - if self.value() == 'False': - return queryset.filter(computed_total__lte=0) + elif self.value() == 'eq': + return queryset.filter(computed_total=0) + elif self.value() == 'lt': + return queryset.filter(computed_total__lt=0) + return queryset class HasBillContactListFilter(SimpleListFilter): @@ -68,5 +74,45 @@ class HasBillContactListFilter(SimpleListFilter): def queryset(self, request, queryset): if self.value() == 'True': return queryset.filter(billcontact__isnull=False) - if self.value() == 'False': + elif self.value() == 'False': return queryset.filter(billcontact__isnull=True) + + +class PaymentStateListFilter(SimpleListFilter): + title = _("payment state") + parameter_name = 'payment_state' + + def lookups(self, request, model_admin): + return ( + ('OPEN', _("Open")), + ('PAID', _("Paid")), + ('PENDING', _("Pending")), + ('BAD_DEBT', _("Bad debt")), + ) + + def queryset(self, request, queryset): + Transaction = queryset.model.transactions.related.related_model + if self.value() == 'OPEN': + return queryset.filter(Q(is_open=True)|Q(type=queryset.model.PROFORMA)) + elif self.value() == 'PAID': + zeros = queryset.filter(computed_total=0).values_list('id', flat=True) + ammounts = Transaction.objects.exclude(bill_id__in=zeros).secured().group_by('bill_id') + paid = [] + for bill_id, total in queryset.exclude(computed_total=0).values_list('id', 'computed_total'): + try: + ammount = sum([t.ammount for t in ammounts[bill_id]]) + except KeyError: + pass + else: + if abs(total) <= abs(ammount): + paid.append(bill_id) + return queryset.filter(Q(computed_total=0)|Q(id__in=paid)) + elif self.value() == 'PENDING': + has_transaction = queryset.exclude(transactions__isnull=True) + non_rejected = has_transaction.exclude(transactions__state=Transaction.REJECTED) + non_rejected = non_rejected.values_list('id', flat=True).distinct() + return queryset.filter(pk__in=non_rejected) + elif self.value() == 'BAD_DEBT': + non_rejected = queryset.exclude(transactions__state=Transaction.REJECTED) + non_rejected = non_rejected.values_list('id', flat=True).distinct() + return queryset.exclude(pk__in=non_rejected) diff --git a/orchestra/contrib/bills/models.py b/orchestra/contrib/bills/models.py index 64707f94..199c1d81 100644 --- a/orchestra/contrib/bills/models.py +++ b/orchestra/contrib/bills/models.py @@ -93,7 +93,7 @@ class Bill(models.Model): due_on = models.DateField(_("due on"), null=True, blank=True) updated_on = models.DateField(_("updated on"), auto_now=True) # TODO allways compute total or what? - total = models.DecimalField(max_digits=12, decimal_places=2, default=0) + total = models.DecimalField(max_digits=12, decimal_places=2, null=True) comments = models.TextField(_("comments"), blank=True) html = models.TextField(_("HTML"), blank=True) @@ -105,6 +105,10 @@ class Bill(models.Model): def __str__(self): return self.number + @classmethod + def get_class_type(cls): + return cls.__name__.upper() + @cached_property def seller(self): return Account.get_main().billcontact @@ -113,20 +117,29 @@ class Bill(models.Model): def buyer(self): return self.account.billcontact + @property + def has_multiple_pages(self): + return self.type != self.FEE + @cached_property def payment_state(self): if self.is_open or self.get_type() == self.PROFORMA: return self.OPEN secured = self.transactions.secured().amount() or 0 - if secured >= self.total: + if abs(secured) >= abs(self.get_total()): return self.PAID elif self.transactions.exclude_rejected().exists(): return self.PENDING return self.BAD_DEBT - @property - def has_multiple_pages(self): - return self.type != self.FEE + def get_total(self): + if not self.is_open: + return self.total + try: + return self.computed_total + except AttributeError: + self.computed_total = self.compute_total() + return self.computed_total def get_payment_state_display(self): value = self.payment_state @@ -135,10 +148,6 @@ class Bill(models.Model): def get_current_transaction(self): return self.transactions.exclude_rejected().first() - @classmethod - def get_class_type(cls): - return cls.__name__.upper() - def get_type(self): return self.type or self.get_class_type() @@ -177,7 +186,7 @@ class Bill(models.Model): payment = self.account.paymentsources.get_default() if not self.due_on: self.due_on = self.get_due_date(payment=payment) - self.total = self.get_total() + self.total = self.compute_total() transaction = None if self.get_type() != self.PROFORMA: transaction = self.transactions.create(bill=self, source=payment, amount=self.total) @@ -241,7 +250,7 @@ class Bill(models.Model): self.number = self.get_number() super(Bill, self).save(*args, **kwargs) - def get_subtotals(self): + def compute_subtotals(self): subtotals = {} lines = self.lines.annotate(totals=(F('subtotal') + Coalesce(F('sublines__total'), 0))) for tax, total in lines.values_list('tax', 'totals'): @@ -250,7 +259,7 @@ class Bill(models.Model): subtotals[tax] = (subtotal, round(tax/100*subtotal, 2)) return subtotals - def get_total(self): + def compute_total(self): totals = self.lines.annotate( totals=(F('subtotal') + Coalesce(F('sublines__total'), 0)) * (1+F('tax')/100)) return round(totals.aggregate(Sum('totals'))['totals__sum'], 2) @@ -304,7 +313,7 @@ class BillLine(models.Model): def __str__(self): return "#%i" % self.pk - def get_total(self): + def compute_total(self): """ Computes subline discounts """ if self.pk: return self.subtotal + sum([sub.total for sub in self.sublines.all()]) diff --git a/orchestra/contrib/bills/templates/bills/microspective.html b/orchestra/contrib/bills/templates/bills/microspective.html index e20e2b0b..e2bb421d 100644 --- a/orchestra/contrib/bills/templates/bills/microspective.html +++ b/orchestra/contrib/bills/templates/bills/microspective.html @@ -107,7 +107,7 @@

 
- {% for tax, subtotal in bill.get_subtotals.items %} + {% for tax, subtotal in bill.compute_subtotals.items %} {% trans "subtotal" %} {{ tax }}% {% trans "VAT" %} {{ subtotal | first }} &{{ currency.lower }};
diff --git a/orchestra/contrib/domains/validators.py b/orchestra/contrib/domains/validators.py index d8aeee6a..569b81e1 100644 --- a/orchestra/contrib/domains/validators.py +++ b/orchestra/contrib/domains/validators.py @@ -49,21 +49,19 @@ def validate_zone_interval(value): def validate_zone_label(value): """ - http://www.ietf.org/rfc/rfc1035.txt - The labels must follow the rules for ARPANET host names. They must - start with a letter, end with a letter or digit, and have as interior - characters only letters, digits, and hyphen. There are also some - restrictions on the length. Labels must be 63 characters or less. + Allowable characters in a label for a host name are only ASCII letters, digits, and the `-' character. + Labels may not be all numbers, but may have a leading digit (e.g., 3com.com). + Labels must end and begin only with a letter or digit. See [RFC 1035] and [RFC 1123]. """ - if not re.match(r'^[a-z][\.\-0-9a-z]*[\.0-9a-z]$', value): - msg = _("Labels must start with a letter, end with a letter or digit, " - "and have as interior characters only letters, digits, and hyphen") + if not re.match(r'^[a-z0-9][\.\-0-9a-z]*[\.0-9a-z]$', value): + msg = _("Labels must start and end with a letter or digit, " + "and have as interior characters only letters, digits, and hyphen.") raise ValidationError(msg) if not value.endswith('.'): - msg = _("Use a fully expanded domain name ending with a dot") + msg = _("Use a fully expanded domain name ending with a dot.") raise ValidationError(msg) if len(value) > 63: - raise ValidationError(_("Labels must be 63 characters or less")) + raise ValidationError(_("Labels must be 63 characters or less.")) def validate_mx_record(value): diff --git a/orchestra/contrib/mailer/engine.py b/orchestra/contrib/mailer/engine.py index b70e6e9f..da6f4dbc 100644 --- a/orchestra/contrib/mailer/engine.py +++ b/orchestra/contrib/mailer/engine.py @@ -14,8 +14,6 @@ from .models import Message def send_message(message, num=0, connection=None, bulk=100): - if not message.pk: - message.save() if num >= bulk: connection.close() connection = None diff --git a/orchestra/contrib/mailer/tasks.py b/orchestra/contrib/mailer/tasks.py index ed5e86eb..5cbe908f 100644 --- a/orchestra/contrib/mailer/tasks.py +++ b/orchestra/contrib/mailer/tasks.py @@ -10,6 +10,7 @@ from . import engine, settings @task def send_message(message): + message.save() engine.send_message(message) diff --git a/orchestra/contrib/payments/actions.py b/orchestra/contrib/payments/actions.py index 78349c8d..c04228e2 100644 --- a/orchestra/contrib/payments/actions.py +++ b/orchestra/contrib/payments/actions.py @@ -11,6 +11,7 @@ from django.utils.translation import ungettext, ugettext_lazy as _ from orchestra.admin.decorators import action_with_confirmation from orchestra.admin.utils import change_url +from . import helpers from .methods import PaymentMethod from .models import Transaction @@ -175,25 +176,9 @@ commit.verbose_name = _("Commit") def delete_selected(modeladmin, request, queryset): """ Has to have same name as admin.actions.delete_selected """ - if not queryset: - messages.warning(request, "No transaction process selected.") - return - if queryset.exclude(transactions__state=Transaction.WAITTING_EXECUTION).exists(): - messages.error(request, "Done nothing. Not all related transactions in waitting execution.") - return - # Store before deleting - related_transactions = [] - for process in queryset: - related_transactions.extend(process.transactions.filter(state=Transaction.WAITTING_EXECUTION)) + related_transactions = helpers.pre_delete_processes(modelamdin, request, queryset) response = actions.delete_selected(modeladmin, request, queryset) - if response is None: - # Confirmation - num = 0 - for transaction in related_transactions: - transaction.state = Transaction.WAITTING_PROCESSING - transaction.save(update_fields=('state',)) - num += 1 - modeladmin.log_change(request, transaction, _("Unprocessed")) - messages.success(request, "%i related transactions marked as waitting for processing." % num) + if response is None: + helpers.post_delete_processes(modelamdin, request, related_transactions) return response delete_selected.short_description = actions.delete_selected.short_description diff --git a/orchestra/contrib/payments/admin.py b/orchestra/contrib/payments/admin.py index 3d52bdf9..1c3740d2 100644 --- a/orchestra/contrib/payments/admin.py +++ b/orchestra/contrib/payments/admin.py @@ -1,5 +1,6 @@ from django.contrib import admin from django.core.urlresolvers import reverse +from django.http import HttpResponseRedirect from django.utils.translation import ugettext_lazy as _ from orchestra.admin import ChangeViewActionsMixin, ExtendedModelAdmin @@ -7,7 +8,7 @@ from orchestra.admin.utils import admin_colored, admin_link from orchestra.contrib.accounts.admin import AccountAdminMixin, SelectAccountAdminMixin from orchestra.plugins.admin import SelectPluginAdminMixin -from . import actions +from . import actions, helpers from .methods import PaymentMethod from .models import PaymentSource, Transaction, TransactionProcess @@ -166,6 +167,14 @@ class TransactionProcessAdmin(ChangeViewActionsMixin, admin.ModelAdmin): elif obj.state == TransactionProcess.ABORTED: exclude = ['mark_process_as_executed', 'abort', 'commit'] return [action for action in actions if action.__name__ not in exclude] + + def delete_view(self, request, object_id, extra_context=None): + queryset = self.model.objects.filter(id=object_id) + related_transactions = helpers.pre_delete_processes(self, request, queryset) + response = super(TransactionProcessAdmin, self).delete_view(request, object_id, extra_context) + if isinstance(response, HttpResponseRedirect): + helpers.post_delete_processes(self, request, related_transactions) + return response admin.site.register(PaymentSource, PaymentSourceAdmin) diff --git a/orchestra/contrib/payments/helpers.py b/orchestra/contrib/payments/helpers.py new file mode 100644 index 00000000..22111062 --- /dev/null +++ b/orchestra/contrib/payments/helpers.py @@ -0,0 +1,37 @@ +from django.contrib import messages +from django.utils.translation import ungettext, ugettext_lazy as _ + +from .models import Transaction + + +def pre_delete_processes(modeladmin, request, queryset): + """ Has to have same name as admin.actions.delete_selected """ + if not queryset: + messages.warning(request, + _("No transaction process selected.")) + return + if queryset.exclude(transactions__state=Transaction.WAITTING_EXECUTION).exists(): + messages.error(request, + _("Done nothing. Not all related transactions in waitting execution.")) + return + # Store before deleting + related_transactions = [] + for process in queryset: + waitting_execution = process.transactions.filter(state=Transaction.WAITTING_EXECUTION) + related_transactions.extend(waitting_execution) + return related_transactions + + +def post_delete_processes(modeladmin, request, related_transactions): + # Confirmation + num = 0 + for transaction in related_transactions: + transaction.state = Transaction.WAITTING_PROCESSING + transaction.save(update_fields=('state',)) + num += 1 + modeladmin.log_change(request, transaction, _("Unprocessed")) + messages.success(request, ungettext( + "One related transaction has been marked as waitting for processing", + "%i related transactions have been marked as waitting for processing." % num, + num + )) diff --git a/orchestra/contrib/payments/methods/sepadirectdebit.py b/orchestra/contrib/payments/methods/sepadirectdebit.py index 52706ad0..98be12c5 100644 --- a/orchestra/contrib/payments/methods/sepadirectdebit.py +++ b/orchestra/contrib/payments/methods/sepadirectdebit.py @@ -185,7 +185,9 @@ class SEPADirectDebit(PaymentMethod): data = transaction.source.data yield E.DrctDbtTxInf( # Direct Debit Transaction Info E.PmtId( # Payment Id - E.EndToEndId(str(transaction.id)) # Payment Id/End to End + E.EndToEndId( # Payment Id/End to End + str(transaction.bill.number)+'-'+str(transaction.id) + ) ), E.InstdAmt( # Instructed Amount str(abs(transaction.amount)), @@ -207,7 +209,7 @@ class SEPADirectDebit(PaymentMethod): ) ), E.Dbtr( # Debtor - E.Nm(account.name), # Name + E.Nm(account.billcontact.get_name()), # Name ), E.DbtrAcct( # Debtor Account E.Id( diff --git a/orchestra/contrib/payments/models.py b/orchestra/contrib/payments/models.py index aba33cfb..9c8a70f9 100644 --- a/orchestra/contrib/payments/models.py +++ b/orchestra/contrib/payments/models.py @@ -4,6 +4,7 @@ from django.utils.functional import cached_property from django.utils.translation import ugettext_lazy as _ from jsonfield import JSONField +from orchestra.models.fields import PrivateFileField from orchestra.models.queryset import group_by from . import settings @@ -164,7 +165,7 @@ class TransactionProcess(models.Model): ) data = JSONField(_("data"), blank=True) - file = models.FileField(_("file"), blank=True) + file = PrivateFileField(_("file"), blank=True) state = models.CharField(_("state"), max_length=16, choices=STATES, default=CREATED) created_at = models.DateTimeField(_("created"), auto_now_add=True) updated_at = models.DateTimeField(_("updated"), auto_now=True) diff --git a/orchestra/contrib/payments/templates/admin/payments/transaction/get_processes.html b/orchestra/contrib/payments/templates/admin/payments/transaction/get_processes.html index d3bb7b74..d214ca6d 100644 --- a/orchestra/contrib/payments/templates/admin/payments/transaction/get_processes.html +++ b/orchestra/contrib/payments/templates/admin/payments/transaction/get_processes.html @@ -6,7 +6,7 @@

{{ content_message }}