Django views for banking operations

How can I Pythonize/Djangoize/DRY my Django code?

views.py

```
class AccountDetail(DetailView):
model = BankAccount
template_name = 'bank/account_detail.html'

def get(self, *args, **kwargs):
self.object = self.get_object()

if self.object.is_owner(self.request.user.citizen):
return render(self.request, self.template_name, self.get_context_data())
raise Http404

class SendTransfer(SingleObjectMixin, FormView):
model = BankAccount
form_class = SendTransferForm
template_name = 'bank/send_transfer.html'

def dispatch(self, request, *args, **kwargs):
self.object = self.get_object()
return super(SendTransfer, self).dispatch(request, *args, **kwargs)

def get_object(self, queryset=None):
obj = super(SendTransfer, self).get_object(queryset)

if not obj.is_owner(self.request.user.citizen):
raise Http404

return obj

def form_valid(self, form):
data = form.cleaned_data
MoneyTransfer.objects.create(sender=self.object,
receiver=data['receiver'], # ModelChoiceField in the form
total=data['total'], # FloatField in the form, etc.
when=timezone.localtime(timezone.now()),
comment=data['comment'])
return redirect('AccountDetail', self.object.pk)

def form_invalid(self, form):
return render(self.request, self.template_name, self.get_context_data())

def get_form_kwargs(self):
kwargs = super(SendTransfer, self).get_form_kwargs()
kwargs['sender'] = BankAccount.objects.get(id=self.kwargs['pk'])
kwargs['user'] = self.request.user
return kwargs

class OutcomeTransfers(DetailView):
model = BankAccount
template_name = 'bank/report.html'

def get_object(self, queryset=None):
obj = super(OutcomeTransfers, self).get_object(queryset)

-
Use exists instead of count when trying to find if there are objects that match that filter.

self.organization.owners.filter(id=citizen.id).exists()

-
Use generic foreign keys instead of multiple one-to-one relationships. Change organization and citizen to one field owner that is generic. There are lots of great examples on these but here is just one.

-
Why do you have Cashing, Deposit and MoneyTransfer as seperate models instead of one model Transaction with a type choice field? Then you can just do your math based on the type

class Transaction(models.Model):
    DEPOSIT = 1
    CASHING = 2
    TRANSFER = 3

    CHOICES = (
        (DEPOSIT, 'Deposit'),
        (CASHING, 'Cashing'),
        (TRANSFER, 'Transfer')
    )

    account = models.ForeignKey(BankAccount)
    total = models.DecimalField(max_digits=10, decimal_places=3)
    banker = models.ForeignKey(Citizen)
    when = models.DateTimeField()
    type = models.IntegerField(choices=CHOICES)

    objects = TransferQuerySet.as_manager()

-
Implement a IsOwnerMixin that has a get_queryset method that does a filter on the objects where owner is equal to the users owner.

def get_queryset(self):
     default_qs = super(IsOwnerMixin, self).get_queryset()
     qs = default_qs.filter(owner=self.request.user.citizen)
     return qs

This of course is an overly simplified example. You will want to add logic to figure out if a user is apart of an organization and if so filter based on the organization id instead of the citizen id. Here is an example taken straight out of our code base that only returns objects that belong to a users `company'. This isn't all of the code but it is enough to get you started.

class CompanyRelationMixin(object):
    company_path = "company"
    force_company_only = False

    def is_customer(self):
        return not self.request.user.is_superuser and self.request.user.user_profile.company

    def company_fk_allows_null(self, qs):
        ## Figures out which field on model relates to `company`, these details aren't important for your case

    def get_queryset(self):
        default_qs = super(CompanyRelationMixin, self).get_queryset()
        request = self.request
        options = ''

        if not self.force_company_only:
            allows_none = self.company_fk_allows_null(default_qs)
            if allows_none:
                options = Q(**{self.company_path: None})

        if self.is_customer():
            company = request.user.user_profile.company.pk
            if options:
                 options |= Q(**{self.company_path: company})
            else:
                 options = Q(**{self.company_path: company})

        elif request.user.is_superuser:
             try:
                 request.QUERY_PARAMS['company']
             except KeyError:
                return default_qs
             options = Q(**{self.company_path: request.QUERY_PARAMS['company']})
             options |= Q(**{self.company_path: None})
        if options:
            default_qs = default_qs.filter(options)
        return default_qs

These are extremely useful as the amount of views you have start to grow. This remove a LOT of redundancy in your views.

I will add more later if I can think of anything else that will help DRY this up.

StackExchange Code Review Q#116660, answer score: 4