From 4782d54d1fb8adfb5538e72e999e74cee38b10a8 Mon Sep 17 00:00:00 2001 From: Tim Glaser Date: Tue, 14 Apr 2020 11:05:45 +0100 Subject: [PATCH] Configure mypy (#562) * server/mypy: Enable no_implicit_optional no_implicit_optional Changes the treatment of arguments with a default value of None by not implicitly making their type Optional. Ref: https://mypy.readthedocs.io/en/stable/config_file.html#none-and-optional-handling * server/mypy: Enable warn_unused_ignores warn_unused_ignores: Warns about unneeded # type: ignore comments. Ref: https://mypy.readthedocs.io/en/stable/config_file.html#configuring-warnings It's best to exclude packages with no typing in mypy.ini rather than in the code. This waym if tin the future these packages add types it's can easily be disabled (by removing the exclusion in mypy.ini). * api/user: Fix user password was not really validated * api/test/base: Remove unreachable code Team is created in setup, so no reason for it not to be there (unless setUp is overriden by child, which as far as I can see is currently not happening), * server/mypy: Enable check_untyped_defs check_untyped_defs: Type-checks the interior of functions without type annotations. Ref: https://mypy.readthedocs.io/en/stable/config_file.html#untyped-definitions-and-calls * server/mypy: Enable strict_equality strict_equality: Prohibit equality checks, identity checks, and container checks between non-overlapping types. Ref: https://mypy.readthedocs.io/en/stable/config_file.html#miscellaneous-strictness-flags * server/mypy: Enable warn_unreachable Added a fixme for a possible oversight in function that parses JSON. warn_unreachable: Shows a warning when encountering any code inferred to be unreachable or redundant after performing type analysis. Ref: https://mypy.readthedocs.io/en/stable/config_file.html#configuring-warnings * api/posthog: fix possible bug when batch is not a list Current code assumes batch is a valid list. If batch is not a list, the capture handling will cause a 500 error. * fix stricter mypy * Use response.json instead of .data Co-authored-by: Haki Benita --- mypy.ini | 29 +++ package.json | 170 +++++++++--------- posthog/api/action.py | 26 +-- posthog/api/capture.py | 18 +- posthog/api/cohort.py | 4 +- posthog/api/dashboard.py | 4 +- posthog/api/event.py | 4 +- posthog/api/funnel.py | 6 +- posthog/api/paths.py | 10 +- posthog/api/test/base.py | 2 - posthog/api/test/test_action.py | 6 +- posthog/api/test/test_capture.py | 12 +- posthog/api/test/test_event.py | 4 +- posthog/api/user.py | 38 ++-- posthog/apps.py | 2 +- .../migrations/0027_move_elements_to_group.py | 6 +- posthog/models.py | 32 +++- posthog/test/test_event_model.py | 4 +- posthog/test/test_migration_0027.py | 38 ++-- posthog/test/test_migration_0039.py | 35 ++-- posthog/test/test_signup.py | 6 +- posthog/test/test_utils.py | 2 +- posthog/urls.py | 9 +- posthog/views.py | 5 +- 24 files changed, 269 insertions(+), 203 deletions(-) diff --git a/mypy.ini b/mypy.ini index 22186a66ca8..4ce8936b703 100644 --- a/mypy.ini +++ b/mypy.ini @@ -3,6 +3,11 @@ plugins = mypy_django_plugin.main, mypy_drf_plugin.main strict_optional = True +no_implicit_optional = True +warn_unused_ignores = True +check_untyped_defs = True +warn_unreachable = True +strict_equality = True [mypy.plugins.django-stubs] django_settings_module = posthog.settings @@ -15,3 +20,27 @@ ignore_missing_imports = True [mypy-celery.*] ignore_missing_imports = True + +[mypy-social_django.strategy] +ignore_missing_imports = True + +[mypy-social_core.utils] +ignore_missing_imports = True + +[mypy-posthoganalytics] +ignore_missing_imports = True + +[mypy-drf_yasg] +ignore_missing_imports = True + +[mypy-drf_yasg.views] +ignore_missing_imports = True + +[mypy-pandas] +ignore_missing_imports = True + +[mypy-numpy] +ignore_missing_imports = True + +[mypy-freezegun] +ignore_missing_imports = True diff --git a/package.json b/package.json index 94612795bf0..51ee71937a8 100644 --- a/package.json +++ b/package.json @@ -1,88 +1,88 @@ { - "name": "posthog", - "description": "", - "version": "0.0.0", - "repository": { - "type": "git", - "url": "https://github.com/posthog/posthog.git" - }, - "author": "", - "bugs": { - "url": "https://github.com/posthog/posthog/issues" - }, - "homepage": "https://github.com/posthog/posthog#readme", - "scripts": { - "copy-array": "cp node_modules/posthog-js/dist/array.js frontend/dist/", - "start": "mkdir -p frontend/dist/ && cp -a frontend/public/* frontend/dist/ && npm run copy-array && webpack --config webpack.config.js --watch", - "build": "NODE_ENV=production webpack --config webpack.config.js && cp -a frontend/public/* frontend/dist/ && npm run copy-array", - "prettier": "prettier --write \"./frontend/src/**/*.{js,css,scss}\"" - }, - "license": "MIT", - "dependencies": { - "@babel/core": "^7.8.7", - "@babel/runtime": "^7.8.7", - "antd": "^4.1.1", - "babel-preset-nano-react-app": "^0.1.0", - "bootstrap": "^4.4.1", - "chart.js": "^2.9.3", - "core-js": "3", - "d3": "^5.15.0", - "d3-sankey": "^0.12.3", - "editor": "^1.0.0", - "funnel-graph-js": "^1.4.1", - "kea": "^1.0.1", - "kea-listeners": "^0.2.3", - "kea-loaders": "^0.0.4", - "kea-router": "^0.1.2", - "moment": "^2.24.0", - "posthog-js": "1.0.7-beta.0", - "prop-types": "^15.7.2", - "react": ">= 16.8", - "react-beautiful-dnd": "^12.2.0", - "react-datepicker": "^2.13.0", - "react-dnd": "^10.0.2", - "react-dom": ">= 16.8", - "react-draggable": "^4.2.0", - "react-redux": "^7.2.0", - "react-router-dom": "^5.1.0", - "react-select": "^3.0.8", - "react-shadow": "^17.4.0", - "react-stripe-elements": "^6.0.1", - "react-toastify": "^5.5.0", - "redux": "^4.0.5", - "reselect": "^4.0.0", - "sass": "^1.26.2", - "simmerjs": "^0.5.6", - "style.css": "^1.0.0", - "styled-components": "^5.0.1" - }, - "devDependencies": { - "@babel/plugin-proposal-class-properties": "^7.8.3", - "@babel/plugin-transform-react-jsx": "^7.8.3", - "@babel/plugin-transform-runtime": "^7.8.3", - "@babel/preset-env": "^7.8.7", - "autoprefixer": "^9.7.4", - "babel-loader": "^8.0.6", - "babel-plugin-import": "^1.13.0", - "css-loader": "^3.4.2", - "cssnano": "^4.1.10", - "file-loader": "^5.1.0", - "husky": ">=4", - "lint-staged": ">=10", - "mini-css-extract-plugin": "^0.9.0", - "parcel-bundler": "1.11.0", - "postcss-loader": "^3.0.0", - "prettier": "1.19.1", - "sass-loader": "^8.0.2", - "webpack": "^4.42.0", - "webpack-cli": "^3.3.11" - }, - "husky": { - "hooks": { - "pre-commit": "lint-staged" + "name": "posthog", + "description": "", + "version": "0.0.0", + "repository": { + "type": "git", + "url": "https://github.com/posthog/posthog.git" + }, + "author": "", + "bugs": { + "url": "https://github.com/posthog/posthog/issues" + }, + "homepage": "https://github.com/posthog/posthog#readme", + "scripts": { + "copy-array": "cp node_modules/posthog-js/dist/array.js frontend/dist/", + "start": "mkdir -p frontend/dist/ && cp -a frontend/public/* frontend/dist/ && npm run copy-array && webpack --config webpack.config.js --watch", + "build": "NODE_ENV=production webpack --config webpack.config.js && cp -a frontend/public/* frontend/dist/ && npm run copy-array", + "prettier": "prettier --write \"./frontend/src/**/*.{js,css,scss}\"" + }, + "license": "MIT", + "dependencies": { + "@babel/core": "^7.8.7", + "@babel/runtime": "^7.8.7", + "antd": "^4.1.1", + "babel-preset-nano-react-app": "^0.1.0", + "bootstrap": "^4.4.1", + "chart.js": "^2.9.3", + "core-js": "3", + "d3": "^5.15.0", + "d3-sankey": "^0.12.3", + "editor": "^1.0.0", + "funnel-graph-js": "^1.4.1", + "kea": "^1.0.1", + "kea-listeners": "^0.2.3", + "kea-loaders": "^0.0.4", + "kea-router": "^0.1.2", + "moment": "^2.24.0", + "posthog-js": "1.0.7-beta.0", + "prop-types": "^15.7.2", + "react": ">= 16.8", + "react-beautiful-dnd": "^12.2.0", + "react-datepicker": "^2.13.0", + "react-dnd": "^10.0.2", + "react-dom": ">= 16.8", + "react-draggable": "^4.2.0", + "react-redux": "^7.2.0", + "react-router-dom": "^5.1.0", + "react-select": "^3.0.8", + "react-shadow": "^17.4.0", + "react-stripe-elements": "^6.0.1", + "react-toastify": "^5.5.0", + "redux": "^4.0.5", + "reselect": "^4.0.0", + "sass": "^1.26.2", + "simmerjs": "^0.5.6", + "style.css": "^1.0.0", + "styled-components": "^5.0.1" + }, + "devDependencies": { + "@babel/plugin-proposal-class-properties": "^7.8.3", + "@babel/plugin-transform-react-jsx": "^7.8.3", + "@babel/plugin-transform-runtime": "^7.8.3", + "@babel/preset-env": "^7.8.7", + "autoprefixer": "^9.7.4", + "babel-loader": "^8.0.6", + "babel-plugin-import": "^1.13.0", + "css-loader": "^3.4.2", + "cssnano": "^4.1.10", + "file-loader": "^5.1.0", + "husky": ">=4", + "lint-staged": ">=10", + "mini-css-extract-plugin": "^0.9.0", + "parcel-bundler": "1.11.0", + "postcss-loader": "^3.0.0", + "prettier": "1.19.1", + "sass-loader": "^8.0.2", + "webpack": "^4.42.0", + "webpack-cli": "^3.3.11" + }, + "husky": { + "hooks": { + "pre-commit": "lint-staged" + } + }, + "lint-staged": { + "!*.{js,css,scss}": "prettier --write" } - }, - "lint-staged": { - "!(*array).{js,css,scss}": "prettier --write" - } } diff --git a/posthog/api/action.py b/posthog/api/action.py index 854b83ce63c..91fbcc35521 100644 --- a/posthog/api/action.py +++ b/posthog/api/action.py @@ -1,10 +1,10 @@ from posthog.models import Event, Team, Action, ActionStep, Element, User, Person from posthog.utils import relative_date_parse, properties_to_Q from posthog.constants import TREND_FILTER_TYPE_ACTIONS, TREND_FILTER_TYPE_EVENTS -from rest_framework import request, serializers, viewsets, authentication # type: ignore +from rest_framework import request, serializers, viewsets, authentication from rest_framework.response import Response -from rest_framework.decorators import action # type: ignore +from rest_framework.decorators import action from rest_framework.exceptions import AuthenticationFailed from rest_framework.utils.serializer_helpers import ReturnDict from django.db.models import Q, F, Count, Prefetch, functions, QuerySet, TextField @@ -14,8 +14,8 @@ from django.forms.models import model_to_dict from django.utils.decorators import method_decorator from django.utils.dateparse import parse_date from typing import Any, List, Dict, Optional, Tuple -import pandas as pd # type: ignore -import numpy as np # type: ignore +import pandas as pd +import numpy as np import datetime import json from dateutil.relativedelta import relativedelta @@ -76,7 +76,7 @@ class ActionViewSet(viewsets.ModelViewSet): def get_queryset(self): queryset = super().get_queryset() - if self.action == 'list': + if self.action == 'list': # type: ignore queryset = queryset.filter(deleted=False) if self.request.GET.get(TREND_FILTER_TYPE_ACTIONS): @@ -226,12 +226,12 @@ class ActionViewSet(viewsets.ModelViewSet): .order_by('-count') events = self._process_math(events, filters) - + values = [{'name': item[key] if item[key] else 'undefined', 'count': item['count']} for item in events] append['breakdown'] = values append['count'] = sum(item['count'] for item in values) return append - + def _append_data(self, append: Dict, dates_filled: pd.DataFrame, interval: str) -> Dict: append['data'] = [] append['labels'] = [] @@ -251,7 +251,7 @@ class ActionViewSet(viewsets.ModelViewSet): append['count'] = sum(append['data']) return append - + def _get_interval_annotation(self, key: str) -> Dict[str, Any]: map: Dict[str, Any] = { 'minute': functions.TruncMinute('timestamp'), @@ -263,7 +263,7 @@ class ActionViewSet(viewsets.ModelViewSet): func = map.get(key) if func is None: return {'day': map.get('day')} # default - + return { key: func } def _aggregate_by_interval(self, filtered_events: QuerySet, filters: Dict[Any, Any], request: request.Request, interval: str) -> Dict[str, Any]: @@ -392,8 +392,8 @@ class ActionViewSet(viewsets.ModelViewSet): filters=event, request=request, ) - if trend_entity is not None and 'labels' in trend_entity: - actions_list.append(trend_entity) + if 'labels' in trend_entity: + actions_list.append(trend_entity) if parsed_actions: for filters in parsed_actions: try: @@ -464,5 +464,5 @@ class ActionViewSet(viewsets.ModelViewSet): filtered_events = self._process_entity_for_events(action, entity_type=TREND_FILTER_TYPE_ACTIONS, order_by=None).filter(self._filter_events(request)) people = _calculate_people(id=action.id, name=action.name, events=filtered_events) return Response([people]) - - return Response([]) \ No newline at end of file + + return Response([]) diff --git a/posthog/api/capture.py b/posthog/api/capture.py index 3616eb75ee3..00b474f46e7 100644 --- a/posthog/api/capture.py +++ b/posthog/api/capture.py @@ -30,7 +30,7 @@ def cors_response(request, response): response["Access-Control-Allow-Headers"] = 'X-Requested-With' return response -def _load_data(request) -> Union[Dict, None]: +def _load_data(request) -> Optional[Union[Dict, List]]: if request.method == 'POST': if request.content_type == 'application/json': data = request.body @@ -40,13 +40,14 @@ def _load_data(request) -> Union[Dict, None]: data = request.GET.get('data') if not data: return None - + # Is it plain json? try: data = json.loads(data) except json.JSONDecodeError: # if not, it's probably base64 encoded from other libraries data = json.loads(base64.b64decode(data + "===").decode('utf8', 'surrogatepass').encode('utf-16', 'surrogatepass')) + # FIXME: data can also be an array, function assumes it's either None or a dictionary. return data def _alias(distinct_id: str, new_distinct_id: str, team: Team): @@ -175,17 +176,18 @@ def get_event(request): if not token: return cors_response(request, JsonResponse({'code': 'validation', 'message': "No api_key set. You can find your API key in the /setup page in posthog"}, status=400)) - if not isinstance(data, list) and data.get('batch'): # posthog-python and posthog-ruby - data = data['batch'] - - if 'engage' in request.path_info: # JS identify call - data['event'] = '$identify' # make sure it has an event name - try: team = Team.objects.get(api_token=token) except Team.DoesNotExist: return cors_response(request, JsonResponse({'code': 'validation', 'message': "API key is incorrect. You can find your API key in the /setup page in PostHog."}, status=400)) + if isinstance(data, dict): + if data.get('batch'): # posthog-python and posthog-ruby + data = data['batch'] + assert data is not None + elif 'engage' in request.path_info: # JS identify call + data['event'] = '$identify' # make sure it has an event name + if isinstance(data, list): for i in data: try: diff --git a/posthog/api/cohort.py b/posthog/api/cohort.py index b61fadb60a0..0bab7006d1b 100644 --- a/posthog/api/cohort.py +++ b/posthog/api/cohort.py @@ -1,4 +1,4 @@ -from rest_framework import request, response, serializers, viewsets # type: ignore +from rest_framework import request, response, serializers, viewsets from posthog.models import Cohort from typing import Dict, Any from django.db.models import QuerySet @@ -19,7 +19,7 @@ class CohortViewSet(viewsets.ModelViewSet): def get_queryset(self) -> QuerySet: queryset = super().get_queryset() - if self.action == 'list': # type: ignore + if self.action == 'list': # type: ignore queryset = queryset.filter(deleted=False) return queryset\ .filter(team=self.request.user.team_set.get())\ diff --git a/posthog/api/dashboard.py b/posthog/api/dashboard.py index 01f18c881a7..71d02949e42 100644 --- a/posthog/api/dashboard.py +++ b/posthog/api/dashboard.py @@ -1,4 +1,4 @@ -from rest_framework import request, response, serializers, viewsets # type: ignore +from rest_framework import request, response, serializers, viewsets from posthog.models import DashboardItem from typing import Dict, Any from django.db.models import QuerySet @@ -20,7 +20,7 @@ class DashboardViewSet(viewsets.ModelViewSet): def get_queryset(self) -> QuerySet: queryset = super().get_queryset() - if self.action == 'list': # type: ignore + if self.action == 'list': # type: ignore queryset = queryset.filter(deleted=False) return queryset\ .filter(team=self.request.user.team_set.get())\ diff --git a/posthog/api/event.py b/posthog/api/event.py index 02565bb14a2..c2a1640279a 100644 --- a/posthog/api/event.py +++ b/posthog/api/event.py @@ -1,7 +1,7 @@ from posthog.models import Event, Team, Person, Element, Action, PersonDistinctId, ElementGroup from posthog.utils import properties_to_Q -from rest_framework import request, response, serializers, viewsets # type: ignore -from rest_framework.decorators import action # type: ignore +from rest_framework import request, response, serializers, viewsets +from rest_framework.decorators import action from django.http import HttpResponse, JsonResponse from django.db.models import Q, Count, QuerySet, query, F, Func, functions, Prefetch from django.forms.models import model_to_dict diff --git a/posthog/api/funnel.py b/posthog/api/funnel.py index c02116ba31e..20787cf40e3 100644 --- a/posthog/api/funnel.py +++ b/posthog/api/funnel.py @@ -1,6 +1,6 @@ from posthog.models import FunnelStep, Action, ActionStep, Event, Funnel, Person, PersonDistinctId -from rest_framework import request, response, serializers, viewsets # type: ignore -from rest_framework.decorators import action # type: ignore +from rest_framework import request, response, serializers, viewsets +from rest_framework.decorators import action from django.db.models import QuerySet, query, Model, Q, Max, Prefetch, Exists, OuterRef, Subquery from django.db import models from typing import List, Dict, Any, Optional @@ -29,7 +29,7 @@ class FunnelSerializer(serializers.HyperlinkedModelSerializer): annotations = {} for index, step in enumerate(funnel_steps): annotations['step_{}'.format(index)] = Subquery( - Event.objects.filter_by_action(step.action) # type: ignore + Event.objects.filter_by_action(step.action) .annotate(person_id=OuterRef('id')) .filter( team_id=team_id, diff --git a/posthog/api/paths.py b/posthog/api/paths.py index bb41cb18fdb..621a96f741d 100644 --- a/posthog/api/paths.py +++ b/posthog/api/paths.py @@ -1,3 +1,4 @@ +from __future__ import annotations from rest_framework import viewsets from rest_framework.response import Response from posthog.models import Event, PersonDistinctId, Team @@ -14,8 +15,11 @@ class PathsViewSet(viewsets.ViewSet): def _event_subquery(self, event: str, key: str): return Event.objects.filter(pk=OuterRef(event)).values(key)[:1] + # FIXME: Timestamp is timezone aware timestamp, date range uses naive date. + # To avoid unexpected results should convert date range to timestamps with timezone. def _add_event_and_url_at_position(self, aggregate: QuerySet, team: Team, index: int, date_query: Dict[str, datetime.date], urls: Optional[List[str]]=None) -> QuerySet: event_key = 'event_{}'.format(index) + # adds event_1, url_1, event_2, url_2 etc for each Person return aggregate.annotate(**{ event_key: Subquery( @@ -38,10 +42,10 @@ class PathsViewSet(viewsets.ViewSet): team = request.user.team_set.get() resp = [] date_query = request_to_date_query(request) - aggregate = PersonDistinctId.objects.filter(team=team) + aggregate: QuerySet[PersonDistinctId] = PersonDistinctId.objects.filter(team=team) aggregate = self._add_event_and_url_at_position(aggregate, team, 1, date_query) - urls = False + urls: List[str] = [] for index in range(1, 4): aggregate = self._add_event_and_url_at_position(aggregate, team, index+1, date_query) @@ -69,4 +73,4 @@ class PathsViewSet(viewsets.ViewSet): urls.append(row[second_url_key]) resp = sorted(resp, key=lambda x: x['value'], reverse=True) - return Response(resp) \ No newline at end of file + return Response(resp) diff --git a/posthog/api/test/base.py b/posthog/api/test/base.py index 198179c6e36..51f3887c6cd 100644 --- a/posthog/api/test/base.py +++ b/posthog/api/test/base.py @@ -9,8 +9,6 @@ class BaseTest(TestCase): def _create_user(self, email, **kwargs) -> User: user = User.objects.create_user(email, **kwargs) - if not hasattr(self, 'team'): - self.team: Team = Team.objects.create(api_token='token123') self.team.users.add(user) self.team.save() return user diff --git a/posthog/api/test/test_action.py b/posthog/api/test/test_action.py index 8d6a0e4f5c8..35bdbc98dd9 100644 --- a/posthog/api/test/test_action.py +++ b/posthog/api/test/test_action.py @@ -1,6 +1,6 @@ from .base import BaseTest from posthog.models import Action, ActionStep, Event, Element, Person -from freezegun import freeze_time # type: ignore +from freezegun import freeze_time from urllib import parse import json @@ -70,6 +70,8 @@ class TestCreateAction(BaseTest): # otherwise evil sites could create actions with a users' session. # NOTE: Origin header is only set on cross domain request def test_create_from_other_domain(self): + # FIXME: BaseTest is using Django client to performe calls to a DRF endpoint. + # Django HttpResponse does not have an attribute `data`. Better use rest_framework.test.APIClient. response = self.client.post('/api/action/', data={ 'name': 'user signed up', }, content_type='application/json', HTTP_ORIGIN='https://evilwebsite.com') @@ -88,7 +90,7 @@ class TestCreateAction(BaseTest): 'post_to_slack': True }, content_type='application/json', HTTP_ORIGIN='https://somewebsite.com') self.assertEqual(response.status_code, 200) - self.assertEqual(response.data['post_to_slack'], True) + self.assertEqual(response.json()['post_to_slack'], True) list_response = self.client.get('/api/action/', content_type='application/json', HTTP_ORIGIN='https://evilwebsite.com') self.assertEqual(list_response.status_code, 403) diff --git a/posthog/api/test/test_capture.py b/posthog/api/test/test_capture.py index e1e82bf17a5..62048d5ffdc 100644 --- a/posthog/api/test/test_capture.py +++ b/posthog/api/test/test_capture.py @@ -1,7 +1,7 @@ from .base import BaseTest from posthog.models import Event, Person, Team, User, ElementGroup, Action, ActionStep from django.test import TransactionTestCase -from freezegun import freeze_time # type: ignore +from freezegun import freeze_time import base64 import json import datetime @@ -37,7 +37,7 @@ class TestCapture(BaseTest): }, }), content_type='application/json', HTTP_ORIGIN='https://localhost') - self.assertEqual(response._headers['access-control-allow-origin'][1], 'https://localhost') + self.assertEqual(response.get('access-control-allow-origin'), 'https://localhost') self.assertEqual(Person.objects.get().distinct_ids, ["2"]) event = Event.objects.get() self.assertEqual(event.event, '$autocapture') @@ -84,7 +84,7 @@ class TestCapture(BaseTest): '$device_id': '16fd4afae9b2d8-0fce8fe900d42b-39637c0e-7e9000-16fd4afae9c395', '$user_id': 3 }), content_type='application/json', HTTP_ORIGIN='https://localhost') - self.assertEqual(response._headers['access-control-allow-origin'][1], 'https://localhost') + self.assertEqual(response.get('access-control-allow-origin'), 'https://localhost') person = Person.objects.get() self.assertEqual(person.properties['whatever'], 'this is') @@ -136,7 +136,9 @@ class TestCapture(BaseTest): self.assertEqual(Person.objects.get().distinct_ids, ["63"]) event = Event.objects.get() - self.assertEqual(ElementGroup.objects.get(hash=event.elements_hash).element_set.all().first().text, '💻 Writing code') + element = ElementGroup.objects.get(hash=event.elements_hash).element_set.all().first() + assert element is not None + self.assertEqual(element.text, '💻 Writing code') def test_incorrect_padding(self): response = self.client.get('/e/?data=eyJldmVudCI6IndoYXRldmVmciIsInByb3BlcnRpZXMiOnsidG9rZW4iOiJ0b2tlbjEyMyIsImRpc3RpbmN0X2lkIjoiYXNkZiJ9fQ', content_type='application/json', HTTP_REFERER='https://localhost') @@ -179,7 +181,7 @@ class TestCapture(BaseTest): class TestIdentify(TransactionTestCase): def _create_user(self, email, **kwargs) -> User: - user: User = User.objects.create_user(email, **kwargs) # type: ignore + user: User = User.objects.create_user(email, **kwargs) if not hasattr(self, 'team'): self.team: Team = Team.objects.create(api_token='token123') self.team.users.add(user) diff --git a/posthog/api/test/test_event.py b/posthog/api/test/test_event.py index c25912a6824..216ef8bc338 100644 --- a/posthog/api/test/test_event.py +++ b/posthog/api/test/test_event.py @@ -1,10 +1,10 @@ from .base import BaseTest from posthog.models import Event, Person, Element, Action, ActionStep -from freezegun import freeze_time # type: ignore +from freezegun import freeze_time class TestEvents(BaseTest): - TESTS_API = True + TESTS_API = True ENDPOINT = 'event' def test_filter_events(self): diff --git a/posthog/api/user.py b/posthog/api/user.py index 33522f45935..fea3590cbe1 100644 --- a/posthog/api/user.py +++ b/posthog/api/user.py @@ -11,7 +11,7 @@ import requests import urllib.parse import secrets import json -import posthoganalytics # type: ignore +import posthoganalytics def user(request): if not request.user.is_authenticated: @@ -94,7 +94,7 @@ def change_password(request): return JsonResponse({'error': 'Incorrect old password'}, status=400) try: - validate_password(new_password, user) + validate_password(new_password, request.user) except ValidationError as err: return JsonResponse({'error': err.messages[0]}, status=400) @@ -118,24 +118,20 @@ def test_slack_webhook(request): webhook = body.get('webhook') - if webhook: - message = { - "text": "Greetings from PostHog!" - } - try: - response = requests.post(webhook, verify=False, json=message) - - if response.ok: - if response.text == 'ok': - return JsonResponse({'success': True}) - else: - return JsonResponse({'error': 'invalid webhook url'}) - else: - return JsonResponse({'error': response.text}) - except: - return JsonResponse({'error': 'invalid webhook url'}) - - else: + if not webhook: return JsonResponse({'error': 'no webhook'}) + message = { + "text": "Greetings from PostHog!" + } + try: + response = requests.post(webhook, verify=False, json=message) - return JsonResponse({}) + if response.ok: + if response.text == 'ok': + return JsonResponse({'success': True}) + else: + return JsonResponse({'error': 'invalid webhook url'}) + else: + return JsonResponse({'error': response.text}) + except: + return JsonResponse({'error': 'invalid webhook url'}) \ No newline at end of file diff --git a/posthog/apps.py b/posthog/apps.py index a9fe0a9597d..19f6dbff2a4 100644 --- a/posthog/apps.py +++ b/posthog/apps.py @@ -1,6 +1,6 @@ from django.apps import AppConfig from django.conf import settings -import posthoganalytics # type: ignore +import posthoganalytics class PostHogConfig(AppConfig): diff --git a/posthog/migrations/0027_move_elements_to_group.py b/posthog/migrations/0027_move_elements_to_group.py index 162411300a3..58a5dbe4bbb 100644 --- a/posthog/migrations/0027_move_elements_to_group.py +++ b/posthog/migrations/0027_move_elements_to_group.py @@ -1,4 +1,6 @@ # Generated by Django 3.0.3 on 2020-02-27 18:13 +from __future__ import annotations +from typing import List from django.db import migrations, transaction, models @@ -6,7 +8,7 @@ from django.forms.models import model_to_dict import json import hashlib -def hash_elements(elements): +def hash_elements(elements) -> str: elements_list = [] for index, element in enumerate(elements): el_dict = model_to_dict(element) @@ -20,7 +22,7 @@ def forwards(apps, schema_editor): ElementGroup = apps.get_model('posthog', 'ElementGroup') Element = apps.get_model('posthog', 'Element') - hashes_seen = [] + hashes_seen: List[str] = [] while Event.objects.filter(element__isnull=False, elements_hash__isnull=True, event='$autocapture').exists(): with transaction.atomic(): events = Event.objects.filter(element__isnull=False, elements_hash__isnull=True, event='$autocapture')\ diff --git a/posthog/models.py b/posthog/models.py index ce3f006af04..c8ab8a823ba 100644 --- a/posthog/models.py +++ b/posthog/models.py @@ -53,21 +53,40 @@ def split_selector_into_parts(selector: str) -> List: ret.append(data) return ret + +def is_email_restricted_from_signup(email: str) -> bool: + if not hasattr(settings, 'RESTRICT_SIGNUPS'): + return False + + restricted_signups: Union[str, bool] = settings.RESTRICT_SIGNUPS + if restricted_signups is False: + return False + + domain = email.rsplit('@', 1)[1] + whitelisted_domains = str(settings.RESTRICT_SIGNUPS).split(',') + if domain in whitelisted_domains: + return False + + return True + + class UserManager(BaseUserManager): """Define a model manager for User model with no username field.""" use_in_migrations = True - def _create_user(self, email, password, **extra_fields): + def _create_user(self, email: Optional[str], password: str, **extra_fields): """Create and save a User with the given email and password.""" - if not email: + if email is None: raise ValueError('The given email must be set') + email = self.normalize_email(email) - if hasattr(settings, 'RESTRICT_SIGNUPS') and settings.RESTRICT_SIGNUPS and email.rsplit('@', 1)[1] not in settings.RESTRICT_SIGNUPS.split(','): + if is_email_restricted_from_signup(email): raise ValueError("Can't sign up with this email") + user = self.model(email=email, **extra_fields) user.set_password(password) - user.save(using=self._db) + user.save() return user def create_user(self, email, password=None, **extra_fields): @@ -104,7 +123,8 @@ class User(AbstractUser): class TeamManager(models.Manager): - def create_with_data(self, users: List[User]=None, **kwargs): + + def create_with_data(self, users: Optional[List[User]], **kwargs): kwargs['api_token'] = kwargs.get('api_token', secrets.token_urlsafe(32)) kwargs['signup_token'] = kwargs.get('signup_token', secrets.token_urlsafe(22)) team = Team.objects.create(**kwargs) @@ -160,7 +180,7 @@ class EventManager(models.QuerySet): subqueries['match_{}'.format(index)] = Subquery( Element.objects.filter(group_id=OuterRef('pk'), **tag).values('order')[:1] ) - groups = groups.annotate(**subqueries) + groups = groups.annotate(**subqueries) # type: ignore for index, _ in enumerate(parts): filter['match_{}__isnull'.format(index)] = False if index > 0: diff --git a/posthog/test/test_event_model.py b/posthog/test/test_event_model.py index 7ff03481e6e..77d50bd081c 100644 --- a/posthog/test/test_event_model.py +++ b/posthog/test/test_event_model.py @@ -168,7 +168,7 @@ class TestElementGroup(BaseTest): Element(tag_name='div') ] group1 = ElementGroup.objects.create(team=self.team, elements=elements) - elements = Element.objects.all() + elements = list(Element.objects.all()) self.assertEqual(elements[0].tag_name, 'button') self.assertEqual(elements[1].tag_name, 'div') @@ -248,7 +248,7 @@ class TestActions(BaseTest): Element(tag_name='a', attr_class=None, order=0) ]) # This would error when attr_class wasn't set. - self.assertEqual(event.actions, []) + self.assertEqual(event.actions, []) class TestPreCalculation(BaseTest): def test_update_or_delete_action_steps(self): diff --git a/posthog/test/test_migration_0027.py b/posthog/test/test_migration_0027.py index ccd87b88334..e71c388bbf9 100644 --- a/posthog/test/test_migration_0027.py +++ b/posthog/test/test_migration_0027.py @@ -1,30 +1,37 @@ - +from typing import Optional, Sequence, Tuple from django.apps import apps from django.test import TestCase from django.db.migrations.executor import MigrationExecutor from django.db import connection -from typing import Optional class TestMigrations(TestCase): @property - def app(self): - return apps.get_containing_app_config(type(self).__module__).name + def app(self) -> str: + app_config = apps.get_containing_app_config(type(self).__module__) + assert app_config is not None + return app_config.name - migrate_from: Optional[str] = None - migrate_to: Optional[str] = None + @property + def migrate_from(self) -> Optional[str]: + raise NotImplementedError("TestCase '{}' must define migrate_from property".format(type(self).__name__)) + + @property + def migrate_to(self) -> Optional[str]: + raise NotImplementedError("TestCase '{}' must define migrate_to property".format(type(self).__name__)) + + def setUpBeforeMigration(self, apps): + pass def setUp(self): - assert self.migrate_from and self.migrate_to, \ - "TestCase '{}' must define migrate_from and migrate_to properties".format(type(self).__name__) - self.migrate_from = [(self.app, self.migrate_from)] - self.migrate_to = [(self.app, self.migrate_to)] + migrate_from = [(self.app, self.migrate_from)] + migrate_to = [(self.app, self.migrate_to)] executor = MigrationExecutor(connection) - old_apps = executor.loader.project_state(self.migrate_from).apps + old_apps = executor.loader.project_state(migrate_from).apps # type: ignore # Reverse to the original migration - executor.migrate(self.migrate_from) + executor.migrate(migrate_from) self.setUpBeforeMigration(old_apps) @@ -32,12 +39,9 @@ class TestMigrations(TestCase): executor = MigrationExecutor(connection) executor.loader.build_graph() # reload. with self.assertNumQueries(22): - executor.migrate(self.migrate_to) + executor.migrate(migrate_to) - self.apps = executor.loader.project_state(self.migrate_to).apps - - def setUpBeforeMigration(self, apps): - pass + self.apps = executor.loader.project_state(migrate_to).apps # type: ignore class TagsTestCase(TestMigrations): diff --git a/posthog/test/test_migration_0039.py b/posthog/test/test_migration_0039.py index 8a50ab73670..7c2c26e56e8 100644 --- a/posthog/test/test_migration_0039.py +++ b/posthog/test/test_migration_0039.py @@ -9,22 +9,30 @@ from typing import Optional class TestMigrations(TestCase): @property - def app(self): - return apps.get_containing_app_config(type(self).__module__).name + def app(self) -> str: + app_config = apps.get_containing_app_config(type(self).__module__) + assert app_config is not None + return app_config.name - migrate_from: Optional[str] = None - migrate_to: Optional[str] = None + @property + def migrate_from(self) -> Optional[str]: + raise NotImplementedError("TestCase '{}' must define migrate_from property".format(type(self).__name__)) + + @property + def migrate_to(self) -> Optional[str]: + raise NotImplementedError("TestCase '{}' must define migrate_to property".format(type(self).__name__)) + + def setUpBeforeMigration(self, apps): + pass def setUp(self): - assert self.migrate_from and self.migrate_to, \ - "TestCase '{}' must define migrate_from and migrate_to properties".format(type(self).__name__) - self.migrate_from = [(self.app, self.migrate_from)] - self.migrate_to = [(self.app, self.migrate_to)] + migrate_from = [(self.app, self.migrate_from)] + migrate_to = [(self.app, self.migrate_to)] executor = MigrationExecutor(connection) - old_apps = executor.loader.project_state(self.migrate_from).apps + old_apps = executor.loader.project_state(migrate_from).apps # type: ignore # Reverse to the original migration - executor.migrate(self.migrate_from) + executor.migrate(migrate_from) self.setUpBeforeMigration(old_apps) @@ -32,12 +40,9 @@ class TestMigrations(TestCase): executor = MigrationExecutor(connection) executor.loader.build_graph() # reload. with self.assertNumQueries(8): - executor.migrate(self.migrate_to) + executor.migrate(migrate_to) - self.apps = executor.loader.project_state(self.migrate_to).apps - - def setUpBeforeMigration(self, apps): - pass + self.apps = executor.loader.project_state(migrate_to).apps # type: ignore class TagsTestCase(TestMigrations): diff --git a/posthog/test/test_signup.py b/posthog/test/test_signup.py index e747f3480cb..8d0d54290d5 100644 --- a/posthog/test/test_signup.py +++ b/posthog/test/test_signup.py @@ -1,8 +1,8 @@ from django.test import TestCase, Client from posthog.models import User, DashboardItem, Action, Person, Event, Team -from social_django.strategy import DjangoStrategy # type: ignore -from social_django.models import DjangoStorage # type: ignore -from social_core.utils import module_member # type: ignore +from social_django.strategy import DjangoStrategy +from social_django.models import DjangoStorage +from social_core.utils import module_member from posthog.urls import social_create_user class TestSignup(TestCase): diff --git a/posthog/test/test_utils.py b/posthog/test/test_utils.py index bb5cc08ddcf..e34d40f21a4 100644 --- a/posthog/test/test_utils.py +++ b/posthog/test/test_utils.py @@ -2,7 +2,7 @@ from django.test import TestCase from posthog.models import Event from posthog.api.test.base import BaseTest from posthog.utils import relative_date_parse, properties_to_Q -from freezegun import freeze_time # type: ignore +from freezegun import freeze_time class TestRelativeDateParse(TestCase): @freeze_time('2020-01-31') diff --git a/posthog/urls.py b/posthog/urls.py index ad7111d9ec0..ddc4c6ad7ec 100644 --- a/posthog/urls.py +++ b/posthog/urls.py @@ -1,3 +1,4 @@ +from typing import cast, Optional from django.contrib import admin from django.urls import path, include, re_path from django.views.generic.base import TemplateView @@ -14,7 +15,7 @@ from .utils import render_template from .views import health, stats from posthog.demo import demo, delete_demo_data import json -import posthoganalytics # type: ignore +import posthoganalytics import os from rest_framework import permissions @@ -33,7 +34,7 @@ def login_view(request): if request.method == 'POST': email = request.POST['email'] password = request.POST['password'] - user = authenticate(request, email=email, password=password) + user = cast(Optional[User], authenticate(request, email=email, password=password)) if user is not None: login(request, user, backend='django.contrib.auth.backends.ModelBackend') if user.distinct_id: @@ -193,8 +194,8 @@ if settings.DEBUG: ] if hasattr(settings, 'INCLUDE_API_DOCS'): - from drf_yasg.views import get_schema_view # type: ignore - from drf_yasg import openapi # type: ignore + from drf_yasg.views import get_schema_view + from drf_yasg import openapi schema_view = get_schema_view( openapi.Info( title="PostHog API", diff --git a/posthog/views.py b/posthog/views.py index 0a0df17b0a6..28b683099a9 100644 --- a/posthog/views.py +++ b/posthog/views.py @@ -1,3 +1,4 @@ +from typing import Dict, Union from django.http import HttpResponse from django.conf import settings import json @@ -15,12 +16,12 @@ def health(request): def stats(request): - stats_response = {} + stats_response: Dict[str, Union[int, str]] = {} last_heartbeat = redis_instance.get("POSTHOG_HEARTBEAT") if redis_instance else None worker_heartbeat = int(time.time()) - int(last_heartbeat) if last_heartbeat else None - if worker_heartbeat == 0 or worker_heartbeat < 300: + if worker_heartbeat and (worker_heartbeat == 0 or worker_heartbeat < 300): stats_response['worker_heartbeat'] = worker_heartbeat else: stats_response['worker_heartbeat'] = 'offline'