mirror of
https://github.com/jazzband/django-analytical.git
synced 2026-03-16 22:20:25 +00:00
Merge pull request #80 from bittner/feature/fix-code-smells
Fix some code smells reported by Landscape
This commit is contained in:
commit
6f35fbb774
9 changed files with 83 additions and 82 deletions
|
|
@ -6,9 +6,9 @@ install:
|
|||
- pip install coveralls tox
|
||||
script:
|
||||
- tox
|
||||
# NOTE: To generate (update) the env list run
|
||||
# $ tox -l | sort | xargs -I ITEM echo " - TOXENV="ITEM
|
||||
env:
|
||||
# NOTE: To generate (update) the env list run
|
||||
# $ tox -l | sort | xargs -I ITEM echo " - TOXENV="ITEM
|
||||
- TOXENV=py27-django17
|
||||
- TOXENV=py27-django18
|
||||
- TOXENV=py27-django19
|
||||
|
|
|
|||
|
|
@ -12,7 +12,10 @@ from analytical.utils import is_internal_ip, disable_html, get_required_setting
|
|||
|
||||
|
||||
ACCOUNT_NUMBER_RE = re.compile(r'^\d+$')
|
||||
SETUP_CODE = """<script type="text/javascript" src="//dnn506yrbagrg.cloudfront.net/pages/scripts/%(account_nr_1)s/%(account_nr_2)s.js"></script>"""
|
||||
SETUP_CODE = '<script type="text/javascript" src="{placeholder_url}">' \
|
||||
'</script>'.\
|
||||
format(placeholder_url='//dnn506yrbagrg.cloudfront.net/pages/scripts/'
|
||||
'%(account_nr_1)s/%(account_nr_2)s.js')
|
||||
USERVAR_CODE = "CE2.set(%(varnr)d, '%(value)s');"
|
||||
|
||||
|
||||
|
|
@ -36,19 +39,25 @@ def crazy_egg(parser, token):
|
|||
|
||||
class CrazyEggNode(Node):
|
||||
def __init__(self):
|
||||
self.account_nr = get_required_setting('CRAZY_EGG_ACCOUNT_NUMBER',
|
||||
ACCOUNT_NUMBER_RE, "must be (a string containing) a number")
|
||||
self.account_nr = get_required_setting(
|
||||
'CRAZY_EGG_ACCOUNT_NUMBER',
|
||||
ACCOUNT_NUMBER_RE, "must be (a string containing) a number"
|
||||
)
|
||||
|
||||
def render(self, context):
|
||||
html = SETUP_CODE % {'account_nr_1': self.account_nr[:4],
|
||||
'account_nr_2': self.account_nr[4:]}
|
||||
html = SETUP_CODE % {
|
||||
'account_nr_1': self.account_nr[:4],
|
||||
'account_nr_2': self.account_nr[4:],
|
||||
}
|
||||
values = (context.get('crazy_egg_var%d' % i) for i in range(1, 6))
|
||||
vars = [(i, v) for i, v in enumerate(values, 1) if v is not None]
|
||||
if vars:
|
||||
js = " ".join(USERVAR_CODE % {'varnr': varnr, 'value': value}
|
||||
for (varnr, value) in vars)
|
||||
html = '%s\n<script type="text/javascript">%s</script>' \
|
||||
% (html, js)
|
||||
params = [(i, v) for i, v in enumerate(values, 1) if v is not None]
|
||||
if params:
|
||||
js = " ".join(USERVAR_CODE % {
|
||||
'varnr': varnr,
|
||||
'value': value,
|
||||
} for (varnr, value) in params)
|
||||
html = '%s\n' \
|
||||
'<script type="text/javascript">%s</script>' % (html, js)
|
||||
if is_internal_ip(context, 'CRAZY_EGG'):
|
||||
html = disable_html(html, 'Crazy Egg')
|
||||
return html
|
||||
|
|
|
|||
|
|
@ -13,14 +13,6 @@ from analytical.utils import is_internal_ip, disable_html, \
|
|||
get_required_setting, get_domain, AnalyticalException
|
||||
|
||||
|
||||
def enumerate(sequence, start=0):
|
||||
"""Copy of the Python 2.6 `enumerate` builtin for compatibility."""
|
||||
n = start
|
||||
for elem in sequence:
|
||||
yield n, elem
|
||||
n += 1
|
||||
|
||||
|
||||
TRACK_SINGLE_DOMAIN = 1
|
||||
TRACK_MULTIPLE_SUBDOMAINS = 2
|
||||
TRACK_MULTIPLE_DOMAINS = 3
|
||||
|
|
@ -98,14 +90,15 @@ class GoogleAnalyticsNode(Node):
|
|||
def _get_domain_commands(self, context):
|
||||
commands = []
|
||||
tracking_type = getattr(settings, 'GOOGLE_ANALYTICS_TRACKING_STYLE',
|
||||
TRACK_SINGLE_DOMAIN)
|
||||
TRACK_SINGLE_DOMAIN)
|
||||
if tracking_type == TRACK_SINGLE_DOMAIN:
|
||||
pass
|
||||
else:
|
||||
domain = get_domain(context, 'google_analytics')
|
||||
if domain is None:
|
||||
raise AnalyticalException("tracking multiple domains with"
|
||||
" Google Analytics requires a domain name")
|
||||
raise AnalyticalException(
|
||||
"tracking multiple domains with Google Analytics"
|
||||
" requires a domain name")
|
||||
commands.append(DOMAIN_CODE % domain)
|
||||
commands.append(NO_ALLOW_HASH_CODE)
|
||||
if tracking_type == TRACK_MULTIPLE_DOMAINS:
|
||||
|
|
@ -113,11 +106,12 @@ class GoogleAnalyticsNode(Node):
|
|||
return commands
|
||||
|
||||
def _get_custom_var_commands(self, context):
|
||||
values = (context.get('google_analytics_var%s' % i)
|
||||
for i in range(1, 6))
|
||||
vars = [(i, v) for i, v in enumerate(values, 1) if v is not None]
|
||||
values = (
|
||||
context.get('google_analytics_var%s' % i) for i in range(1, 6)
|
||||
)
|
||||
params = [(i, v) for i, v in enumerate(values, 1) if v is not None]
|
||||
commands = []
|
||||
for index, var in vars:
|
||||
for index, var in params:
|
||||
name = var[0]
|
||||
value = var[1]
|
||||
try:
|
||||
|
|
|
|||
|
|
@ -6,10 +6,9 @@ from __future__ import absolute_import
|
|||
|
||||
import re
|
||||
|
||||
from django.conf import settings
|
||||
from django.template import Library, Node, TemplateSyntaxError
|
||||
|
||||
from analytical.utils import get_identity, get_user_from_context, \
|
||||
from analytical.utils import get_identity, \
|
||||
is_internal_ip, disable_html, get_required_setting
|
||||
|
||||
|
||||
|
|
@ -51,7 +50,8 @@ def gosquared(parser, token):
|
|||
|
||||
class GoSquaredNode(Node):
|
||||
def __init__(self):
|
||||
self.site_token = get_required_setting('GOSQUARED_SITE_TOKEN', TOKEN_RE,
|
||||
self.site_token = get_required_setting(
|
||||
'GOSQUARED_SITE_TOKEN', TOKEN_RE,
|
||||
"must be a string looking like XXX-XXXXXX-X")
|
||||
|
||||
def render(self, context):
|
||||
|
|
|
|||
|
|
@ -9,8 +9,8 @@ import re
|
|||
|
||||
from django.template import Library, Node, TemplateSyntaxError
|
||||
|
||||
from analytical.utils import disable_html, get_required_setting, is_internal_ip,\
|
||||
get_user_from_context, get_identity
|
||||
from analytical.utils import disable_html, get_required_setting, \
|
||||
is_internal_ip, get_user_from_context, get_identity
|
||||
|
||||
APP_ID_RE = re.compile(r'[\da-f]+$')
|
||||
TRACKING_CODE = """
|
||||
|
|
@ -51,33 +51,37 @@ class IntercomNode(Node):
|
|||
return name
|
||||
|
||||
def _get_custom_attrs(self, context):
|
||||
vars = {}
|
||||
params = {}
|
||||
for dict_ in context:
|
||||
for var, val in dict_.items():
|
||||
if var.startswith('intercom_'):
|
||||
vars[var[9:]] = val
|
||||
params[var[9:]] = val
|
||||
|
||||
user = get_user_from_context(context)
|
||||
if user is not None and user.is_authenticated():
|
||||
if 'name' not in vars:
|
||||
vars['name'] = get_identity(context, 'intercom', self._identify, user)
|
||||
if 'email' not in vars and user.email:
|
||||
vars['email'] = user.email
|
||||
if 'name' not in params:
|
||||
params['name'] = get_identity(
|
||||
context, 'intercom', self._identify, user)
|
||||
if 'email' not in params and user.email:
|
||||
params['email'] = user.email
|
||||
|
||||
vars['created_at'] = int(time.mktime(user.date_joined.timetuple()))
|
||||
params['created_at'] = int(time.mktime(
|
||||
user.date_joined.timetuple()))
|
||||
else:
|
||||
vars['created_at'] = None
|
||||
params['created_at'] = None
|
||||
|
||||
return vars
|
||||
return params
|
||||
|
||||
def render(self, context):
|
||||
html = ""
|
||||
user = get_user_from_context(context)
|
||||
vars = self._get_custom_attrs(context)
|
||||
vars["app_id"] = self.app_id
|
||||
html = TRACKING_CODE % {"settings_json": json.dumps(vars, sort_keys=True)}
|
||||
params = self._get_custom_attrs(context)
|
||||
params["app_id"] = self.app_id
|
||||
html = TRACKING_CODE % {
|
||||
"settings_json": json.dumps(params, sort_keys=True)
|
||||
}
|
||||
|
||||
if is_internal_ip(context, 'INTERCOM') or not user or not user.is_authenticated():
|
||||
if is_internal_ip(context, 'INTERCOM') \
|
||||
or not user or not user.is_authenticated():
|
||||
# Intercom is disabled for non-logged in users.
|
||||
html = disable_html(html, 'Intercom')
|
||||
return html
|
||||
|
|
|
|||
|
|
@ -52,8 +52,9 @@ def spring_metrics(parser, token):
|
|||
|
||||
class SpringMetricsNode(Node):
|
||||
def __init__(self):
|
||||
self.tracking_id = get_required_setting('SPRING_METRICS_TRACKING_ID',
|
||||
TRACKING_ID_RE, "must be a hexadecimal string")
|
||||
self.tracking_id = get_required_setting(
|
||||
'SPRING_METRICS_TRACKING_ID',
|
||||
TRACKING_ID_RE, "must be a hexadecimal string")
|
||||
|
||||
def render(self, context):
|
||||
custom = {}
|
||||
|
|
@ -63,23 +64,25 @@ class SpringMetricsNode(Node):
|
|||
custom[var[15:]] = val
|
||||
if 'email' not in custom:
|
||||
identity = get_identity(context, 'spring_metrics',
|
||||
lambda u: u.email)
|
||||
lambda u: u.email)
|
||||
if identity is not None:
|
||||
custom['email'] = identity
|
||||
|
||||
html = TRACKING_CODE % {'tracking_id': self.tracking_id,
|
||||
'custom_commands': self._generate_custom_javascript(custom)}
|
||||
html = TRACKING_CODE % {
|
||||
'tracking_id': self.tracking_id,
|
||||
'custom_commands': self._generate_custom_javascript(custom),
|
||||
}
|
||||
if is_internal_ip(context, 'SPRING_METRICS'):
|
||||
html = disable_html(html, 'Spring Metrics')
|
||||
return html
|
||||
|
||||
def _generate_custom_javascript(self, vars):
|
||||
def _generate_custom_javascript(self, params):
|
||||
commands = []
|
||||
convert = vars.pop('convert', None)
|
||||
convert = params.pop('convert', None)
|
||||
if convert is not None:
|
||||
commands.append("_springMetq.push(['convert', '%s'])" % convert)
|
||||
commands.extend("_springMetq.push(['setdata', {'%s': '%s'}]);"
|
||||
% (var, val) for var, val in vars.items())
|
||||
% (var, val) for var, val in params.items())
|
||||
return " ".join(commands)
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -50,7 +50,8 @@ def woopra(parser, token):
|
|||
|
||||
class WoopraNode(Node):
|
||||
def __init__(self):
|
||||
self.domain = get_required_setting('WOOPRA_DOMAIN', DOMAIN_RE,
|
||||
self.domain = get_required_setting(
|
||||
'WOOPRA_DOMAIN', DOMAIN_RE,
|
||||
"must be a domain name")
|
||||
|
||||
def render(self, context):
|
||||
|
|
@ -74,19 +75,19 @@ class WoopraNode(Node):
|
|||
return vars
|
||||
|
||||
def _get_visitor(self, context):
|
||||
vars = {}
|
||||
params = {}
|
||||
for dict_ in context:
|
||||
for var, val in dict_.items():
|
||||
if var.startswith('woopra_'):
|
||||
vars[var[7:]] = val
|
||||
if 'name' not in vars and 'email' not in vars:
|
||||
params[var[7:]] = val
|
||||
if 'name' not in params and 'email' not in params:
|
||||
user = get_user_from_context(context)
|
||||
if user is not None and user.is_authenticated():
|
||||
vars['name'] = get_identity(context, 'woopra',
|
||||
self._identify, user)
|
||||
params['name'] = get_identity(
|
||||
context, 'woopra', self._identify, user)
|
||||
if user.email:
|
||||
vars['email'] = user.email
|
||||
return vars
|
||||
params['email'] = user.email
|
||||
return params
|
||||
|
||||
def _identify(self, user):
|
||||
name = user.get_full_name()
|
||||
|
|
|
|||
|
|
@ -1,29 +1,23 @@
|
|||
"""
|
||||
Tests for the analytical.utils module.
|
||||
"""
|
||||
import django
|
||||
# import django
|
||||
|
||||
from django.conf import settings
|
||||
from django.contrib.auth.models import AbstractBaseUser
|
||||
from django.db import models
|
||||
from django.http import HttpRequest
|
||||
from django.template import Context
|
||||
from django.test.utils import override_settings
|
||||
|
||||
from analytical.utils import (
|
||||
get_domain, get_identity, is_internal_ip, get_required_setting,
|
||||
AnalyticalException)
|
||||
AnalyticalException,
|
||||
get_domain,
|
||||
get_identity,
|
||||
get_required_setting,
|
||||
is_internal_ip,
|
||||
)
|
||||
from analytical.tests.utils import TestCase
|
||||
|
||||
try:
|
||||
from unittest import skipIf
|
||||
except ImportError: # Python 2.6 fallback
|
||||
from unittest2 import skipIf
|
||||
|
||||
try:
|
||||
from django.contrib.auth.models import AbstractBaseUser
|
||||
except ImportError: # Django < 1.5 fallback
|
||||
AbstractBaseUser = models.Model
|
||||
|
||||
|
||||
class SettingDeletedTestCase(TestCase):
|
||||
|
||||
|
|
@ -52,7 +46,6 @@ class MyUser(AbstractBaseUser):
|
|||
|
||||
|
||||
class GetIdentityTestCase(TestCase):
|
||||
@skipIf(django.VERSION < (1, 5,), 'Custom usernames not supported in Django < 1.5')
|
||||
def test_custom_username_field(self):
|
||||
get_id = get_identity(Context({}), user=MyUser(identity='fake_id'))
|
||||
self.assertEqual(get_id, 'fake_id')
|
||||
|
|
|
|||
|
|
@ -75,10 +75,7 @@ def get_identity(context, prefix=None, identity_func=None, user=None):
|
|||
if identity_func is not None:
|
||||
return identity_func(user)
|
||||
else:
|
||||
try:
|
||||
return user.get_username()
|
||||
except AttributeError: # Django < 1.5 fallback
|
||||
return user.username
|
||||
return user.get_username()
|
||||
except (KeyError, AttributeError):
|
||||
pass
|
||||
return None
|
||||
|
|
|
|||
Loading…
Reference in a new issue