Get flake8 running OK

apps/api/urls.py

@@ -1,13 +1,4 @@
-from django.conf.urls import include, url
+from rest_framework import routers
-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 . import views
 

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

apps/contact/tests.py

@@ -1,3 +1,3 @@
-from django.test import TestCase
+# from django.test import TestCase
 
 # Create your tests here.

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=".")

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})

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

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?"),

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)
+        with pytest.raises(Http404):
-        self.assertEqual(response.content, b"")
+            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

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 = [

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

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 . import models
-from .forms import ShortCaseStudyForm, LongCaseStudyForm, PointOfInterest
+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
+    model = models.PointOfInterest
-    form_class = 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(draft.data, content_type="application/json")
-            return HttpResponse(status=404)  # Not Found
-        else:
-            return HttpResponse(draft.data, content_type="application/json")
 
     def put(self, request):
-        # Find an existing object is there is one
+        try:
-        draft = self.get_object(request)
+            draft = CaseStudyDraft.objects.get(author=request.user)
-
-        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:
             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 [
+        for k in [
-                "official_project_documents",
+            "official_project_documents",
-                "other_documents",
+            "other_documents",
-                "shapefiles",
+            "shapefiles",
-                "images",
+            "images",
-            ]:
+        ]:
+            try:
+                keyname = k + "_files"
+                field = data["data"]["form"][keyname]
 
-                try:
+                # Ignore empty fields
-                    keyname = k + "_files"
+                if field["value"] == "":
-                    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:
                     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)

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

apps/profiles/admin.py

@@ -1,3 +1,3 @@
-from django.contrib import admin
+# from django.contrib import admin
 
 # Register your models here.

apps/profiles/models.py

@@ -1,3 +1,3 @@
-from django.db import models
+# from django.db import models
 
 # Create your models here.

apps/profiles/tests.py

@@ -1,3 +1,3 @@
-from django.test import TestCase
+# from django.test import TestCase
 
 # Create your tests here.

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 "

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: &copy; <a href="http://www.openstreetmap.org/copyright">OpenStreetMap</a>, <a href="http://viewfinderpanoramas.org">SRTM</a> | Map style: &copy; <a href="https://opentopomap.org">OpenTopoMap</a> (<a href="https://creativecommons.org/licenses/by-sa/3.0/">CC-BY-SA</a>)',
+                "attribution": (
+                    'Map data: &copy; <a href="http://www.openstreetmap.org/copyright">OpenStreetMap</a>,'
+                    ' <a href="http://viewfinderpanoramas.org">SRTM</a> |'
+                    ' Map style: &copy; <a href="https://opentopomap.org">OpenTopoMap</a>'
+                    ' (<a href="https://creativecommons.org/licenses/by-sa/3.0/">CC-BY-SA</a>)'
+                ),
             },
         ),
         (
             "English",
             "https://{s}.basemaps.cartocdn.com/light_all/{z}/{x}/{y}{r}.png",
             {
-                "attribution": '&copy; <a href="http://www.openstreetmap.org/copyright">OpenStreetMap</a> &copy; <a href="https://carto.com/attributions">CARTO</a>',
+                "attribution": (
+                    '&copy; <a href="http://www.openstreetmap.org/copyright">OpenStreetMap</a>'
+                    ' &copy; <a href="https://carto.com/attributions">CARTO</a>'
+                ),
                 "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

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 test with firefox, uncomment these lines and comment those pertaining
-        ### to Chrome. However, it will not work with GitLab CI because the docker
+        # to Chrome. However, it will not work with GitLab CI because the docker
-        ### container does not have the geckodriver installed.
+        # container does not have the geckodriver installed.
+        #
         #   profile = webdriver.FirefoxProfile()
         #   profile.set_preference("dom.forms.number", False)
         #   cls.sl = webdriver.Firefox(profile)

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
 
 

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