Merge pull request #2897 from lordmauve/lint-fix-donate

Fix lint issues for donate app

Author: ericholscher

prospector-more.yml

@@ -3,7 +3,6 @@ inherits: prospector
 strictness: medium
 
 ignore-paths:
-  - donate/
   - restapi/
 
 pylint:

readthedocs/donate/__init__.py

@@ -1 +1,3 @@
+"""Django app for one-time donations and promotions."""
+
 default_app_config = 'readthedocs.donate.apps.DonateAppConfig'

readthedocs/donate/admin.py

@@ -1,3 +1,5 @@
+"""Django admin configuration for the donate app."""
+
 from django.contrib import admin
 from .models import (Supporter, SupporterPromo, Country,
                      PromoImpressions, GeoFilter)
@@ -8,10 +10,11 @@
 
 
 def set_default_countries(modeladmin, request, queryset):
+    del modeladmin, request  # unused arguments
     for project in queryset:
-        filter = project.geo_filters.create(filter_type='exclude')
+        geo_filter = project.geo_filters.create(filter_type='exclude')
         for country in Country.objects.filter(country__in=DEFAULT_EXCLUDES):
-            filter.countries.add(country)
+            geo_filter.countries.add(country)
 set_default_countries.short_description = "Add default exclude countries to this Promo"
 
 

readthedocs/donate/apps.py

@@ -1,3 +1,5 @@
+"""Django app config for the donate app."""
+
 from django.apps import AppConfig
 
 

readthedocs/donate/constants.py

