Feature: enable ignoring query string (#5)

* Enable ignoring url query strin

* Register cms pytest marker

* Update documentation

* Fix black style

* Update python version

* Update factories

* Drop django 1.11

* Update pipfile

* Fix style

* Naming

Co-authored-by: Magdalena Rother <mrother@magdalenas-mbp.localdomain>

Pipfile

@@ -14,8 +14,9 @@ sphinx = "*"
 sphinx-rtd-theme = "*"
 tox = "*"
 isort = "*"
-pytest-black = "*"
 twine = "*"
+black = "*"
+pytest-black = "*"
 
 [packages]
 django = "*"
@@ -24,4 +25,7 @@ django-rq = "*"
 django-inline-static = "*"
 
 [requires]
-python_version = "3.6"
+python_version = "3.7"
+
+[pipenv]
+allow_prereleases = true

critical/api.py

@@ -6,7 +6,6 @@
 from .exceptions import PenthouseException
 from .utils import complete_url
 
-
 logger = logging.getLogger(__name__)
 
 

critical/apps.py

@@ -11,6 +11,7 @@ def ready(self):
             # If django-cms is installed include signals
             # for refreshing critical css on page publish
             import cms  # noqa
+
             import critical.signals.handlers  # noqa
         except ImportError:
             pass

critical/signals/handlers.py

@@ -5,7 +5,6 @@
 
 from ..models import Critical
 
-
 logger = logging.getLogger(__name__)
 
 

critical/tasks.py

@@ -4,15 +4,14 @@
 from django_rq import job
 from inline_static.css import transform_css_urls
 
-
 logger = logging.getLogger(__name__)
 
 
 @job
 def calculate_critical_css(critical_id, original_path):
-    from .services import calculate_critical_css as service_calculate_critical_css
     from .exceptions import CriticalException
     from .models import Critical
+    from .services import calculate_critical_css as service_calculate
 
     logger.info('Task: critical css with id {0} requested.'.format(critical_id))
     critical = Critical.objects.filter(id=critical_id).first()
@@ -25,7 +24,7 @@ def calculate_critical_css(critical_id, original_path):
     logger.info('Task: critical css with id {0} pending.'.format(critical_id))
 
     try:
-        critical_css_raw = service_calculate_critical_css(critical.url, critical.path)
+        critical_css_raw = service_calculate(critical.url, critical.path)
         critical_css = transform_css_urls(original_path, critical.path, critical_css_raw)
     except Exception as exc:
         critical.is_pending = False

critical/templatetags/critical_tags.py

@@ -7,7 +7,6 @@
 from ..tasks import calculate_critical_css
 from ..utils import get_url_from_request, use_critical_css_for_request
 
-
 register = template.Library()
 logger = logging.getLogger(__name__)
 

critical/utils.py

@@ -28,6 +28,8 @@ def get_url_from_request(request):
         Page ansolute url.
 
     """
+    if getattr(settings, 'CRITICAL_CSS_IGNORE_QUERY_STRING', False):
+        return request.path
     return request.get_full_path()
 
 

docs/usage.rst

@@ -91,7 +91,9 @@ Additional settings
 
 * By setting ``CRITICAL_CSS_ACTIVE`` to `False` you can deactivate css calculation
   for your dev environment. By default critical css calculation is set to active.
-
+* By setting ``CRITICAL_CSS_IGNORE_QUERY_STRING`` to `False` you can ignore query string
+  in the url. Then both urls `/?p=1`and `/?p=2` will be treated as one `/` and only one database
+  object will be created.
 
 Usage with django-cms
 ---------------------

examples/critical-example/criticalexample/settings.py

@@ -12,7 +12,6 @@
 
 import os
 
-
 # Build paths inside the project like this: os.path.join(BASE_DIR, ...)
 BASE_DIR = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
 

examples/critical-example/criticalexample/urls.py

@@ -18,7 +18,6 @@
 
 from .views import ExampleView
 
-
 urlpatterns = [
     path('admin/rq/', include('django_rq.urls')),
     path('admin/', admin.site.urls),

examples/critical-example/criticalexample/wsgi.py

@@ -11,7 +11,6 @@
 
 from django.core.wsgi import get_wsgi_application
 
-
 os.environ.setdefault('DJANGO_SETTINGS_MODULE', 'criticalexample.settings')
 
 application = get_wsgi_application()

examples/critical-example/manage.py

@@ -2,7 +2,6 @@
 import os
 import sys
 
-
 if __name__ == '__main__':
     os.environ.setdefault('DJANGO_SETTINGS_MODULE', 'criticalexample.settings')
     try:

setup.cfg

@@ -10,6 +10,9 @@ python_files =
 
 cov_report = term-missing
 
+markers =
+	cms: marks tests that require django-cms
+
 
 DJANGO_SETTINGS_MODULE = tests.settings
 

setup.py

@@ -34,6 +34,6 @@
         'Intended Audience :: Developers',
         'License :: OSI Approved :: MIT License',
         'Programming Language :: Python :: 3',
-        'Programming Language :: Python :: 3.6',
+        'Programming Language :: Python :: 3.7',
     ],
 )

tests/cms_settings.py

@@ -1,6 +1,5 @@
 import os
 
-
 LANGUAGE_CODE = 'de'
 
 ROOT_URLCONF = 'tests.urls'

tests/factories.py

@@ -3,7 +3,7 @@
 from critical.models import Critical
 
 
-class CriticalFactory(factory.DjangoModelFactory):
+class CriticalFactory(factory.django.DjangoModelFactory):
     url = factory.Faker('url')
     path = factory.Faker('uri_path')
     css = factory.Faker('text')

tests/settings.py

@@ -26,6 +26,7 @@
 
 CRITICAL_CSS_ACTIVE = True
 PENTHOUSE_URL = 'http://localhost:3000/'
+CRITICAL_CSS_IGNORE_QUERY_STRING = False
 
 try:
     import cms  # noqa

tests/test_api.py

@@ -4,7 +4,11 @@
 import requests
 from django.contrib.sites.models import Site
 
-from critical.api import PenthouseApi, PenthouseException, calculate_critical_css
+from critical.api import (
+    PenthouseApi,
+    PenthouseException,
+    calculate_critical_css,
+)
 
 
 @pytest.mark.django_db

tests/test_signals.py

@@ -2,7 +2,6 @@
 
 from critical.models import Critical
 
-
 try:
     from cms.api import create_page, publish_page
 except ImportError:

tests/test_templatetags.py

@@ -53,6 +53,58 @@ def test_critical_object_created(self, use_critical_mock, critical_task_mock, rf
             '<link rel="stylesheet" type="text/css" ' 'href="/static/css/styles.css" />'
         )
 
+    @mock.patch('critical.templatetags.critical_tags.calculate_critical_css')
+    @mock.patch('critical.templatetags.critical_tags.use_critical_css_for_request')
+    def test_critical_object_created_with_query_string(
+        self, use_critical_mock, critical_task_mock, rf, settings
+    ):
+        settings.CRITICAL_CSS_IGNORE_QUERY_STRING = False
+        use_critical_mock.return_value = True
+        critical_task_mock.delay.return_value = 'foo bar bazz'
+
+        request = rf.get('/?p=1')
+        context = Context({'request': request})
+        template_to_render = Template(
+            '{% load critical_tags %}' '{% critical_css "css/styles.css" %}'
+        )
+        template_to_render.render(context)
+
+        request = rf.get('/?p=2')
+        context = Context({'request': request})
+        template_to_render = Template(
+            '{% load critical_tags %}' '{% critical_css "css/styles.css" %}'
+        )
+        template_to_render.render(context)
+
+        assert Critical.objects.filter(url='/').count() == 0
+        assert Critical.objects.filter(url='/?p=1').count() == 1
+        assert Critical.objects.filter(url='/?p=2').count() == 1
+
+    @mock.patch('critical.templatetags.critical_tags.calculate_critical_css')
+    @mock.patch('critical.templatetags.critical_tags.use_critical_css_for_request')
+    def test_critical_object_created_ignore_query_string(
+        self, use_critical_mock, critical_task_mock, rf, settings
+    ):
+        settings.CRITICAL_CSS_IGNORE_QUERY_STRING = True
+
+        request = rf.get('/?p=1')
+        context = Context({'request': request})
+        template_to_render = Template(
+            '{% load critical_tags %}' '{% critical_css "css/styles.css" %}'
+        )
+        template_to_render.render(context)
+
+        request = rf.get('/?p=2')
+        context = Context({'request': request})
+        template_to_render = Template(
+            '{% load critical_tags %}' '{% critical_css "css/styles.css" %}'
+        )
+        template_to_render.render(context)
+
+        assert Critical.objects.filter(url='/').count() == 1
+        assert Critical.objects.filter(url='/?p=1').count() == 0
+        assert Critical.objects.filter(url='/?p=2').count() == 0
+
     @mock.patch('critical.templatetags.critical_tags.calculate_critical_css')
     @mock.patch('critical.templatetags.critical_tags.use_critical_css_for_request')
     def test_success(self, use_critical_mock, critical_task_mock, rf):
@@ -69,6 +121,25 @@ def test_success(self, use_critical_mock, critical_task_mock, rf):
         assert '<style type="text/css">foo bar</style>' in rendered_template
         assert critical_task_mock.delay.call_count == 0
 
+    @mock.patch('critical.templatetags.critical_tags.calculate_critical_css')
+    @mock.patch('critical.templatetags.critical_tags.use_critical_css_for_request')
+    def test_success_ignore_query_string(
+        self, use_critical_mock, critical_task_mock, rf, settings
+    ):
+        settings.CRITICAL_CSS_IGNORE_QUERY_STRING = True
+        use_critical_mock.return_value = True
+        request = rf.get('/?p=3')
+        CriticalFactory.create(url='/', path='/static/css/styles.css', css='foo bar')
+
+        context = Context({'request': request})
+        template_to_render = Template(
+            '{% load critical_tags %}' '{% critical_css "css/styles.css" %}'
+        )
+
+        rendered_template = template_to_render.render(context)
+        assert '<style type="text/css">foo bar</style>' in rendered_template
+        assert critical_task_mock.delay.call_count == 0
+
     @mock.patch('critical.templatetags.critical_tags.calculate_critical_css')
     @mock.patch('critical.templatetags.critical_tags.use_critical_css_for_request')
     def test_wrong_path(self, use_critical_mock, critical_task_mock, rf):

tests/test_utils.py

@@ -10,7 +10,6 @@
     use_critical_css_for_request,
 )
 
-
 try:
     from cms.api import create_page, publish_page
     from cms.models import Page
@@ -27,7 +26,9 @@ def test_django_cms_is_present():
         assert django_cms_is_present() is False
 
 
-def test_get_url_from_request(rf):
+def test_get_url_from_request(rf, settings):
+    settings.CRITICAL_CSS_IGNORE_QUERY_STRING = False
+
     request = rf.get('/')
     assert get_url_from_request(request) == '/'
 
@@ -37,6 +38,11 @@ def test_get_url_from_request(rf):
     request = rf.get('/#anchor')
     assert get_url_from_request(request) == '/'
 
+    settings.CRITICAL_CSS_IGNORE_QUERY_STRING = True
+
+    request = rf.get('/foo/?bar=bazz')
+    assert get_url_from_request(request) == '/foo/'
+
 
 @mock.patch('critical.utils.Site.objects.get_current')
 def test_complete_url_secure(site_mock, admin_user):

tox.ini

@@ -1,16 +1,13 @@
 [tox]
 skipsdist = True
 envlist=
-	py36-django111
-	py36-django22
-	py36-django111-djangocms36
-	py36-django22-djangocms36
+	py37-django22
+	py37-django22-djangocms36
 
 [testenv]
 whitelist_externals = /bin/sh
 deps =
 	pipenv
-	django111: Django>=1.11,<1.12
 	django22: Django>=2.2,<2.3
 	djangocms36: django-cms>=3.6,<3.7