From 73fe51ba9c09bddd5275797c5f869d46089a1d84 Mon Sep 17 00:00:00 2001 From: Anna Sidwell Date: Mon, 19 Aug 2019 23:52:27 +0200 Subject: [PATCH] Get flake8 running OK --- apps/api/urls.py | 11 +--- apps/api/views.py | 6 +-- apps/contact/tests.py | 2 +- apps/files/models.py | 2 - apps/files/tests.py | 2 +- apps/files/views.py | 1 - apps/map/models.py | 7 +-- apps/map/tests.py | 12 ++--- apps/map/urls.py | 2 - apps/map/validators.py | 2 +- apps/map/views.py | 108 +++++++++++++++++++--------------------- apps/map/widgets.py | 2 +- apps/profiles/admin.py | 2 +- apps/profiles/models.py | 2 +- apps/profiles/tests.py | 2 +- manage.py | 2 +- ojusomap/settings.py | 15 ++++-- ojusomap/tests.py | 8 +-- ojusomap/urls.py | 6 --- setup.cfg | 8 +++ 20 files changed, 93 insertions(+), 109 deletions(-) diff --git a/apps/api/urls.py b/apps/api/urls.py index 19d7426..589584e 100644 --- a/apps/api/urls.py +++ b/apps/api/urls.py @@ -1,13 +1,4 @@ -from django.conf.urls import include, url -from django.urls import reverse -from django.conf.urls.i18n import i18n_patterns -from django.contrib import admin -from django.contrib.auth.models import User -from rest_framework import routers, serializers, viewsets -from rest_framework_gis import serializers as gis_serializers - -from apps.files.models import File -from apps.map.models import CaseStudy, PointOfInterest +from rest_framework import routers from . import views diff --git a/apps/api/views.py b/apps/api/views.py index 2e48253..56f6ee5 100644 --- a/apps/api/views.py +++ b/apps/api/views.py @@ -1,9 +1,5 @@ -from django.conf.urls import include, url -from django.urls import reverse -from django.conf.urls.i18n import i18n_patterns -from django.contrib import admin from django.contrib.auth.models import User -from rest_framework import routers, serializers, viewsets +from rest_framework import serializers, viewsets from rest_framework_gis import serializers as gis_serializers from apps.files.models import File diff --git a/apps/contact/tests.py b/apps/contact/tests.py index 7ce503c..a79ca8b 100644 --- a/apps/contact/tests.py +++ b/apps/contact/tests.py @@ -1,3 +1,3 @@ -from django.test import TestCase +# from django.test import TestCase # Create your tests here. diff --git a/apps/files/models.py b/apps/files/models.py index 5a2809a..89772cc 100644 --- a/apps/files/models.py +++ b/apps/files/models.py @@ -2,8 +2,6 @@ from django.contrib.auth.models import User from django.db import models from django.utils.translation import ugettext_lazy as _ -from apps.map.models import CaseStudy, CaseStudyDraft - class BaseFile(models.Model): file = models.FileField(upload_to=".") diff --git a/apps/files/tests.py b/apps/files/tests.py index 77df135..0120cb3 100644 --- a/apps/files/tests.py +++ b/apps/files/tests.py @@ -47,7 +47,7 @@ class FileTests(TestCase): self.assertRedirects(response, login_url) def test_post_and_delete(self): - login = self.login() + self.login() with open("apps/map/static/map/ojuso-logo-white.png", "rb") as fp: response = self.client.post(reverse("files:upload_image"), {"file": fp}) diff --git a/apps/files/views.py b/apps/files/views.py index abda5dc..ff7c621 100644 --- a/apps/files/views.py +++ b/apps/files/views.py @@ -1,7 +1,6 @@ from django.core.exceptions import PermissionDenied from django.contrib.auth.mixins import LoginRequiredMixin from django.http import JsonResponse -from django.shortcuts import render from django.views.generic import FormView, DetailView from .forms import ImageFileForm, FileForm diff --git a/apps/map/models.py b/apps/map/models.py index 7058af3..70e45e6 100644 --- a/apps/map/models.py +++ b/apps/map/models.py @@ -129,7 +129,8 @@ class CaseStudy(models.Model): ( "FMO", _( - "Nederlandse Financieringsmaatschappij voor Ontwikkelingslanden NV (Netherlands Development Finance Company, FMO)" + "Nederlandse Financieringsmaatschappij voor Ontwikkelingslanden NV" + " (Netherlands Development Finance Company, FMO)" ), ), ("NDB", _("New Development Bank (NDB) (formerly BRICS Development Bank)")), @@ -712,7 +713,7 @@ class CaseStudy(models.Model): blank=True, ) - ## Energy generation project + # -- Energy generation project -- generation_type = models.CharField( verbose_name=_("What kind of energy is generated?"), @@ -917,7 +918,7 @@ class CaseStudy(models.Model): blank=True, ) - ## Manufacturing + # -- Manufacturing -- manufacturing_type = models.CharField( verbose_name=_("Which of the following options best describes this case?"), diff --git a/apps/map/tests.py b/apps/map/tests.py index b930721..ae7f0e8 100644 --- a/apps/map/tests.py +++ b/apps/map/tests.py @@ -1,6 +1,7 @@ +import pytest from django.contrib.auth.models import User -from django.contrib.sessions.middleware import SessionMiddleware from django.http import QueryDict +from django.http.response import Http404 from django.test import RequestFactory from django.test import TestCase from django.urls import reverse @@ -60,10 +61,9 @@ class CaseStudyDraftAPITests(TestCase): def test_get_empty(self): request = self.factory.get(self.url) request.user = self.user - response = DraftsAPI.as_view()(request) - self.assertEqual(response.status_code, 404) - self.assertEqual(response.content, b"") + with pytest.raises(Http404): + DraftsAPI.as_view()(request) def test_put_works(self): contents = '{"test":1}' @@ -86,7 +86,7 @@ class CaseStudyDraftAPITests(TestCase): self.assertEqual(response.content.decode(), draft.data) def test_update(self): - draft = self.fake_draft() + self.fake_draft() new_contents = '{"fnord": 7}' request = self.factory.put(self.url, data=new_contents) @@ -100,7 +100,7 @@ class CaseStudyDraftAPITests(TestCase): ) def test_delete(self): - draft = self.fake_draft() + self.fake_draft() request = self.factory.delete(self.url) request.user = self.user diff --git a/apps/map/urls.py b/apps/map/urls.py index 45dcab2..f7d18a3 100644 --- a/apps/map/urls.py +++ b/apps/map/urls.py @@ -2,9 +2,7 @@ from django.conf.urls import url from django.urls import reverse_lazy from django.views.generic import RedirectView from django.views.i18n import JavaScriptCatalog -from djgeojson.views import GeoJSONLayerView -from .models import CaseStudy from . import views urlpatterns = [ diff --git a/apps/map/validators.py b/apps/map/validators.py index 4959445..8ed6493 100644 --- a/apps/map/validators.py +++ b/apps/map/validators.py @@ -20,4 +20,4 @@ class VimeoURLValidator(RegexValidator): class YouTubeOrVimeoValidator(RegexValidator): - regex = r"https?:\/\/(player.|www.)?(vimeo\.com|youtu(be\.com|\.be))\/(video\/|embed\/|watch\?v=|v\/)?([A-Za-z0-9]{1,11}).+" + regex = r"https?:\/\/(player.|www.)?(vimeo\.com|youtu(be\.com|\.be))\/(video\/|embed\/|watch\?v=|v\/)?([A-Za-z0-9]{1,11}).+" # noqa diff --git a/apps/map/views.py b/apps/map/views.py index 4ddfe21..3bb4054 100644 --- a/apps/map/views.py +++ b/apps/map/views.py @@ -1,10 +1,12 @@ +import logging import json +from django.shortcuts import get_object_or_404 from django.conf import settings from django.contrib.auth.mixins import LoginRequiredMixin, UserPassesTestMixin from django.core.mail import send_mail from django.db.models import Q -from django.http import Http404, HttpResponse +from django.http import HttpResponse from django.urls import reverse from django.urls import reverse_lazy from django.utils.translation import get_language @@ -17,9 +19,12 @@ from dal import autocomplete from apps.files.models import File -from .models import CaseStudy, CaseStudyDraft, SpatialRefSys, PointOfInterest -from .forms import ShortCaseStudyForm, LongCaseStudyForm, PointOfInterest +from . import models +from . import forms +from .models import CaseStudy, CaseStudyDraft, SpatialRefSys +from .forms import ShortCaseStudyForm, LongCaseStudyForm +logger = logging.getLogger(__name__) NOTIFY_MESSAGE = """ Hello, @@ -47,8 +52,8 @@ class CreatePointOfInterest(LoginRequiredMixin, CreateView): template_name = "map/form-poi.html" success_url = "/case-study/create/success/" - model = PointOfInterest - form_class = PointOfInterest + model = models.PointOfInterest + form_class = forms.PointOfInterest def send_email(study_id): @@ -67,10 +72,10 @@ def send_email(study_id): fail_silently=False, ) - except: + except Exception: + logging.exception("Sending mail failed") # XXX This is bad. We should do something more useful with the error # than this. - pass def delete_user_draft(user_id): @@ -200,70 +205,57 @@ class DraftsAPI(LoginRequiredMixin, View): XXX This should be refactored to use csrf protection. """ - def get_object(self, request): - try: - return CaseStudyDraft.objects.get(author=request.user) - except: - return None - def get(self, request): - draft = self.get_object(request) + draft = get_object_or_404(models.CaseStudyDraft, author=request.user) - if draft == None: - return HttpResponse(status=404) # Not Found - else: - return HttpResponse(draft.data, content_type="application/json") + return HttpResponse(draft.data, content_type="application/json") def put(self, request): - # Find an existing object is there is one - draft = self.get_object(request) - - if draft == None: - # If there isn't, create a new draft... - draft = CaseStudyDraft(author=request.user, data=request.body.decode()) - draft.save() - return HttpResponse(status=201) # Created - else: + try: + draft = CaseStudyDraft.objects.get(author=request.user) draft.data = request.body.decode() draft.save() return HttpResponse(status=200) # OK + except models.CaseStudyDraft.DoesNotExist: + # If it doesn't exist, create it + CaseStudyDraft.objects.create(author=request.user, data=request.body.decode()) + return HttpResponse(status=201) # Created + def delete(self, request): - draft = self.get_object(request) + draft = get_object_or_404(models.CaseStudyDraft, author=request.user) - if draft != None: - data = json.loads(draft.data) + data = json.loads(draft.data) - for k in [ - "official_project_documents", - "other_documents", - "shapefiles", - "images", - ]: + for k in [ + "official_project_documents", + "other_documents", + "shapefiles", + "images", + ]: + try: + keyname = k + "_files" + field = data["data"]["form"][keyname] - try: - keyname = k + "_files" - field = data["data"]["form"][keyname] - - # Ignore empty fields - if field["value"] == "": - continue - - file_list = json.loads(field["value"]) - - # Delete those items - for item in file_list: - try: - f = File.objects.get(id=item["id"]) - if f.user != self.request.user: - continue - f.delete() - except File.DoesNotExist: - continue - - except: + # Ignore empty fields + if field["value"] == "": continue - draft.delete() + file_list = json.loads(field["value"]) + + # Delete those items + for item in file_list: + try: + f = File.objects.get(id=item["id"]) + if f.user != self.request.user: + continue + f.delete() + except File.DoesNotExist: + continue + + except Exception: # XXX What are we guarding against here? + continue + + draft.delete() return HttpResponse(status=204) diff --git a/apps/map/widgets.py b/apps/map/widgets.py index ea1b69e..2db4d46 100644 --- a/apps/map/widgets.py +++ b/apps/map/widgets.py @@ -50,5 +50,5 @@ class JSONFileListWidget(widgets.HiddenInput): try: filelist = json.loads(value) return [file["id"] for file in filelist] - except JSONDecodeError: + except json.JSONDecodeError: return None diff --git a/apps/profiles/admin.py b/apps/profiles/admin.py index 8c38f3f..4185d36 100644 --- a/apps/profiles/admin.py +++ b/apps/profiles/admin.py @@ -1,3 +1,3 @@ -from django.contrib import admin +# from django.contrib import admin # Register your models here. diff --git a/apps/profiles/models.py b/apps/profiles/models.py index 71a8362..0b4331b 100644 --- a/apps/profiles/models.py +++ b/apps/profiles/models.py @@ -1,3 +1,3 @@ -from django.db import models +# from django.db import models # Create your models here. diff --git a/apps/profiles/tests.py b/apps/profiles/tests.py index 7ce503c..a79ca8b 100644 --- a/apps/profiles/tests.py +++ b/apps/profiles/tests.py @@ -1,3 +1,3 @@ -from django.test import TestCase +# from django.test import TestCase # Create your tests here. diff --git a/manage.py b/manage.py index 74641be..63c4763 100755 --- a/manage.py +++ b/manage.py @@ -11,7 +11,7 @@ if __name__ == "__main__": # issue is really that Django is missing to avoid masking other # exceptions on Python 2. try: - import django + import django # noqa except ImportError: raise ImportError( "Couldn't import Django. Are you sure it's installed and " diff --git a/ojusomap/settings.py b/ojusomap/settings.py index f20c696..c7a89d9 100644 --- a/ojusomap/settings.py +++ b/ojusomap/settings.py @@ -13,6 +13,7 @@ https://docs.djangoproject.com/en/1.11/ref/settings/ import os import raven from django.utils.translation import ugettext_lazy as _ +from django.contrib.messages import constants as messages # Build paths inside the project like this: os.path.join(BASE_DIR, ...) BASE_DIR = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) @@ -257,14 +258,22 @@ LEAFLET_CONFIG = { "https://{s}.tile.opentopomap.org/{z}/{x}/{y}.png", { "maxZoom": 17, - "attribution": 'Map data: © OpenStreetMap, SRTM | Map style: © OpenTopoMap (CC-BY-SA)', + "attribution": ( + 'Map data: © OpenStreetMap,' + ' SRTM |' + ' Map style: © OpenTopoMap' + ' (CC-BY-SA)' + ), }, ), ( "English", "https://{s}.basemaps.cartocdn.com/light_all/{z}/{x}/{y}{r}.png", { - "attribution": '© OpenStreetMap © CARTO', + "attribution": ( + '© OpenStreetMap' + ' © CARTO' + ), "subdomains": "abcd", "maxZoom": 19, }, @@ -329,8 +338,6 @@ AVATAR_GRAVATAR_DEFAULT = "mm" AVATAR_CLEANUP_DELETED = True # Messages -from django.contrib.messages import constants as messages - MESSAGE_TAGS = {messages.ERROR: "danger"} # Feature flags diff --git a/ojusomap/tests.py b/ojusomap/tests.py index f597884..bff653e 100644 --- a/ojusomap/tests.py +++ b/ojusomap/tests.py @@ -5,7 +5,6 @@ from django.contrib.auth.models import User from django.test import LiveServerTestCase from selenium import webdriver -from selenium.common import exceptions from selenium.webdriver.common.by import By from selenium.webdriver.support.expected_conditions import ( staleness_of, @@ -23,9 +22,10 @@ TIMEOUT = 8 class SeleniumTest(LiveServerTestCase): @classmethod def setUpClass(cls): - ### To test with firefox, uncomment these lines and comment those pertaining - ### to Chrome. However, it will not work with GitLab CI because the docker - ### container does not have the geckodriver installed. + # To test with firefox, uncomment these lines and comment those pertaining + # to Chrome. However, it will not work with GitLab CI because the docker + # container does not have the geckodriver installed. + # # profile = webdriver.FirefoxProfile() # profile.set_preference("dom.forms.number", False) # cls.sl = webdriver.Firefox(profile) diff --git a/ojusomap/urls.py b/ojusomap/urls.py index 051ba4a..76fdb2a 100644 --- a/ojusomap/urls.py +++ b/ojusomap/urls.py @@ -1,13 +1,7 @@ from django.conf.urls import include, url -from django.urls import reverse from django.conf.urls.i18n import i18n_patterns from django.contrib import admin -from django.contrib.auth.models import User -from rest_framework import routers, serializers, viewsets -from rest_framework_gis import serializers as gis_serializers -from apps.files.models import File -from apps.map.models import CaseStudy, PointOfInterest from .views import LanguageDropdownView diff --git a/setup.cfg b/setup.cfg index cae3ea2..4e06a17 100644 --- a/setup.cfg +++ b/setup.cfg @@ -1,3 +1,11 @@ +[flake8] +max-line-length = 120 +exclude = .tox,.git,*/migrations/*,*/static/CACHE/*,docs/*,node_modules + +[pycodestyle] +max-line-length = 120 +exclude = .tox,.git,*/migrations/*,*/static/CACHE/*,docs,node_modules + [tool:pytest] DJANGO_SETTINGS_MODULE = ojusomap.settings python_files = tests.py test_*.py *_tests.py