@@ -1,3 +1,5 @@
+"""Constants used by the donate app."""
+
 DISPLAY_CHOICES = (
     ('doc', 'Documentation Pages'),
     ('site-footer', 'Site Footer'),

readthedocs/donate/forms.py

@@ -3,7 +3,6 @@
 import logging
 
 from django import forms
-from django.conf import settings
 from django.utils.translation import ugettext_lazy as _
 
 from readthedocs.payments.forms import StripeModelForm, StripeResourceMixin

readthedocs/donate/models.py

@@ -1,3 +1,7 @@
+"""Django models for the donate app."""
+# We use 'type' and 'hash' heavily in the API here.
+# pylint: disable=redefined-builtin
+
 from django.db import models
 from django.utils.crypto import get_random_string
 from django.utils.translation import ugettext_lazy as _
@@ -33,11 +37,14 @@ class Supporter(models.Model):
     stripe_id = models.CharField(max_length=255)
     subscribed = models.BooleanField(default=False)
 
-    def __str__(self):
+    def __unicode__(self):
         return self.name
 
 
 class SupporterPromo(models.Model):
+
+    """A banner advertisement."""
+
     pub_date = models.DateTimeField(_('Publication date'), auto_now_add=True)
     modified_date = models.DateTimeField(_('Modified date'), auto_now=True)
 
@@ -62,7 +69,7 @@ class SupporterPromo(models.Model):
     class Meta:
         ordering = ('analytics_id', '-live')
 
-    def __str__(self):
+    def __unicode__(self):
         return self.name
 
     def as_dict(self):
@@ -110,7 +117,9 @@ def incr(self, type, project=None):
         impression.save()
 
         # TODO: Support redis, more info on this PR
-        # github.com/rtfd/readthedocs.org/pull/2105/files/1b5f8568ae0a7760f7247149bcff481efc000f32#r58253051
+        #
+        # https://github.com/rtfd/readthedocs.org
+        #     /pull/2105/files/1b5f8568ae0a7760f7247149bcff481efc000f32#r58253051
 
     def view_ratio(self, day=None):
         if not day:
@@ -201,6 +210,9 @@ class PromoImpressions(BaseImpression):
     promo = models.ForeignKey(SupporterPromo, related_name='impressions',
                               blank=True, null=True)
 
+    def __unicode__(self):
+        return u'%s on %s' % (self.promo, self.date)
+
 
 class ProjectImpressions(BaseImpression):
 
@@ -218,6 +230,9 @@ class ProjectImpressions(BaseImpression):
     class Meta:
         unique_together = ('project', 'promo', 'date')
 
+    def __unicode__(self):
+        return u'%s / %s on %s' % (self.promo, self.project, self.date)
+
 
 class Country(models.Model):
     country = CountryField(unique=True)

readthedocs/donate/signals.py

@@ -1,3 +1,5 @@
+"""Django signal plumbing for the donate app."""
+
 import random
 import logging
 
@@ -25,14 +27,14 @@
 
 def show_to_geo(promo, country_code):
     # Remove promo's that exclude this country.
-    for filter in promo.geo_filters.all():
-        if filter.filter_type == INCLUDE:
-            if country_code in filter.codes:
+    for geo_filter in promo.geo_geo_filters.all():
+        if geo_filter.geo_filter_type == INCLUDE:
+            if country_code in geo_filter.codes:
                 continue
             else:
                 return False
-        if filter.filter_type == EXCLUDE:
-            if country_code in filter.codes:
+        if geo_filter.geo_filter_type == EXCLUDE:
+            if country_code in geo_filter.codes:
                 return False
 
     return True
@@ -141,70 +143,89 @@ def get_promo(country_code, programming_language, theme, gold_project=False, gol
     return promo_obj
 
 
+def is_gold_user(user):
+    """Return True if the user is a Gold supporter."""
+    return user.is_authenticated() and (
+        user.gold.count() or
+        user.goldonce.count()
+    )
+
+
+def is_gold_project(project):
+    """Return True if the project has been mapped by a Gold supporter."""
+    return project.gold_owners.count()
+
+
+def get_user_country(request):
+    """Return the ISO country code from geo-IP data, or None if not found."""
+    if not PROMO_GEO_PATH:
+        return None
+    ip = request.META.get('REMOTE_ADDR')
+    if not ip:
+        return None
+    try:
+        geo_response = geo_reader.city(ip)
+        return geo_response.country.iso_code
+    except (AddressNotFoundError, ValueError):  # Invalid IP
+        return None
+
+
 @receiver(footer_response)
-def attach_promo_data(sender, **kwargs):
-    request = kwargs['request']
-    context = kwargs['context']
-    resp_data = kwargs['resp_data']
+def attach_promo_data(sender, request, context, resp_data, **__):
+    """Insert promotion data keys into the footer API response."""
+    del sender  # unused
 
     project = context['project']
     theme = context['theme']
 
-    # Bail out early if promo's are disabled.
-    use_promo = getattr(settings, 'USE_PROMOS', True)
-    if not use_promo:
+    if getattr(settings, 'USE_PROMOS', True):
+        promo = lookup_promo(request, project, theme)
+    else:
+        promo = None
+
+    if promo:
+        resp_data.update({
+            'promo': True,
+            'promo_data': promo,
+        })
+    else:
         resp_data['promo'] = False
-        return
 
-    gold_user = gold_project = False
-    promo_obj = country_code = None
 
-    show_promo = project.allow_promos
+def lookup_promo(request, project, theme):
+    """Look up a promo to show for the given project.
+
+    Return a dict of promo_data for inclusion in the footer response, or None
+    if no promo should be shown.
 
-    # The request is by a GoldUser
-    if request.user.is_authenticated():
-        if request.user.gold.count() or request.user.goldonce.count():
-            gold_user = True
+    """
+    if not project.allow_promos:
+        return None
 
-    # A GoldUser has mapped this project
-    if project.gold_owners.count():
-        gold_project = True
+    gold_user = is_gold_user(request.user)
+    gold_project = is_gold_project(project)
 
-    # Don't show gold users promos.
-    # This will get overridden if we have specific promos for them below.
+    # Don't show promos to gold users or on gold projects for now
+    # (Some day we may show them something customised for them)
     if gold_user or gold_project:
-        show_promo = False
-
-    if PROMO_GEO_PATH:
-        # Get geo information from the IP, but don't record it anywhere
-        ip = request.META.get('REMOTE_ADDR')
-        if ip:
-            try:
-                geo_response = geo_reader.city(ip)
-                country_code = geo_response.country.iso_code
-            except (AddressNotFoundError, ValueError):  # Invalid IP
-                country_code = None
-
-    # Try to get a promo if we should be using one.
-    if show_promo:
-        promo_obj = get_promo(
-            country_code=country_code,
-            programming_language=project.programming_language,
-            theme=theme,
-            gold_project=gold_project,
-            gold_user=gold_user,
-        )
+        return None
+
+    promo_obj = get_promo(
+        country_code=get_user_country(request),
+        programming_language=project.programming_language,
+        theme=theme,
+        gold_project=gold_project,
+        gold_user=gold_user,
+    )
 
     # If we don't have anything to show, don't show it.
     if not promo_obj:
-        show_promo = False
-
-    if show_promo:
-        promo_dict = offer_promo(promo_obj=promo_obj, project=project)
-        resp_data['promo_data'] = promo_dict
+        return None
 
-    # Set promo object on return JSON
-    resp_data['promo'] = show_promo
+    return offer_promo(
+        promo_obj=promo_obj,
+        project=project
+    )
 
 
 @receiver(footer_response)
@@ -217,6 +238,7 @@ def index_theme_data(sender, **kwargs):
     This will allow us to give people fair warning before we put ads on their docs.
 
     """
+    del sender  # unused
     context = kwargs['context']
 
     project = context['project']

readthedocs/donate/urls.py

@@ -1,4 +1,6 @@
-from django.conf.urls import url, include
+"""Django URL patterns for the donate app."""
+
+from django.conf.urls import url
 
 from .views import DonateCreateView, DonateListView, DonateSuccessView
 from .views import PayAdsView, PaySuccess, PromoDetailView

readthedocs/donate/utils.py

@@ -1,3 +1,5 @@
+"""Support functions for donations."""
+
 import pytz
 import datetime
 

readthedocs/donate/views.py

@@ -1,4 +1,6 @@
 """Donation views"""
+# We use 'hash' heavily in the API here.
+# pylint: disable=redefined-builtin
 
 import logging
 
@@ -109,6 +111,7 @@ def get_context_data(self, **kwargs):
 
 
 def click_proxy(request, promo_id, hash):
+    """Track a click on a promotion and redirect to the link."""
     promo = get_object_or_404(SupporterPromo, pk=promo_id)
     count = cache.get(promo.cache_key(type=CLICKS, hash=hash), None)
     if count is None:
@@ -136,6 +139,7 @@ def click_proxy(request, promo_id, hash):
 
 
 def view_proxy(request, promo_id, hash):
+    """Track a view of a promotion and redirect to the image."""
     promo = get_object_or_404(SupporterPromo, pk=promo_id)
     if not promo.image:
         raise Http404('No image defined for this promo.')
@@ -174,7 +178,7 @@ def _add_promo_data(display_type):
     return promo_dict
 
 
-def promo_500(request, template_name='donate/promo_500.html', **kwargs):
+def promo_500(request, template_name='donate/promo_500.html', **__):
     """A simple 500 handler so we get media"""
     promo_dict = _add_promo_data(display_type='error')
     r = render_to_response(template_name,
@@ -186,7 +190,7 @@ def promo_500(request, template_name='donate/promo_500.html', **kwargs):
     return r
 
 
-def promo_404(request, template_name='donate/promo_404.html', **kwargs):
+def promo_404(request, template_name='donate/promo_404.html', **__):
     """A simple 404 handler so we get media"""
     promo_dict = _add_promo_data(display_type='error')
     response = get_redirect_response(request, path=request.get_full_path())