Add integration tests (#20)

* Adding integration tests with inlines * Adding more tests * FIx make file * Setup CI build matrix to work with integration tests * Try again * Fix workflow synctax * Clean up workflow * Format and some lint stuff * Try codecov * yml * More Testing * Minor lint things * Update * Try again for codecov * Updates * Try? * Ignore quotes * Exclude test project * try this? * checkout required * Rename * clean up configs * Fix * Allow to fail * ignores * FInish the integration tests for cache * Fix workflow yml * fix * Try up again * TRy again * Fix * Fix * Fix tests Co-authored-by: Thu Trang Pham <[email protected]>
TrangPham · Mar 6, 2021 · ad7409b · ad7409b
1 parent 4f50c63
commit ad7409b
Show file tree

Hide file tree

Showing 31 changed files with 677 additions and 185 deletions.
diff --git a/.coveragerc b/.coveragerc
diff --git a/.env b/.env
@@ -0,0 +1,2 @@
+PYTHON_VERSION=3.8
+DJANGO_VERSION=3.1.7
diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
@@ -14,47 +14,41 @@ on:
       - created
 
 jobs:
+  lint:
+    runs-on: ubuntu-latest
+    steps:
+    - uses: actions/checkout@v2
+    - name: wemake-python-styleguide
+      uses: wemake-services/[email protected]
+      with:
+        path: admin_confirm
+        reporter: 'github-pr-review'
+      env:
+        GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+      continue-on-error: true
+
   test:
     runs-on: ubuntu-latest
     strategy:
       matrix:
         python-version: [3.6, 3.7, 3.8, 3.9]
         django-version: [2.2, 3.0]
+    env:
+      DJANGO_VERSION: ${{ matrix.django-version }}
+      PYTHON_VERSION: ${{ matrix.python-version }}
+      COMPOSE_INTERACTIVE_NO_CLI: 1
     steps:
     - uses: actions/checkout@v2
-    - name: Set up Python ${{ matrix.python-version }}
-      uses: actions/setup-python@v2
-      with:
-        python-version: ${{ matrix.python-version }}
-    - name: Install Django ${{ matrix.django-version }}
-      run: |
-        pip install django==${{ matrix.django-version }}
-    - name: Install dependencies
-      run: |
-        python -m pip install --upgrade pip
-        pip install flake8 pytest
-        if [ -f requirements.txt ]; then pip install -r requirements.txt; fi
-    - name: Lint with flake8
+    - name: Build Docker for Python 3.6
+      if: ${{ matrix.python-version == 3.6 }}
       run: |
-        # stop the build if there are Python syntax errors or undefined names
-        flake8 . --count --select=E9,F63,F7,F82 --show-source --statistics
-        # exit-zero treats all errors as warnings. The GitHub editor is 127 chars wide
-        flake8 . --count --exit-zero --max-complexity=10 --max-line-length=127 --statistics
-    - name: Unit Test
+        export SELENIUM_VERSION=3.141.0
+        docker-compose build
+    - name: Build Docker for other Python versions
+      if: ${{ matrix.python-version != 3.6 }}
       run: |
-        make test
-    - name: Coveralls
-      uses: AndreMiras/coveralls-python-action@develop
-      with:
-        parallel: true
-        flag-name: Unit Test
-
-  integration-test:
-    runs-on: ubuntu-latest
-    steps:
-    - uses: actions/checkout@v2
-    - name: Build Docker
-      run: docker-compose build
+        export SELENIUM_VERSION=4.0.0a7
+        docker-compose build
     - name: Start Docker
       run: docker-compose up -d
     - name: Integration Test
@@ -63,10 +57,9 @@ jobs:
       uses: AndreMiras/coveralls-python-action@develop
       with:
         parallel: true
-        flag-name: Integration Test
 
   coveralls:
-    needs: [test, integration-test]
+    needs: [test]
     runs-on: ubuntu-latest
     steps:
     - name: Coveralls Finished

diff --git a/Dockerfile b/Dockerfile
@@ -1,7 +1,12 @@
-FROM python:3
+ARG PYTHON_VERSION=3.8
+FROM python:${PYTHON_VERSION}
 ENV PYTHONUNBUFFERED=1
 ENV USE_DOCKER=true
 WORKDIR /code
-COPY requirements.txt /code/
-RUN pip install -r requirements.txt
 COPY . /code/
+ARG DJANGO_VERSION="3.1.7"
+RUN pip install django==${DJANGO_VERSION}
+RUN pip install -r requirements.txt
+RUN pip install -e .
+ARG SELENIUM_VERSION="4.0.0a7"
+RUN pip install selenium~=${SELENIUM_VERSION}
diff --git a/README.md b/README.md
@@ -153,6 +153,13 @@ pyenv vituralenv 3.8.0 django-admin-confirm-3.8.0
 
 Now your terminal should have `(django-admin-confirm-3.8.0)` prefix, because `.python-version` should have auto switch your virtual env
 
+Install requirements
+
+```
+pip install -r requirements.txt
+pip install -e .
+```
+
 Run **migrations** and create a superuser and run the server
 
 ```
@@ -237,7 +244,6 @@ pip uninstall django_admin_confirm
 make install-testpypi VERSION=<VERSION>
 ```
 
-Update version in `requirements.txt`
 Add test locally
 
 ```

diff --git a/admin_confirm/admin.py b/admin_confirm/admin.py
@@ -11,7 +11,6 @@
 from admin_confirm.utils import get_admin_change_url, snake_to_title_case
 from django.core.cache import cache
 from django.views.decorators.cache import cache_control
-from django.forms.formsets import all_valid
 from admin_confirm.constants import (
     CACHE_TIMEOUT,
     CONFIRMATION_RECEIVED,
@@ -139,7 +138,7 @@ def _display_for_changed_data(field, initial_value, new_value):
                 return [initial_value, new_value]
 
             if initial_value:
-                if new_value == False:
+                if new_value is False:
                     # Clear has been selected
                     return [initial_value.name, None]
                 elif new_value:
@@ -249,7 +248,7 @@ def _reconstruct_request_files():
             if CONFIRM_CHANGE in modified_post:
                 del modified_post[CONFIRM_CHANGE]
 
-            if object_id and not SAVE_AS_NEW in request.POST:
+            if object_id and SAVE_AS_NEW not in request.POST:
                 # Update the obj with the new uploaded files
                 # then pass rest of changes to Django
                 obj = self.model.objects.filter(id=object_id).first()
@@ -287,6 +286,20 @@ def _reconstruct_request_files():
         cache.delete_many(CACHE_KEYS.values())
         return super()._changeform_view(request, object_id, form_url, extra_context)
 
+    def _get_cleared_fields(self, request):
+        """
+        Checks for any ImageField or FileField which have been cleared by user.
+
+        Because the form that is generated by Django for the model, would not have the
+        `<field>-clear` inputs in them, they have to be injected into the hidden form
+        on the confirmation page.
+        """
+        return [
+            input_name.split("-clear")[0]
+            for input_name in request.POST.keys()
+            if input_name.endswith("-clear")
+        ]
+
     def _change_confirmation_view(self, request, object_id, form_url, extra_context):
         # This code is taken from super()._changeform_view
         # https://github.com/django/django/blob/master/django/contrib/admin/options.py#L1575-L1592
@@ -348,10 +361,14 @@ def _change_confirmation_view(self, request, object_id, form_url, extra_context)
                 save_action = key
                 break
 
+        cleared_fields = []
         if form.is_multipart():
             cache.set(CACHE_KEYS["post"], request.POST, timeout=CACHE_TIMEOUT)
             cache.set(CACHE_KEYS["object"], new_object, timeout=CACHE_TIMEOUT)
 
+            # Handle when files are cleared - since the `form` object would not hold that info
+            cleared_fields = self._get_cleared_fields(request)
+
         title_action = _("adding") if add_or_new else _("changing")
         context = {
             **self.admin_site.each_context(request),
@@ -368,6 +385,7 @@ def _change_confirmation_view(self, request, object_id, form_url, extra_context)
             "save_as_new": SAVE_AS_NEW in request.POST,
             "submit_name": save_action,
             "form": form,
+            "cleared_fields": cleared_fields,
             "formsets": formsets,
             **(extra_context or {}),
         }

diff --git a/admin_confirm/templates/admin/change_confirmation.html b/admin_confirm/templates/admin/change_confirmation.html
@@ -40,8 +40,11 @@
     {% include "admin/change_data.html" %}
 
     <form {% if form.is_multipart %}enctype="multipart/form-data"{% endif %} method="post" {% if add %}action="{% url opts|admin_urlname:'add'%}" {% else %}action="{% url opts|admin_urlname:'change' object_id|admin_urlquote %}"{% endif %}>{% csrf_token %}
-        <div class="hidden">
+        <div class="hidden" id="hidden-form">
             {{form.as_p}}
+            {% for cleared_field in cleared_fields %}
+                <input type="checkbox" name="{{ cleared_field }}-clear" checked>
+            {% endfor %}
             {% for formset in formsets %}
                 {{ formset.as_p }}
             {% endfor %}

diff --git a/admin_confirm/templatetags/formatting.py b/admin_confirm/templatetags/formatting.py
@@ -1,5 +1,4 @@
 from django import template
-from django.db.models.query import QuerySet
 from django.utils.html import escape
 from django.utils.safestring import mark_safe
 
@@ -16,5 +15,5 @@ def format_change_data_field_value(field_value):
             output += "<li>" + escape(value) + "</li>"
         output += "</ul>"
         return mark_safe(output)
-    except:
+    except Exception:
         return field_value
diff --git a/admin_confirm/tests/README.md b/admin_confirm/tests/README.md
@@ -8,6 +8,12 @@ You seem concerned about the stability and reliability of this package. You're p
 
 So if you want to include this package in your production codebase, be aware that AdminConfirmMixin works best with simple unmodified ModelAdmins.
 
+# Probable Issues
+
+These are some areas which might/probably have issues that are not currently tested. Use at your own risk!
+
+- [ ] Saving file/image changes on inlines when confirming change on parent model
+
 ## Save Options
 
 - [x] Save
@@ -71,10 +77,10 @@ Confirmation on inline changes is not a current feature of this project.
 
 Confirmation on add/change of ModelAdmin that includes inlines needs to be tested. Use AdminConfirmMixin with ModelAdmin containing inlines at your own risk.
 
-- [ ] .inlines
-- [ ] .get_inline_instances()
-- [ ] .get_inlines() (New in Django 3.0)
-- [ ] .get_formsets_with_inlines()
+- [x] .inlines
+- [x] .get_inline_instances()
+- [x] .get_inlines() (New in Django 3.0)
+- [ ] .get_formsets_with_inlines() ???
 
 #### Options for inlines
 

diff --git a/admin_confirm/tests/helpers.py b/admin_confirm/tests/helpers.py
@@ -1,6 +1,12 @@
+import socket
+
 from django.core.cache import cache
 from django.test import TestCase, RequestFactory
 from django.contrib.auth.models import User
+from django.test import LiveServerTestCase
+from selenium import webdriver
+from selenium.webdriver.common.desired_capabilities import DesiredCapabilities
+
 
 class AdminConfirmTestCase(TestCase):
     """
@@ -62,23 +68,36 @@ def _assertFormsetsFormHtml(self, rendered_content, inlines):
                 self.assertIn("apple", rendered_content)
 
 
-
-import socket
-from django.test import LiveServerTestCase
-from selenium import webdriver
-from selenium.webdriver.common.desired_capabilities import DesiredCapabilities
-
-
 class AdminConfirmIntegrationTestCase(LiveServerTestCase):
     @classmethod
     def setUpClass(cls):
+
         cls.host = socket.gethostbyname(socket.gethostname())
         cls.selenium = webdriver.Remote(
             command_executor="http://selenium:4444/wd/hub",
             desired_capabilities=DesiredCapabilities.FIREFOX,
         )
         super().setUpClass()
 
+    def setUp(self):
+        self.superuser = User.objects.create_superuser(
+            username="super", email="[email protected]", password="pass"
+        )
+        self.client.force_login(self.superuser)
+
+        cookie = self.client.cookies["sessionid"]
+        self.selenium.get(
+            self.live_server_url + "/admin/"
+        )  # selenium will set cookie domain based on current page domain
+        self.selenium.add_cookie(
+            {"name": "sessionid", "value": cookie.value, "secure": False, "path": "/"}
+        )
+        return super().setUp()
+
+    def tearDown(self):
+        cache.clear()
+        return super().tearDown()
+
     @classmethod
     def tearDownClass(cls):
         cls.selenium.quit()

diff --git a/admin_confirm/tests/integration/test_smoke.py b/admin_confirm/tests/integration/test_smoke.py
@@ -3,5 +3,6 @@
 
 class SmokeTest(AdminConfirmIntegrationTestCase):
     def test_load_admin(self):
-        self.selenium.get(self.live_server_url+'/admin/')
-        self.assertIn('Django', self.selenium.title)
+        self.selenium.get(self.live_server_url + "/admin/")
+        self.assertIn("Django", self.selenium.title)
+        self.assertIn("Market", self.selenium.page_source)