From 14dc6268b0866cec6029dea6e29e849292931485 Mon Sep 17 00:00:00 2001 From: Thu Trang Pham Date: Wed, 28 Apr 2021 12:03:16 -0700 Subject: [PATCH] fix(Issue-27): Confirmations not triggered on models with validation failures (#30) * feat(ISSUE-27): Added some validation integration tests * feat(ISSUE-27): Integration tests for validations * feat(ISSUE-27): Unable to select autocomplete with selenium * feat(ISSUE-27): Fix file caching with proper FileCache * feat(ISSUE-27): Some minor tweaks * feat(ISSUE-27): Try reviewdog * feat(ISSUE-27): Remove noisy linting * feat(ISSUE-27): Black format * feat(ISSUE-27): Adding some simple file cache tests * feat(ISSUE-27): Some cleaning up Co-authored-by: Thu Trang Pham --- .github/workflows/lint.yml | 44 + .github/workflows/test.yml | 13 - Makefile | 6 + admin_confirm/admin.py | 93 +- admin_confirm/constants.py | 8 +- admin_confirm/file_cache.py | 110 ++ admin_confirm/tests/helpers.py | 22 + .../tests/integration/test_with_cache.py | 5 +- .../tests/integration/test_with_inlines.py | 4 +- .../test_with_validators_integration.py | 202 ++++ .../tests/unit/test_confirm_save_actions.py | 43 +- .../tests/unit/test_confirmation_cache.py | 22 +- .../test_confirmation_using_file_cache.py | 920 +++++++++++++++++ admin_confirm/tests/unit/test_file_cache.py | 942 +----------------- .../tests/unit/test_with_validators.py | 125 +-- admin_confirm/utils.py | 17 +- setup.cfg | 14 +- tests/market/admin/checkout_admin.py | 3 + tests/market/admin/shop_admin.py | 1 + .../0010_checkout_itemsale_transaction.py | 90 +- .../migrations/0012_auto_20210326_0240.py | 10 +- tests/market/models.py | 2 +- tests/test_project/settings/base.py | 5 +- 23 files changed, 1593 insertions(+), 1108 deletions(-) create mode 100644 .github/workflows/lint.yml create mode 100644 admin_confirm/file_cache.py create mode 100644 admin_confirm/tests/integration/test_with_validators_integration.py create mode 100644 admin_confirm/tests/unit/test_confirmation_using_file_cache.py diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml new file mode 100644 index 0000000..0f1fe60 --- /dev/null +++ b/.github/workflows/lint.yml @@ -0,0 +1,44 @@ +name: Lint + +on: + # Trigger the workflow on push or pull request, + # but only for the main branch + push: + branches: + - main + pull_request: + branches: + - main + release: + types: + - created + +jobs: + flake8-lint: + runs-on: ubuntu-latest + name: flake8 + steps: + - name: Check out source repository + uses: actions/checkout@v2 + - name: Set up Python environment + uses: actions/setup-python@v2 + with: + python-version: "3.8" + - name: flake8 Lint + uses: reviewdog/action-flake8@v3 + with: + github_token: ${{ secrets.GITHUB_TOKEN }} + workdir: admin_confirm + black-lint: + name: black + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + - uses: reviewdog/action-black@v2 + with: + github_token: ${{ secrets.github_token }} + # Change reviewdog reporter if you need [github-pr-check, github-check]. + reporter: github-pr-check + # Change reporter level if you need. + # GitHub Status Check won't become failure with a warning. + level: warning diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index d4fbd19..cf7ea1e 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -14,19 +14,6 @@ on: - created jobs: - lint: - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v2 - - name: wemake-python-styleguide - uses: wemake-services/wemake-python-styleguide@0.15.2 - with: - path: admin_confirm - reporter: 'github-pr-review' - env: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - continue-on-error: true - test: runs-on: ubuntu-latest strategy: diff --git a/Makefile b/Makefile index f8bf912..b0d2557 100644 --- a/Makefile +++ b/Makefile @@ -12,6 +12,12 @@ test-all: t: python -m pytest --last-failed -x +test-integration: + coverage run --source admin_confirm --branch -m pytest --ignore=admin_confirm/tests/unit + +docker-exec: + docker-compose exec -T web ${COMMAND} + check-readme: python -m readme_renderer README.md -o /tmp/README.html diff --git a/admin_confirm/admin.py b/admin_confirm/admin.py index 615de0e..6671610 100644 --- a/admin_confirm/admin.py +++ b/admin_confirm/admin.py @@ -1,6 +1,7 @@ from typing import Dict from django.contrib.admin.exceptions import DisallowedModelAdminToField from django.contrib.admin.utils import flatten_fieldsets, unquote +from django.core.cache import cache from django.core.exceptions import PermissionDenied from django.template.response import TemplateResponse from django.contrib.admin.options import TO_FIELD_VAR @@ -8,11 +9,14 @@ from django.utils.translation import gettext as _ from django.contrib.admin import helpers from django.db.models import Model, ManyToManyField, FileField, ImageField from django.forms import ModelForm -from admin_confirm.utils import get_admin_change_url, snake_to_title_case -from django.core.cache import cache +from admin_confirm.utils import ( + log, + get_admin_change_url, + snake_to_title_case, + format_cache_key, +) from django.views.decorators.cache import cache_control from admin_confirm.constants import ( - CACHE_TIMEOUT, CONFIRMATION_RECEIVED, CONFIRM_ADD, CONFIRM_CHANGE, @@ -21,7 +25,9 @@ from admin_confirm.constants import ( CACHE_KEYS, SAVE_AND_CONTINUE, SAVE_AS_NEW, + CACHE_TIMEOUT, ) +from admin_confirm.file_cache import FileCache class AdminConfirmMixin: @@ -38,6 +44,8 @@ class AdminConfirmMixin: change_confirmation_template = None action_confirmation_template = None + _file_cache = FileCache() + def get_confirmation_fields(self, request, obj=None): """ Hook for specifying confirmation fields @@ -100,6 +108,8 @@ class AdminConfirmMixin: if (not object_id and CONFIRM_ADD in request.POST) or ( object_id and CONFIRM_CHANGE in request.POST ): + log("confirmation is asked for") + self._file_cache.delete_all() cache.delete_many(CACHE_KEYS.values()) return self._change_confirmation_view( request, object_id, form_url, extra_context @@ -109,14 +119,21 @@ class AdminConfirmMixin: request, object_id, form_url, extra_context ) else: + self._file_cache.delete_all() cache.delete_many(CACHE_KEYS.values()) - extra_context = { + extra_context = self._add_confirmation_options_to_extra_context(extra_context) + return super().changeform_view(request, object_id, form_url, extra_context) + + def _add_confirmation_options_to_extra_context(self, extra_context): + log( + f"Adding confirmation to extra_content {self.confirm_add} {self.confirm_change}" + ) + return { **(extra_context or {}), "confirm_add": self.confirm_add, "confirm_change": self.confirm_change, } - return super().changeform_view(request, object_id, form_url, extra_context) def _get_changed_data( self, form: ModelForm, model: Model, obj: object, add: bool @@ -200,53 +217,58 @@ class AdminConfirmMixin: - or save the new object and modify the request from `add` to `change` and pass the request to Django """ + log("Confirmation has been received") def _reconstruct_request_files(): """ - Reconstruct the file(s) from the cached object (if any). + Reconstruct the file(s) from the file cache (if any). Returns a dictionary of field name to cached file """ reconstructed_files = {} cached_object = cache.get(CACHE_KEYS["object"]) - query_dict = cache.get(CACHE_KEYS["post"]) # Reconstruct the files from cached object if not cached_object: + log("Warning: no cached_object") return - if not query_dict: - # Use the current POST, since it should mirror cached POST - query_dict = request.POST - if type(cached_object) != self.model: # Do not use cache if the model doesn't match this model + log(f"Warning: cached_object is not of type {self.model}") return + query_dict = request.POST + for field in self.model._meta.get_fields(): if not (isinstance(field, FileField) or isinstance(field, ImageField)): continue - cached_file = getattr(cached_object, field.name) + cached_file = self._file_cache.get( + format_cache_key(model=self.model.__name__, field=field.name) + ) + # If a file was uploaded, the field is omitted from the POST since it's in request.FILES - if not query_dict.get(field.name) and cached_file: - reconstructed_files[field.name] = cached_file + if not query_dict.get(field.name): + if not cached_file: + log( + f"Warning: Could not find file cached for field {field.name}" + ) + else: + reconstructed_files[field.name] = cached_file return reconstructed_files reconstructed_files = _reconstruct_request_files() if reconstructed_files: + log(f"Found reconstructed files for fields: {reconstructed_files.keys()}") obj = None # remove the _confirm_add and _confirm_change from post modified_post = request.POST.copy() - cached_post = cache.get(CACHE_KEYS["post"]) - # No cover: __reconstruct_request_files currently checks for cached post so cached_post won't be None - if cached_post: # pragma: no cover - modified_post = cached_post.copy() if CONFIRM_ADD in modified_post: - del modified_post[CONFIRM_ADD] + del modified_post[CONFIRM_ADD] # pragma: no cover if CONFIRM_CHANGE in modified_post: - del modified_post[CONFIRM_CHANGE] + del modified_post[CONFIRM_CHANGE] # pragma: no cover if object_id and SAVE_AS_NEW not in request.POST: # Update the obj with the new uploaded files @@ -262,6 +284,7 @@ class AdminConfirmMixin: # No cover: __reconstruct_request_files currently checks for cached obj so obj won't be None if obj: # pragma: no cover for field, file in reconstructed_files.items(): + log(f"Setting file field {field} to file {file}") setattr(obj, field, file) obj.save() object_id = str(obj.id) @@ -283,7 +306,9 @@ class AdminConfirmMixin: request.POST = modified_post + self._file_cache.delete_all() cache.delete_many(CACHE_KEYS.values()) + return super()._changeform_view(request, object_id, form_url, extra_context) def _get_cleared_fields(self, request): @@ -312,6 +337,9 @@ class AdminConfirmMixin: model = self.model opts = model._meta + if SAVE_AS_NEW in request.POST: + object_id = None + add = object_id is None if add: if not self.has_add_permission(request): @@ -331,7 +359,7 @@ class AdminConfirmMixin: request, obj, change=not add, fields=flatten_fieldsets(fieldsets) ) - form = ModelForm(request.POST, request.FILES, obj) + form = ModelForm(request.POST, request.FILES, instance=obj) form_validated = form.is_valid() if form_validated: new_object = self.save_form(request, form, change=not add) @@ -341,6 +369,16 @@ class AdminConfirmMixin: request, new_object, change=not add ) # End code from super()._changeform_view + # form.is_valid() checks both errors and "is_bound" + # If form has errors, show the errors on the form instead of showing confirmation page + if not form_validated: + log("Invalid Form: return early") + log(form.errors) + # We must ensure that we ask for confirmation when showing errors + extra_context = self._add_confirmation_options_to_extra_context( + extra_context + ) + return super()._changeform_view(request, object_id, form_url, extra_context) add_or_new = add or SAVE_AS_NEW in request.POST # Get changed data to show on confirmation @@ -350,6 +388,7 @@ class AdminConfirmMixin: self.get_confirmation_fields(request, obj) ) & set(changed_data.keys()) if not bool(changed_confirmation_fields): + log("No change detected") # No confirmation required for changed fields, continue to save return super()._changeform_view(request, object_id, form_url, extra_context) @@ -363,12 +402,20 @@ class AdminConfirmMixin: 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) + log("Caching files") + cache.set(CACHE_KEYS["object"], new_object, CACHE_TIMEOUT) + + # Save files as tempfiles + for field_name in request.FILES: + file = request.FILES[field_name] + self._file_cache.set( + format_cache_key(model=model.__name__, field=field_name), file + ) # Handle when files are cleared - since the `form` object would not hold that info cleared_fields = self._get_cleared_fields(request) + log("Render Change Confirmation") title_action = _("adding") if add_or_new else _("changing") context = { **self.admin_site.each_context(request), diff --git a/admin_confirm/constants.py b/admin_confirm/constants.py index b062cd4..95de6b2 100644 --- a/admin_confirm/constants.py +++ b/admin_confirm/constants.py @@ -10,8 +10,14 @@ CONFIRM_ADD = "_confirm_add" CONFIRM_CHANGE = "_confirm_change" CONFIRMATION_RECEIVED = "_confirmation_received" -CACHE_TIMEOUT = getattr(settings, "ADMIN_CONFIRM_CACHE_TIMEOUT", 10) +CACHE_TIMEOUT = getattr(settings, "ADMIN_CONFIRM_CACHE_TIMEOUT", 1000) CACHE_KEYS = { "object": "admin_confirm__confirmation_object", "post": "admin_confirm__confirmation_request_post", } +CACHE_KEY_PREFIX = getattr( + settings, "ADMIN_CONFIRM_CACHE_KEY_PREFIX", "admin_confirm__file_cache" +) + + +DEBUG = getattr(settings, "ADMIN_CONFIRM_DEBUG", False) diff --git a/admin_confirm/file_cache.py b/admin_confirm/file_cache.py new file mode 100644 index 0000000..7baaeff --- /dev/null +++ b/admin_confirm/file_cache.py @@ -0,0 +1,110 @@ +""" FileCache - caches files for ModelAdmins with confirmations. + +Code modified from: https://github.com/MaistrenkoAnton/filefield-cache/blob/master/filefield_cache/cache.py +Original copy date: April 22, 2021 +--- +Modified to be compatible with more versions of Django/Python +--- +MIT License + +Copyright (c) 2020 Maistrenko Anton + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in all +copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +SOFTWARE. +""" +from django.core.files.uploadedfile import InMemoryUploadedFile + +try: + from cStringIO import StringIO as BytesIO # noqa: WPS433 +except ImportError: + from io import BytesIO # noqa: WPS433, WPS440 + +from django.core.cache import cache + +from admin_confirm.constants import CACHE_TIMEOUT +from admin_confirm.utils import log + + +class FileCache(object): + "Cache file data and retain the file upon confirmation." + + timeout = CACHE_TIMEOUT + + def __init__(self): + self.cache = cache + self.cached_keys = [] + + def set(self, key, upload): + """ + Set file data to cache for 1000s + + :param key: cache key + :param upload: file data + """ + try: # noqa: WPS229 + state = { + "name": upload.name, + "size": upload.size, + "content_type": upload.content_type, + "charset": upload.charset, + "content": upload.file.read(), + } + upload.file.seek(0) + self.cache.set(key, state, self.timeout) + log(f"Setting file cache with {key}") + self.cached_keys.append(key) + except AttributeError: # pragma: no cover + pass # noqa: WPS420 + + def get(self, key): + """ + Get the file data from cache using specific cache key + + :param key: cache key + :return: File data + """ + upload = None + state = self.cache.get(key) + if state: + file = BytesIO() + file.write(state["content"]) + upload = InMemoryUploadedFile( + file=file, + field_name="file", + name=state["name"], + content_type=state["content_type"], + size=state["size"], + charset=state["charset"], + ) + upload.file.seek(0) + log(f"Getting file cache with {key}") + return upload + + def delete(self, key): + """ + Delete file data from cache + + :param key: cache key + """ + self.cache.delete(key) + self.cached_keys.remove(key) + + def delete_all(self): + "Delete all cached file data from cache." + self.cache.delete_many(self.cached_keys) + self.cached_keys = [] diff --git a/admin_confirm/tests/helpers.py b/admin_confirm/tests/helpers.py index 666a1b3..23f3925 100644 --- a/admin_confirm/tests/helpers.py +++ b/admin_confirm/tests/helpers.py @@ -6,6 +6,8 @@ from django.contrib.auth.models import User from django.test import LiveServerTestCase from selenium import webdriver from selenium.webdriver.common.desired_capabilities import DesiredCapabilities +from selenium.webdriver.support.ui import Select +from selenium.webdriver.common.by import By class AdminConfirmTestCase(TestCase): @@ -69,6 +71,7 @@ class AdminConfirmTestCase(TestCase): class AdminConfirmIntegrationTestCase(LiveServerTestCase): + # Setup/Teardown Methods @classmethod def setUpClass(cls): cls.host = socket.gethostbyname(socket.gethostname()) @@ -102,3 +105,22 @@ class AdminConfirmIntegrationTestCase(LiveServerTestCase): def tearDownClass(cls): cls.selenium.quit() super().tearDownClass() + + # Helper Methods + + def set_value(self, by, by_value, value): + element = self.selenium.find_element(by, by_value) + element.clear() + element.send_keys(value) + + def select_value(self, by, by_value, value): + element = Select(self.selenium.find_element(by, by_value)) + element.select_by_value(value) + + def select_index(self, by, by_value, index): + element = Select(self.selenium.find_element(by, by_value)) + element.select_by_index(index) + + def print_hidden_form(self): + hidden_form = self.selenium.find_element(By.ID, "hidden-form") + print(hidden_form.get_attribute("innerHTML")) diff --git a/admin_confirm/tests/integration/test_with_cache.py b/admin_confirm/tests/integration/test_with_cache.py index 246a5d9..dfe5def 100644 --- a/admin_confirm/tests/integration/test_with_cache.py +++ b/admin_confirm/tests/integration/test_with_cache.py @@ -68,6 +68,7 @@ class ConfirmWithInlinesTests(AdminConfirmIntegrationTestCase): # Change price price = self.selenium.find_element(By.NAME, "price") price.send_keys(2) + self.selenium.find_element(By.ID, "id_currency_0").click() self.selenium.find_element(By.NAME, "_continue").click() @@ -120,7 +121,7 @@ class ConfirmWithInlinesTests(AdminConfirmIntegrationTestCase): item.refresh_from_db() self.assertEqual(21, int(item.price)) - self.assertIn("screenshot.png", item.file.name) + self.assertRegex(item.file.name, r"screenshot.*\.png$") def test_should_save_file_changes(self): selenium_version = pkg_resources.get_distribution("selenium").parsed_version @@ -165,7 +166,7 @@ class ConfirmWithInlinesTests(AdminConfirmIntegrationTestCase): item.refresh_from_db() self.assertEqual(21, int(item.price)) - self.assertIn("screenshot.png", item.file.name) + self.assertRegex(item.file.name, r"screenshot.*\.png$") def test_should_remove_file_if_clear_selected(self): file = SimpleUploadedFile( diff --git a/admin_confirm/tests/integration/test_with_inlines.py b/admin_confirm/tests/integration/test_with_inlines.py index 04744a8..9aef829 100644 --- a/admin_confirm/tests/integration/test_with_inlines.py +++ b/admin_confirm/tests/integration/test_with_inlines.py @@ -69,14 +69,14 @@ class ConfirmWithInlinesTests(AdminConfirmIntegrationTestCase): # Make a change to trigger confirmation page name = self.selenium.find_element(By.NAME, "name") - name.send_keys("New Name") + name.send_keys("This is New Name") self.selenium.find_element(By.NAME, "_continue").click() self.assertIn("Confirm", self.selenium.page_source) - hidden_form = self.selenium.find_element(By.ID, "hidden-form") hidden_form.find_element(By.NAME, "ShoppingMall_shops-TOTAL_FORMS") + self.selenium.find_element(By.NAME, "_continue").click() mall.refresh_from_db() diff --git a/admin_confirm/tests/integration/test_with_validators_integration.py b/admin_confirm/tests/integration/test_with_validators_integration.py new file mode 100644 index 0000000..57ce837 --- /dev/null +++ b/admin_confirm/tests/integration/test_with_validators_integration.py @@ -0,0 +1,202 @@ +""" +Ensures that confirmations work with validators on the Model and on the Modelform. +""" + +from logging import currentframe +from unittest import mock +from django.urls import reverse +from django.utils import timezone + +from admin_confirm.tests.helpers import AdminConfirmTestCase +from tests.market.models import Checkout, ItemSale +from tests.factories import ( + InventoryFactory, + ItemFactory, + ShopFactory, + TransactionFactory, +) + + +import pytest +import pkg_resources +from importlib import reload +from tests.factories import ShopFactory +from tests.market.models import GeneralManager, ShoppingMall, Town + +from admin_confirm.tests.helpers import AdminConfirmIntegrationTestCase +from tests.market.admin import shoppingmall_admin + +from admin_confirm.constants import CONFIRM_ADD, CONFIRM_CHANGE +from selenium.webdriver.support.ui import Select +from selenium.webdriver.common.by import By + + +class ConfirmWithValidatorsTests(AdminConfirmIntegrationTestCase): + def setUp(self): + self.admin = shoppingmall_admin.ShoppingMallAdmin + self.admin.inlines = [shoppingmall_admin.ShopInline] + super().setUp() + + def tearDown(self): + reload(shoppingmall_admin) + super().tearDown() + + @mock.patch("tests.market.models.ItemSale.clean") + def test_can_confirm_for_models_with_validator_on_model_field(self, _mock_clean): + # ItemSale.currency has a validator on it + ItemFactory() + TransactionFactory() + + self.selenium.get(self.live_server_url + f"/admin/market/itemsale/add/") + # Should ask for confirmation of change + self.assertIn(CONFIRM_ADD, self.selenium.page_source) + + self.set_value(by=By.NAME, by_value="quantity", value="1") + self.set_value(by=By.NAME, by_value="total", value="10.00") + self.set_value(by=By.NAME, by_value="currency", value="USD") + self.select_index(by=By.NAME, by_value="transaction", index=1) + self.select_index(by=By.NAME, by_value="item", index=1) + + self.selenium.find_element(By.NAME, "_continue").click() + + # Should have hidden form containing the updated name + self.assertIn("Confirm", self.selenium.page_source) + hidden_form = self.selenium.find_element(By.ID, "hidden-form") + currency = hidden_form.find_element(By.NAME, "currency") + self.assertIn("USD", currency.get_attribute("value")) + + # Should not have been added yet + self.assertEqual(ItemSale.objects.count(), 0) + self.selenium.find_element(By.NAME, "_continue").click() + + # Should persist change + self.assertEqual(ItemSale.objects.count(), 1) + + def test_cannot_confirm_for_models_with_validator_on_model_field_if_validator_fails( + self, + ): + # ItemSale.currency has a validator on it + shop = ShopFactory() + item = ItemFactory() + InventoryFactory(shop=shop, item=item, quantity=10) + TransactionFactory(shop=shop) + + self.selenium.get(self.live_server_url + f"/admin/market/itemsale/add/") + # Should ask for confirmation of change + self.assertIn(CONFIRM_ADD, self.selenium.page_source) + + self.set_value(by=By.NAME, by_value="quantity", value="1") + self.set_value(by=By.NAME, by_value="total", value="10.00") + self.set_value(by=By.NAME, by_value="currency", value="FAKE") + self.select_index(by=By.NAME, by_value="transaction", index=1) + self.select_index(by=By.NAME, by_value="item", index=1) + + self.selenium.find_element(By.NAME, "_continue").click() + + # Should show errors and not confirmation page + self.assertNotIn("Confirm", self.selenium.page_source) + self.assertIn("Invalid Currency", self.selenium.page_source) + self.assertIn(CONFIRM_ADD, self.selenium.page_source) + + # Should not have been added yet + self.assertEqual(ItemSale.objects.count(), 0) + + # Now fix the issue + self.set_value(by=By.NAME, by_value="currency", value="USD") + self.selenium.find_element(By.NAME, "_continue").click() + + # Should have hidden form containing the updated currency + self.assertIn("Confirm", self.selenium.page_source) + hidden_form = self.selenium.find_element(By.ID, "hidden-form") + currency = hidden_form.find_element(By.NAME, "currency") + self.assertIn("USD", currency.get_attribute("value")) + + # Should not have been added yet + self.assertEqual(ItemSale.objects.count(), 0) + self.selenium.find_element(By.NAME, "_continue").click() + + # Should persist change + self.assertEqual(ItemSale.objects.count(), 1) + + def test_can_confirm_for_models_with_clean_overridden(self): + shop = ShopFactory() + item = ItemFactory() + InventoryFactory(shop=shop, item=item, quantity=10) + transaction = TransactionFactory(shop=shop) + + self.selenium.get(self.live_server_url + f"/admin/market/itemsale/add/") + # Should ask for confirmation of change + self.assertIn(CONFIRM_ADD, self.selenium.page_source) + + self.set_value(by=By.NAME, by_value="quantity", value="9") + self.set_value(by=By.NAME, by_value="total", value="10.00") + self.set_value(by=By.NAME, by_value="currency", value="USD") + self.select_index(by=By.NAME, by_value="transaction", index=1) + self.select_index(by=By.NAME, by_value="item", index=1) + + self.selenium.find_element(By.NAME, "_continue").click() + + # Should not have been added yet + self.assertEqual(ItemSale.objects.count(), 0) + + # Should have hidden form containing the updated currency + self.assertIn("Confirm", self.selenium.page_source) + hidden_form = self.selenium.find_element(By.ID, "hidden-form") + currency = hidden_form.find_element(By.NAME, "currency") + self.assertIn("USD", currency.get_attribute("value")) + + # Confirm the change + self.selenium.find_element(By.NAME, "_continue").click() + + # Should persist change + self.assertEqual(ItemSale.objects.count(), 1) + + # Ensure that the date and timestamp saved correctly + item_sale = ItemSale.objects.first() + self.assertEqual(item_sale.transaction, transaction) + self.assertEqual(item_sale.item, item) + self.assertEqual(item_sale.currency, "USD") + + def test_cannot_confirm_for_models_with_clean_overridden_if_clean_fails(self): + shop = ShopFactory() + item = ItemFactory() + InventoryFactory(shop=shop, item=item, quantity=1) + TransactionFactory(shop=shop) + + self.selenium.get(self.live_server_url + f"/admin/market/itemsale/add/") + # Should ask for confirmation of change + self.assertIn(CONFIRM_ADD, self.selenium.page_source) + + self.set_value(by=By.NAME, by_value="quantity", value="9") + self.set_value(by=By.NAME, by_value="total", value="10.00") + self.set_value(by=By.NAME, by_value="currency", value="USD") + self.select_index(by=By.NAME, by_value="transaction", index=1) + self.select_index(by=By.NAME, by_value="item", index=1) + + self.selenium.find_element(By.NAME, "_continue").click() + + # Should show errors and not confirmation page + self.assertNotIn("Confirm", self.selenium.page_source) + self.assertIn( + "Shop does not have enough of the item stocked", self.selenium.page_source + ) + self.assertIn(CONFIRM_ADD, self.selenium.page_source) + + # Should not have been added yet + self.assertEqual(ItemSale.objects.count(), 0) + + # Now fix the issue + self.set_value(by=By.NAME, by_value="quantity", value="1") + self.selenium.find_element(By.NAME, "_continue").click() + + # Should have hidden form containing the updated currency + self.assertIn("Confirm", self.selenium.page_source) + hidden_form = self.selenium.find_element(By.ID, "hidden-form") + quantity = hidden_form.find_element(By.NAME, "quantity") + self.assertIn("1", str(quantity.get_attribute("value"))) + + # Confirm change + self.selenium.find_element(By.NAME, "_continue").click() + + # Should persist change + self.assertEqual(ItemSale.objects.count(), 1) diff --git a/admin_confirm/tests/unit/test_confirm_save_actions.py b/admin_confirm/tests/unit/test_confirm_save_actions.py index f50d99a..dca9be3 100644 --- a/admin_confirm/tests/unit/test_confirm_save_actions.py +++ b/admin_confirm/tests/unit/test_confirm_save_actions.py @@ -102,10 +102,6 @@ class TestConfirmSaveActions(AdminConfirmTestCase): # Should have cached the unsaved item cached_item = cache.get(CACHE_KEYS["object"]) self.assertIsNotNone(cached_item) - self.assertIsNone(cached_item.id) - self.assertEqual(cached_item.name, data["name"]) - self.assertEqual(cached_item.price, data["price"]) - self.assertEqual(cached_item.currency, data["currency"]) # Should not have saved the changes yet self.assertEqual(Item.objects.count(), 1) @@ -171,16 +167,6 @@ class TestConfirmSaveActions(AdminConfirmTestCase): multipart_form=True, ) - # Should have cached the unsaved item - cached_item = cache.get(CACHE_KEYS["object"]) - self.assertIsNotNone(cached_item) - self.assertIsNone(cached_item.id) - self.assertEqual(cached_item.name, data["name"]) - self.assertEqual(cached_item.price, data["price"]) - self.assertEqual(cached_item.currency, data["currency"]) - self.assertEqual(cached_item.file, data["file"]) - self.assertEqual(cached_item.image, data["image"]) - # Should not have saved the item yet self.assertEqual(Item.objects.count(), 0) @@ -205,13 +191,14 @@ class TestConfirmSaveActions(AdminConfirmTestCase): self.assertEqual(saved_item.name, data["name"]) self.assertEqual(saved_item.price, data["price"]) self.assertEqual(saved_item.currency, data["currency"]) - self.assertEqual(saved_item.file, data["file"]) - self.assertEqual(saved_item.image, data["image"]) + self.assertIsNotNone(saved_item.file) + self.assertIsNotNone(saved_item.image) - self.assertEqual(saved_item.file.name, "test_file.jpg") - self.assertEqual(saved_item.image.name, "test_image.jpg") + self.assertRegex(saved_item.file.name, r"test_file.*\.jpg$") + self.assertRegex(saved_item.image.name, r"test_image.*\.jpg$") # Should have cleared cache + self.assertEqual(len(ItemAdmin._file_cache.cached_keys), 0) for key in CACHE_KEYS.values(): self.assertIsNone(cache.get(key)) @@ -271,16 +258,6 @@ class TestConfirmSaveActions(AdminConfirmTestCase): multipart_form=True, ) - # Should have cached the unsaved item - cached_item = cache.get(CACHE_KEYS["object"]) - self.assertIsNotNone(cached_item) - self.assertIsNone(cached_item.id) - self.assertEqual(cached_item.name, data["name"]) - self.assertEqual(cached_item.price, data["price"]) - self.assertEqual(cached_item.currency, data["currency"]) - self.assertFalse(cached_item.file.name) - self.assertEqual(cached_item.image, i2) - # Should not have saved the changes yet self.assertEqual(Item.objects.count(), 1) item.refresh_from_db() @@ -312,11 +289,11 @@ class TestConfirmSaveActions(AdminConfirmTestCase): self.assertEqual(new_item.price, data["price"]) self.assertEqual(new_item.currency, data["currency"]) self.assertFalse(new_item.file) - self.assertEqual(new_item.image, i2) + self.assertIsNotNone(new_item.image) + self.assertRegex(new_item.image.name, r"test_image2.*\.jpg$") # Should have cleared cache - for key in CACHE_KEYS.values(): - self.assertIsNone(cache.get(key)) + self.assertEqual(len(ItemAdmin._file_cache.cached_keys), 0) def test_relations_add(self): gm = GeneralManager.objects.create(name="gm") @@ -379,6 +356,7 @@ class TestConfirmSaveActions(AdminConfirmTestCase): self.assertIn(shop, shops) # Should have cleared cache + self.assertEqual(len(ItemAdmin._file_cache.cached_keys), 0) for key in CACHE_KEYS.values(): self.assertIsNone(cache.get(key)) @@ -473,5 +451,4 @@ class TestConfirmSaveActions(AdminConfirmTestCase): self.assertIn(shop, shops2) # Should have cleared cache - for key in CACHE_KEYS.values(): - self.assertIsNone(cache.get(key)) + self.assertEqual(len(ItemAdmin._file_cache.cached_keys), 0) diff --git a/admin_confirm/tests/unit/test_confirmation_cache.py b/admin_confirm/tests/unit/test_confirmation_cache.py index 50d8e87..b3b2d69 100644 --- a/admin_confirm/tests/unit/test_confirmation_cache.py +++ b/admin_confirm/tests/unit/test_confirmation_cache.py @@ -102,10 +102,6 @@ class TestConfirmationCache(AdminConfirmTestCase): # Should have cached the unsaved item cached_item = cache.get(CACHE_KEYS["object"]) self.assertIsNotNone(cached_item) - self.assertIsNone(cached_item.id) - self.assertEqual(cached_item.name, data["name"]) - self.assertEqual(cached_item.price, data["price"]) - self.assertEqual(cached_item.currency, data["currency"]) # Should not have saved the changes yet self.assertEqual(Item.objects.count(), 1) @@ -204,11 +200,11 @@ class TestConfirmationCache(AdminConfirmTestCase): self.assertEqual(saved_item.name, data["name"]) self.assertEqual(saved_item.price, data["price"]) self.assertEqual(saved_item.currency, data["currency"]) - self.assertEqual(saved_item.file, data["file"]) - self.assertEqual(saved_item.image, data["image"]) + self.assertIsNotNone(saved_item.file) + self.assertIsNotNone(saved_item.image) - self.assertEqual(saved_item.file.name, "test_file.jpg") - self.assertEqual(saved_item.image.name, "test_image.jpg") + self.assertRegex(saved_item.file.name, r"test_file.*\.jpg$") + self.assertRegex(saved_item.image.name, r"test_image.*\.jpg$") # Should have cleared cache for key in CACHE_KEYS.values(): @@ -271,12 +267,6 @@ class TestConfirmationCache(AdminConfirmTestCase): # Should have cached the unsaved item cached_item = cache.get(CACHE_KEYS["object"]) self.assertIsNotNone(cached_item) - self.assertIsNone(cached_item.id) - self.assertEqual(cached_item.name, data["name"]) - self.assertEqual(cached_item.price, data["price"]) - self.assertEqual(cached_item.currency, data["currency"]) - self.assertFalse(cached_item.file.name) - self.assertEqual(cached_item.image, i2) # Should not have saved the changes yet self.assertEqual(Item.objects.count(), 1) @@ -301,7 +291,9 @@ class TestConfirmationCache(AdminConfirmTestCase): self.assertEqual(saved_item.price, data["price"]) self.assertEqual(saved_item.currency, data["currency"]) self.assertFalse(saved_item.file) - self.assertEqual(saved_item.image, i2) + self.assertIsNotNone(saved_item.image) + + self.assertRegex(saved_item.image.name, r"test_image2.*\.jpg$") # Should have cleared cache for key in CACHE_KEYS.values(): diff --git a/admin_confirm/tests/unit/test_confirmation_using_file_cache.py b/admin_confirm/tests/unit/test_confirmation_using_file_cache.py new file mode 100644 index 0000000..1cc4eed --- /dev/null +++ b/admin_confirm/tests/unit/test_confirmation_using_file_cache.py @@ -0,0 +1,920 @@ +""" +Ensure that files are saved during confirmation +Without file changes, Django is relied on + +With file changes, we cache the object, save it with +the files if new, or add files to existing obj and save + +Then send the rest of the changes to Django to handle + +This is arguably the most we fiddle with the Django request +Thus we should test it extensively +""" +from admin_confirm.utils import format_cache_key +from admin_confirm.file_cache import FileCache +import time +from unittest import mock + +from django.core.files.uploadedfile import SimpleUploadedFile +from django.core.cache import cache + +from admin_confirm.tests.helpers import AdminConfirmTestCase +from admin_confirm.constants import CACHE_KEYS, CONFIRMATION_RECEIVED + +from tests.market.admin import ItemAdmin +from tests.market.models import Item, Shop +from tests.factories import ItemFactory, ShopFactory + + +class TestConfirmationUsingFileCache(AdminConfirmTestCase): + def setUp(self): + # Load the Change Item Page + ItemAdmin.confirm_change = True + ItemAdmin.fields = ["name", "price", "file", "image", "currency"] + ItemAdmin.save_as = True + ItemAdmin.save_as_continue = True + + self.image_path = "screenshot.png" + f = SimpleUploadedFile( + name="test_file.jpg", + content=open(self.image_path, "rb").read(), + content_type="image/jpeg", + ) + i = SimpleUploadedFile( + name="test_image.jpg", + content=open(self.image_path, "rb").read(), + content_type="image/jpeg", + ) + self.item = ItemFactory(name="Not name", file=f, image=i) + + return super().setUp() + + def test_save_as_continue_true_should_not_redirect_to_changelist(self): + item = self.item + # Load the Change Item Page + ItemAdmin.save_as_continue = True + + # Upload new image and remove file + i2 = SimpleUploadedFile( + name="test_image2.jpg", + content=open(self.image_path, "rb").read(), + content_type="image/jpeg", + ) + # Request.POST + data = { + "id": item.id, + "name": "name", + "price": 2.0, + "file": "", + "file-clear": "on", + "currency": Item.VALID_CURRENCIES[0][0], + "_confirm_change": True, + "_saveasnew": True, + } + + # Set cache + cache_item = Item( + name=data["name"], + price=data["price"], + currency=data["currency"], + image=i2, + ) + file_cache = FileCache() + file_cache.set(format_cache_key(model="Item", field="image"), i2) + + cache.set(CACHE_KEYS["object"], cache_item) + cache.set(CACHE_KEYS["post"], data) + + # Click "Yes, I'm Sure" + del data["_confirm_change"] + data[CONFIRMATION_RECEIVED] = True + + with mock.patch.object(ItemAdmin, "message_user") as message_user: + response = self.client.post( + f"/admin/market/item/{self.item.id}/change/", data=data + ) + # Should show message to user with correct obj and path + message_user.assert_called_once() + message = message_user.call_args[0][1] + self.assertIn("/admin/market/item/2/change/", message) + self.assertIn(data["name"], message) + self.assertIn("You may edit it again below.", message) + + # Should not have redirected to changelist + self.assertEqual(response.url, f"/admin/market/item/{self.item.id + 1}/change/") + + # Should not have changed existing item + item.refresh_from_db() + self.assertEqual(item.name, "Not name") + self.assertEqual(item.file.name.count("test_file"), 1) + self.assertEqual(item.image.name.count("test_image2"), 0) + self.assertEqual(item.image.name.count("test_image"), 1) + + # Should have saved new item + self.assertEqual(Item.objects.count(), 2) + new_item = Item.objects.filter(id=item.id + 1).first() + self.assertIsNotNone(new_item) + self.assertEqual(new_item.name, data["name"]) + self.assertEqual(new_item.price, data["price"]) + self.assertEqual(new_item.currency, data["currency"]) + self.assertFalse(new_item.file) + self.assertEqual(new_item.image.name.count("test_image2"), 1) + + # Should have cleared cache + for key in CACHE_KEYS.values(): + self.assertIsNone(cache.get(key)) + + def test_save_as_continue_false_should_redirect_to_changelist(self): + item = self.item + # Load the Change Item Page + ItemAdmin.save_as_continue = False + + # Upload new image and remove file + i2 = SimpleUploadedFile( + name="test_image2.jpg", + content=open(self.image_path, "rb").read(), + content_type="image/jpeg", + ) + # Request.POST + data = { + "id": item.id, + "name": "name", + "price": 2.0, + "file": "", + "file-clear": "on", + "currency": Item.VALID_CURRENCIES[0][0], + "_confirm_change": True, + "_saveasnew": True, + } + + # Set cache + cache_item = Item( + name=data["name"], + price=data["price"], + currency=data["currency"], + image=i2, + ) + file_cache = FileCache() + file_cache.set(format_cache_key(model="Item", field="image"), i2) + + cache.set(CACHE_KEYS["object"], cache_item) + cache.set(CACHE_KEYS["post"], data) + + # Click "Yes, I'm Sure" + del data["_confirm_change"] + data[CONFIRMATION_RECEIVED] = True + + with mock.patch.object(ItemAdmin, "message_user") as message_user: + response = self.client.post( + f"/admin/market/item/{self.item.id}/change/", data=data + ) + # Should show message to user with correct obj and path + message_user.assert_called_once() + message = message_user.call_args[0][1] + self.assertIn("/admin/market/item/2/change/", message) + self.assertIn(data["name"], message) + self.assertNotIn("You may edit it again below.", message) + + # Should have redirected to changelist + self.assertEqual(response.url, "/admin/market/item/") + + # Should not have changed existing item + item.refresh_from_db() + self.assertEqual(item.name, "Not name") + self.assertEqual(item.file.name.count("test_file"), 1) + self.assertEqual(item.image.name.count("test_image2"), 0) + self.assertEqual(item.image.name.count("test_image"), 1) + + # Should have saved new item + self.assertEqual(Item.objects.count(), 2) + new_item = Item.objects.filter(id=item.id + 1).first() + self.assertIsNotNone(new_item) + self.assertEqual(new_item.name, data["name"]) + self.assertEqual(new_item.price, data["price"]) + self.assertEqual(new_item.currency, data["currency"]) + self.assertFalse(new_item.file) + self.assertEqual(new_item.image.name.count("test_image2"), 1) + + # Should have cleared cache + for key in CACHE_KEYS.values(): + self.assertIsNone(cache.get(key)) + + def test_saveasnew_without_any_file_changes_should_save_new_instance_without_files( + self, + ): + item = self.item + + # Request.POST + data = { + "id": item.id, + "name": "name", + "price": 2.0, + "file": "", + "image": "", + "currency": Item.VALID_CURRENCIES[0][0], + "_confirm_change": True, + "_saveasnew": True, + } + + # Set cache + cache_item = Item( + name=data["name"], + price=data["price"], + currency=data["currency"], + ) + + cache.set(CACHE_KEYS["object"], cache_item) + cache.set(CACHE_KEYS["post"], data) + + # Click "Yes, I'm Sure" + del data["_confirm_change"] + data[CONFIRMATION_RECEIVED] = True + + with mock.patch.object(ItemAdmin, "message_user") as message_user: + response = self.client.post( + f"/admin/market/item/{self.item.id}/change/", data=data + ) + # Should show message to user with correct obj and path + message_user.assert_called_once() + message = message_user.call_args[0][1] + self.assertIn("/admin/market/item/2/change/", message) + self.assertIn(data["name"], message) + self.assertIn("You may edit it again below.", message) + + # Should not have redirected to changelist + self.assertEqual(response.url, f"/admin/market/item/{self.item.id + 1}/change/") + + # Should not have changed existing item + item.refresh_from_db() + self.assertEqual(item.name, "Not name") + self.assertEqual(item.file.name.count("test_file"), 1) + self.assertEqual(item.image.name.count("test_image2"), 0) + self.assertEqual(item.image.name.count("test_image"), 1) + + # Should have saved new item + self.assertEqual(Item.objects.count(), 2) + new_item = Item.objects.filter(id=item.id + 1).first() + self.assertIsNotNone(new_item) + self.assertEqual(new_item.name, data["name"]) + self.assertEqual(new_item.price, data["price"]) + self.assertEqual(new_item.currency, data["currency"]) + # In Django (by default), the save as new does not transfer over the files + self.assertFalse(new_item.file) + self.assertFalse(new_item.image) + + # Should have cleared cache + for key in CACHE_KEYS.values(): + self.assertIsNone(cache.get(key)) + + def test_add_with_upload_file_should_save_new_instance_with_files(self): + # Upload new file + f2 = SimpleUploadedFile( + name="test_file2.jpg", + content=open(self.image_path, "rb").read(), + content_type="image/jpeg", + ) + + # Request.POST + data = { + "name": "name", + "price": 2.0, + "image": "", + "file": f2, + "currency": Item.VALID_CURRENCIES[0][0], + "_confirm_add": True, + "_save": True, + } + + # Set cache + cache_item = Item( + name=data["name"], price=data["price"], currency=data["currency"], file=f2 + ) + + cache.set(CACHE_KEYS["object"], cache_item) + cache.set(CACHE_KEYS["post"], data) + + # Click "Yes, I'm Sure" + del data["_confirm_add"] + data[CONFIRMATION_RECEIVED] = True + + with mock.patch.object(ItemAdmin, "message_user") as message_user: + response = self.client.post("/admin/market/item/add/", data=data) + # Should show message to user with correct obj and path + message_user.assert_called_once() + message = message_user.call_args[0][1] + self.assertIn("/admin/market/item/2/change/", message) + self.assertIn(data["name"], message) + self.assertNotIn("You may edit it again below.", message) + + # Should not have redirected to changelist + self.assertEqual(response.url, "/admin/market/item/") + + # Should not have changed existing item + self.item.refresh_from_db() + self.assertEqual(self.item.name, "Not name") + self.assertEqual(self.item.file.name.count("test_file"), 1) + self.assertEqual(self.item.image.name.count("test_image2"), 0) + self.assertEqual(self.item.image.name.count("test_image"), 1) + + # Should have saved new item + self.assertEqual(Item.objects.count(), 2) + new_item = Item.objects.filter(id=self.item.id + 1).first() + self.assertIsNotNone(new_item) + self.assertEqual(new_item.name, data["name"]) + self.assertEqual(new_item.price, data["price"]) + self.assertEqual(new_item.currency, data["currency"]) + self.assertIsNotNone(new_item.file) + self.assertFalse(new_item.image) + + self.assertRegex(new_item.file.name, r"test_file2.*\.jpg$") + + # Should have cleared cache + for key in CACHE_KEYS.values(): + self.assertIsNone(cache.get(key)) + + def test_add_without_cached_post_should_save_new_instance_with_file(self): + # Upload new file + f2 = SimpleUploadedFile( + name="test_file2.jpg", + content=open(self.image_path, "rb").read(), + content_type="image/jpeg", + ) + + # Request.POST + data = { + "name": "name", + "price": 2.0, + "image": "", + "file": f2, + "currency": Item.VALID_CURRENCIES[0][0], + "_confirm_add": True, + "_save": True, + } + + # Set cache + cache_item = Item( + name=data["name"], price=data["price"], currency=data["currency"], file=f2 + ) + + cache.set(CACHE_KEYS["object"], cache_item) + # Make sure there's no post cached post + cache.delete(CACHE_KEYS["post"]) + + # Click "Yes, I'm Sure" + del data["_confirm_add"] + data[CONFIRMATION_RECEIVED] = True + + with mock.patch.object(ItemAdmin, "message_user") as message_user: + response = self.client.post("/admin/market/item/add/", data=data) + # Should show message to user with correct obj and path + message_user.assert_called_once() + message = message_user.call_args[0][1] + self.assertIn("/admin/market/item/2/change/", message) + self.assertIn(data["name"], message) + self.assertNotIn("You may edit it again below.", message) + + # Should not have redirected to changelist + self.assertEqual(response.url, "/admin/market/item/") + + # Should not have changed existing item + self.item.refresh_from_db() + self.assertEqual(self.item.name, "Not name") + self.assertEqual(self.item.file.name.count("test_file"), 1) + self.assertEqual(self.item.image.name.count("test_image2"), 0) + self.assertEqual(self.item.image.name.count("test_image"), 1) + + # Should have saved new item + self.assertEqual(Item.objects.count(), 2) + new_item = Item.objects.filter(id=self.item.id + 1).first() + self.assertIsNotNone(new_item) + self.assertEqual(new_item.name, data["name"]) + self.assertEqual(new_item.price, data["price"]) + self.assertEqual(new_item.currency, data["currency"]) + self.assertFalse(new_item.image) + + # Able to save the cached file since cached object was there even though cached post was not + self.assertRegex(new_item.file.name, r"test_file2.*\.jpg$") + + # Should have cleared cache + for key in CACHE_KEYS.values(): + self.assertIsNone(cache.get(key)) + + def test_add_without_cached_object_should_save_new_instance_but_not_have_file(self): + # Request.POST + data = { + "name": "name", + "price": 2.0, + "image": "", + "currency": Item.VALID_CURRENCIES[0][0], + "_confirm_add": True, + "_save": True, + } + + # Make sure there's no post cached obj + cache.delete(CACHE_KEYS["object"]) + cache.set(CACHE_KEYS["post"], data) + + # Click "Yes, I'm Sure" + del data["_confirm_add"] + data[CONFIRMATION_RECEIVED] = True + + with mock.patch.object(ItemAdmin, "message_user") as message_user: + response = self.client.post("/admin/market/item/add/", data=data) + # Should show message to user with correct obj and path + message_user.assert_called_once() + message = message_user.call_args[0][1] + self.assertIn("/admin/market/item/2/change/", message) + self.assertIn(data["name"], message) + self.assertNotIn("You may edit it again below.", message) + + # Should not have redirected to changelist + self.assertEqual(response.url, "/admin/market/item/") + + # Should not have changed existing item + self.item.refresh_from_db() + self.assertEqual(self.item.name, "Not name") + self.assertEqual(self.item.file.name.count("test_file"), 1) + self.assertEqual(self.item.image.name.count("test_image2"), 0) + self.assertEqual(self.item.image.name.count("test_image"), 1) + + # Should have saved new item + self.assertEqual(Item.objects.count(), 2) + new_item = Item.objects.filter(id=self.item.id + 1).first() + self.assertIsNotNone(new_item) + self.assertEqual(new_item.name, data["name"]) + self.assertEqual(new_item.price, data["price"]) + self.assertEqual(new_item.currency, data["currency"]) + self.assertFalse(new_item.image) + + # FAILED to save the file, because cached item was not there + self.assertFalse(new_item.file) + + # Should have cleared cache + for key in CACHE_KEYS.values(): + self.assertIsNone(cache.get(key)) + + def test_add_without_any_cache_should_save_new_instance_but_not_have_file(self): + # Request.POST + data = { + "name": "name", + "price": 2.0, + "image": "", + "currency": Item.VALID_CURRENCIES[0][0], + "_confirm_add": True, + "_save": True, + } + + # Make sure there's no cache + cache.delete(CACHE_KEYS["object"]) + cache.delete(CACHE_KEYS["post"]) + + # Click "Yes, I'm Sure" + del data["_confirm_add"] + data[CONFIRMATION_RECEIVED] = True + + with mock.patch.object(ItemAdmin, "message_user") as message_user: + response = self.client.post("/admin/market/item/add/", data=data) + # Should show message to user with correct obj and path + message_user.assert_called_once() + message = message_user.call_args[0][1] + self.assertIn("/admin/market/item/2/change/", message) + self.assertIn(data["name"], message) + self.assertNotIn("You may edit it again below.", message) + + # Should not have redirected to changelist + self.assertEqual(response.url, "/admin/market/item/") + + # Should not have changed existing item + self.item.refresh_from_db() + self.assertEqual(self.item.name, "Not name") + self.assertEqual(self.item.file.name.count("test_file"), 1) + self.assertEqual(self.item.image.name.count("test_image2"), 0) + self.assertEqual(self.item.image.name.count("test_image"), 1) + + # Should have saved new item + self.assertEqual(Item.objects.count(), 2) + new_item = Item.objects.filter(id=self.item.id + 1).first() + self.assertIsNotNone(new_item) + self.assertEqual(new_item.name, data["name"]) + self.assertEqual(new_item.price, data["price"]) + self.assertEqual(new_item.currency, data["currency"]) + self.assertFalse(new_item.image) + + # FAILED to save the file, because cached item was not there + self.assertFalse(new_item.file) + + # Should have cleared cache + for key in CACHE_KEYS.values(): + self.assertIsNone(cache.get(key)) + + def test_change_without_cached_post_should_save_file_changes(self): + item = self.item + # Load the Change Item Page + ItemAdmin.save_as_continue = False + + # Upload new image and remove file + i2 = SimpleUploadedFile( + name="test_image2.jpg", + content=open(self.image_path, "rb").read(), + content_type="image/jpeg", + ) + # Request.POST + data = { + "id": item.id, + "name": "name", + "price": 2.0, + "image": i2, + "file": "", + "file-clear": "on", + "currency": Item.VALID_CURRENCIES[0][0], + "_confirm_change": True, + "_saveasnew": True, + } + + # Set cache + cache_item = Item( + name=data["name"], + price=data["price"], + currency=data["currency"], + image=i2, + ) + file_cache = FileCache() + file_cache.set(format_cache_key(model="Item", field="image"), i2) + + cache.set(CACHE_KEYS["object"], cache_item) + # Ensure no cached post + cache.delete(CACHE_KEYS["post"]) + + # Click "Yes, I'm Sure" + del data["_confirm_change"] + # Image would have been in FILES and not in POST + del data["image"] + data[CONFIRMATION_RECEIVED] = True + + with mock.patch.object(ItemAdmin, "message_user") as message_user: + response = self.client.post( + f"/admin/market/item/{self.item.id}/change/", data=data + ) + # Should show message to user with correct obj and path + message_user.assert_called_once() + message = message_user.call_args[0][1] + self.assertIn("/admin/market/item/2/change/", message) + self.assertIn(data["name"], message) + self.assertNotIn("You may edit it again below.", message) + + # Should have redirected to changelist + self.assertEqual(response.url, "/admin/market/item/") + + # Should not have changed existing item + item.refresh_from_db() + self.assertEqual(item.name, "Not name") + self.assertEqual(item.file.name.count("test_file"), 1) + self.assertEqual(item.image.name.count("test_image2"), 0) + self.assertEqual(item.image.name.count("test_image"), 1) + + # Should have saved new item + self.assertEqual(Item.objects.count(), 2) + new_item = Item.objects.filter(id=item.id + 1).first() + self.assertIsNotNone(new_item) + self.assertEqual(new_item.name, data["name"]) + self.assertEqual(new_item.price, data["price"]) + self.assertEqual(new_item.currency, data["currency"]) + # Should have cleared `file` since clear was selected + self.assertFalse(new_item.file) + self.assertIsNotNone(new_item.image) + # Saved cached file from cached obj even if cached post was missing + self.assertIn("test_image2", new_item.image.name) + + # Should have cleared cache + for key in CACHE_KEYS.values(): + self.assertIsNone(cache.get(key)) + + def test_change_without_cached_object_should_save_but_without_file_changes(self): + item = self.item + # Load the Change Item Page + ItemAdmin.save_as_continue = False + + # Request.POST + data = { + "id": item.id, + "name": "name", + "price": 2.0, + "file": "", + "file-clear": "on", + "currency": Item.VALID_CURRENCIES[0][0], + "_confirm_change": True, + "_saveasnew": True, + } + + # Ensure no cached obj + cache.delete(CACHE_KEYS["object"]) + cache.set(CACHE_KEYS["post"], data) + + # Click "Yes, I'm Sure" + del data["_confirm_change"] + data[CONFIRMATION_RECEIVED] = True + + with mock.patch.object(ItemAdmin, "message_user") as message_user: + response = self.client.post( + f"/admin/market/item/{self.item.id}/change/", data=data + ) + # Should show message to user with correct obj and path + message_user.assert_called_once() + message = message_user.call_args[0][1] + self.assertIn("/admin/market/item/2/change/", message) + self.assertIn(data["name"], message) + self.assertNotIn("You may edit it again below.", message) + + # Should have redirected to changelist + self.assertEqual(response.url, "/admin/market/item/") + + # Should not have changed existing item + item.refresh_from_db() + self.assertEqual(item.name, "Not name") + self.assertEqual(item.file.name.count("test_file"), 1) + self.assertEqual(item.image.name.count("test_image2"), 0) + self.assertEqual(item.image.name.count("test_image"), 1) + + # Should have saved new item + self.assertEqual(Item.objects.count(), 2) + new_item = Item.objects.filter(id=item.id + 1).first() + self.assertIsNotNone(new_item) + self.assertEqual(new_item.name, data["name"]) + self.assertEqual(new_item.price, data["price"]) + self.assertEqual(new_item.currency, data["currency"]) + self.assertFalse(new_item.file) + # FAILED to save image + self.assertFalse(new_item.image) + + # Should have cleared cache + for key in CACHE_KEYS.values(): + self.assertIsNone(cache.get(key)) + + def test_change_without_any_cache_should_save_but_not_have_file_changes(self): + item = self.item + # Load the Change Item Page + ItemAdmin.save_as_continue = False + + # Request.POST + data = { + "id": item.id, + "name": "name", + "price": 2.0, + "file": "", + "file-clear": "on", + "currency": Item.VALID_CURRENCIES[0][0], + "_confirm_change": True, + "_saveasnew": True, + } + + # Ensure no cache + cache.delete(CACHE_KEYS["object"]) + cache.delete(CACHE_KEYS["post"]) + + # Click "Yes, I'm Sure" + del data["_confirm_change"] + data[CONFIRMATION_RECEIVED] = True + + with mock.patch.object(ItemAdmin, "message_user") as message_user: + response = self.client.post( + f"/admin/market/item/{self.item.id}/change/", data=data + ) + # Should show message to user with correct obj and path + message_user.assert_called_once() + message = message_user.call_args[0][1] + self.assertIn("/admin/market/item/2/change/", message) + self.assertIn(data["name"], message) + self.assertNotIn("You may edit it again below.", message) + + # Should have redirected to changelist + self.assertEqual(response.url, "/admin/market/item/") + + # Should not have changed existing item + item.refresh_from_db() + self.assertEqual(item.name, "Not name") + self.assertEqual(item.file.name.count("test_file"), 1) + self.assertEqual(item.image.name.count("test_image2"), 0) + self.assertEqual(item.image.name.count("test_image"), 1) + + # Should have saved new item + self.assertEqual(Item.objects.count(), 2) + new_item = Item.objects.filter(id=item.id + 1).first() + self.assertIsNotNone(new_item) + self.assertEqual(new_item.name, data["name"]) + self.assertEqual(new_item.price, data["price"]) + self.assertEqual(new_item.currency, data["currency"]) + self.assertFalse(new_item.file) + # FAILED to save image + self.assertFalse(new_item.image) + + # Should have cleared cache + for key in CACHE_KEYS.values(): + self.assertIsNone(cache.get(key)) + + def test_change_without_changing_file_should_save_changes(self): + item = self.item + # Load the Change Item Page + ItemAdmin.save_as_continue = False + + # Request.POST + data = { + "id": item.id, + "name": "name", + "price": 2.0, + "file": "", + "image": "", + "file-clear": "on", + "currency": Item.VALID_CURRENCIES[0][0], + "_confirm_change": True, + "_save": True, + } + + # Set cache + cache_item = Item( + name=data["name"], + price=data["price"], + currency=data["currency"], + ) + + cache.get(CACHE_KEYS["object"], cache_item) + cache.get(CACHE_KEYS["post"], data) + + # Click "Yes, I'm Sure" + del data["_confirm_change"] + data[CONFIRMATION_RECEIVED] = True + + with mock.patch.object(ItemAdmin, "message_user") as message_user: + response = self.client.post( + f"/admin/market/item/{self.item.id}/change/", data=data + ) + # Should show message to user with correct obj and path + message_user.assert_called_once() + message = message_user.call_args[0][1] + self.assertIn("/admin/market/item/1/change/", message) + self.assertIn(data["name"], message) + self.assertNotIn("You may edit it again below.", message) + + # Should have redirected to changelist + self.assertEqual(response.url, "/admin/market/item/") + + # Should have changed existing item + self.assertEqual(Item.objects.count(), 1) + item.refresh_from_db() + self.assertEqual(item.name, "name") + # Should have cleared if requested + self.assertFalse(item.file.name) + self.assertEqual(item.image.name.count("test_image2"), 0) + self.assertEqual(item.image.name.count("test_image"), 1) + + # Should have cleared cache + for key in CACHE_KEYS.values(): + self.assertIsNone(cache.get(key)) + + @mock.patch("admin_confirm.admin.CACHE_TIMEOUT", 1) + def test_old_cache_should_not_be_used(self): + item = self.item + + # Upload new image and remove file + i2 = SimpleUploadedFile( + name="test_image2.jpg", + content=open(self.image_path, "rb").read(), + content_type="image/jpeg", + ) + # Click "Save And Continue" + data = { + "id": item.id, + "name": "name", + "price": 2.0, + "image": i2, + "file": "", + "file-clear": "on", + "currency": Item.VALID_CURRENCIES[0][0], + "_confirm_change": True, + "_continue": True, + } + response = self.client.post(f"/admin/market/item/{item.id}/change/", data=data) + + # Should be shown confirmation page + self._assertSubmitHtml( + rendered_content=response.rendered_content, + save_action="_continue", + multipart_form=True, + ) + + # Should have cached the unsaved item + cached_item = cache.get(CACHE_KEYS["object"]) + self.assertIsNotNone(cached_item) + + # Should not have saved the changes yet + self.assertEqual(Item.objects.count(), 1) + item.refresh_from_db() + self.assertEqual(item.name, "Not name") + self.assertIsNotNone(item.file) + self.assertIsNotNone(item.image) + + # Wait for cache to time out + + time.sleep(1) + + # Check that it did time out + cached_item = cache.get(CACHE_KEYS["object"]) + self.assertIsNone(cached_item) + + # Click "Yes, I'm Sure" + del data["_confirm_change"] + data["image"] = "" + data[CONFIRMATION_RECEIVED] = True + response = self.client.post(f"/admin/market/item/{item.id}/change/", data=data) + + # Should not have redirected to changelist + self.assertEqual(response.url, f"/admin/market/item/{item.id}/change/") + + # Should have saved item + self.assertEqual(Item.objects.count(), 1) + saved_item = Item.objects.all().first() + self.assertEqual(saved_item.name, data["name"]) + self.assertEqual(saved_item.price, data["price"]) + self.assertEqual(saved_item.currency, data["currency"]) + self.assertFalse(saved_item.file) + + # SHOULD not have saved image since it was in the old cache + self.assertNotIn("test_image2", saved_item.image) + + # Should have cleared cache + for key in CACHE_KEYS.values(): + self.assertIsNone(cache.get(key)) + + def test_cache_with_incorrect_model_should_not_be_used(self): + item = self.item + # Load the Change Item Page + ItemAdmin.save_as_continue = False + + # Request.POST + data = { + "id": item.id, + "name": "name", + "price": 2.0, + "file": "", + "file-clear": "on", + "currency": Item.VALID_CURRENCIES[0][0], + "_confirm_change": True, + "_save": True, + } + + # Set cache to incorrect model + cache_obj = Shop(name="ShopName") + + cache.set(CACHE_KEYS["object"], cache_obj) + cache.set(CACHE_KEYS["post"], data) + + # Click "Yes, I'm Sure" + del data["_confirm_change"] + data[CONFIRMATION_RECEIVED] = True + + with mock.patch.object(ItemAdmin, "message_user") as message_user: + response = self.client.post( + f"/admin/market/item/{self.item.id}/change/", data=data + ) + # Should show message to user with correct obj and path + message_user.assert_called_once() + message = message_user.call_args[0][1] + self.assertIn("/admin/market/item/1/change/", message) + self.assertIn(data["name"], message) + self.assertNotIn("You may edit it again below.", message) + + # Should have redirected to changelist + self.assertEqual(response.url, "/admin/market/item/") + + # Should have changed existing item + self.assertEqual(Item.objects.count(), 1) + item.refresh_from_db() + self.assertEqual(item.name, "name") + # Should have cleared if requested + self.assertFalse(item.file.name) + self.assertEqual(item.image.name.count("test_image2"), 0) + self.assertEqual(item.image.name.count("test_image"), 1) + + # Should have cleared cache + for key in CACHE_KEYS.values(): + self.assertIsNone(cache.get(key)) + + def test_form_without_files_should_not_use_cache(self): + cache.delete_many(CACHE_KEYS.values()) + shop = ShopFactory() + # Click "Save And Continue" + data = { + "id": shop.id, + "name": "name", + "_confirm_change": True, + "_continue": True, + } + response = self.client.post(f"/admin/market/shop/{shop.id}/change/", data=data) + + # Should be shown confirmation page + self._assertSubmitHtml( + rendered_content=response.rendered_content, save_action="_continue" + ) + + # Should not have set cache since not multipart form + for key in CACHE_KEYS.values(): + self.assertIsNone(cache.get(key)) diff --git a/admin_confirm/tests/unit/test_file_cache.py b/admin_confirm/tests/unit/test_file_cache.py index adf59d6..ffd138a 100644 --- a/admin_confirm/tests/unit/test_file_cache.py +++ b/admin_confirm/tests/unit/test_file_cache.py @@ -1,911 +1,33 @@ -""" -Ensure that files are saved during confirmation -Without file changes, Django is relied on - -With file changes, we cache the object, save it with -the files if new, or add files to existing obj and save - -Then send the rest of the changes to Django to handle - -This is arguably the most we fiddle with the Django request -Thus we should test it extensively -""" -import time -from unittest import mock - from django.core.files.uploadedfile import SimpleUploadedFile -from django.core.cache import cache - -from admin_confirm.tests.helpers import AdminConfirmTestCase -from admin_confirm.constants import CACHE_KEYS, CONFIRMATION_RECEIVED - -from tests.market.admin import ItemAdmin -from tests.market.models import Item, Shop -from tests.factories import ItemFactory, ShopFactory - - -class TestFileCache(AdminConfirmTestCase): - def setUp(self): - # Load the Change Item Page - ItemAdmin.confirm_change = True - ItemAdmin.fields = ["name", "price", "file", "image", "currency"] - ItemAdmin.save_as = True - ItemAdmin.save_as_continue = True - - self.image_path = "screenshot.png" - f = SimpleUploadedFile( - name="test_file.jpg", - content=open(self.image_path, "rb").read(), - content_type="image/jpeg", - ) - i = SimpleUploadedFile( - name="test_image.jpg", - content=open(self.image_path, "rb").read(), - content_type="image/jpeg", - ) - self.item = ItemFactory(name="Not name", file=f, image=i) - - return super().setUp() - - def test_save_as_continue_true_should_not_redirect_to_changelist(self): - item = self.item - # Load the Change Item Page - ItemAdmin.save_as_continue = True - - # Upload new image and remove file - i2 = SimpleUploadedFile( - name="test_image2.jpg", - content=open(self.image_path, "rb").read(), - content_type="image/jpeg", - ) - # Request.POST - data = { - "id": item.id, - "name": "name", - "price": 2.0, - "file": "", - "file-clear": "on", - "currency": Item.VALID_CURRENCIES[0][0], - "_confirm_change": True, - "_saveasnew": True, - } - - # Set cache - cache_item = Item( - name=data["name"], - price=data["price"], - currency=data["currency"], - image=i2, - ) - - cache.set(CACHE_KEYS["object"], cache_item) - cache.set(CACHE_KEYS["post"], data) - - # Click "Yes, I'm Sure" - del data["_confirm_change"] - data[CONFIRMATION_RECEIVED] = True - - with mock.patch.object(ItemAdmin, "message_user") as message_user: - response = self.client.post( - f"/admin/market/item/{self.item.id}/change/", data=data - ) - # Should show message to user with correct obj and path - message_user.assert_called_once() - message = message_user.call_args[0][1] - self.assertIn("/admin/market/item/2/change/", message) - self.assertIn(data["name"], message) - self.assertIn("You may edit it again below.", message) - - # Should not have redirected to changelist - self.assertEqual(response.url, f"/admin/market/item/{self.item.id + 1}/change/") - - # Should not have changed existing item - item.refresh_from_db() - self.assertEqual(item.name, "Not name") - self.assertEqual(item.file.name.count("test_file"), 1) - self.assertEqual(item.image.name.count("test_image2"), 0) - self.assertEqual(item.image.name.count("test_image"), 1) - - # Should have saved new item - self.assertEqual(Item.objects.count(), 2) - new_item = Item.objects.filter(id=item.id + 1).first() - self.assertIsNotNone(new_item) - self.assertEqual(new_item.name, data["name"]) - self.assertEqual(new_item.price, data["price"]) - self.assertEqual(new_item.currency, data["currency"]) - self.assertFalse(new_item.file) - self.assertEqual(new_item.image.name.count("test_image2"), 1) - - # Should have cleared cache - for key in CACHE_KEYS.values(): - self.assertIsNone(cache.get(key)) - - def test_save_as_continue_false_should_redirect_to_changelist(self): - item = self.item - # Load the Change Item Page - ItemAdmin.save_as_continue = False - - # Upload new image and remove file - i2 = SimpleUploadedFile( - name="test_image2.jpg", - content=open(self.image_path, "rb").read(), - content_type="image/jpeg", - ) - # Request.POST - data = { - "id": item.id, - "name": "name", - "price": 2.0, - "file": "", - "file-clear": "on", - "currency": Item.VALID_CURRENCIES[0][0], - "_confirm_change": True, - "_saveasnew": True, - } - - # Set cache - cache_item = Item( - name=data["name"], - price=data["price"], - currency=data["currency"], - image=i2, - ) - - cache.set(CACHE_KEYS["object"], cache_item) - cache.set(CACHE_KEYS["post"], data) - - # Click "Yes, I'm Sure" - del data["_confirm_change"] - data[CONFIRMATION_RECEIVED] = True - - with mock.patch.object(ItemAdmin, "message_user") as message_user: - response = self.client.post( - f"/admin/market/item/{self.item.id}/change/", data=data - ) - # Should show message to user with correct obj and path - message_user.assert_called_once() - message = message_user.call_args[0][1] - self.assertIn("/admin/market/item/2/change/", message) - self.assertIn(data["name"], message) - self.assertNotIn("You may edit it again below.", message) - - # Should have redirected to changelist - self.assertEqual(response.url, "/admin/market/item/") - - # Should not have changed existing item - item.refresh_from_db() - self.assertEqual(item.name, "Not name") - self.assertEqual(item.file.name.count("test_file"), 1) - self.assertEqual(item.image.name.count("test_image2"), 0) - self.assertEqual(item.image.name.count("test_image"), 1) - - # Should have saved new item - self.assertEqual(Item.objects.count(), 2) - new_item = Item.objects.filter(id=item.id + 1).first() - self.assertIsNotNone(new_item) - self.assertEqual(new_item.name, data["name"]) - self.assertEqual(new_item.price, data["price"]) - self.assertEqual(new_item.currency, data["currency"]) - self.assertFalse(new_item.file) - self.assertEqual(new_item.image.name.count("test_image2"), 1) - - # Should have cleared cache - for key in CACHE_KEYS.values(): - self.assertIsNone(cache.get(key)) - - def test_saveasnew_without_any_file_changes_should_save_new_instance_without_files( - self, - ): - item = self.item - - # Request.POST - data = { - "id": item.id, - "name": "name", - "price": 2.0, - "file": "", - "image": "", - "currency": Item.VALID_CURRENCIES[0][0], - "_confirm_change": True, - "_saveasnew": True, - } - - # Set cache - cache_item = Item( - name=data["name"], - price=data["price"], - currency=data["currency"], - ) - - cache.set(CACHE_KEYS["object"], cache_item) - cache.set(CACHE_KEYS["post"], data) - - # Click "Yes, I'm Sure" - del data["_confirm_change"] - data[CONFIRMATION_RECEIVED] = True - - with mock.patch.object(ItemAdmin, "message_user") as message_user: - response = self.client.post( - f"/admin/market/item/{self.item.id}/change/", data=data - ) - # Should show message to user with correct obj and path - message_user.assert_called_once() - message = message_user.call_args[0][1] - self.assertIn("/admin/market/item/2/change/", message) - self.assertIn(data["name"], message) - self.assertIn("You may edit it again below.", message) - - # Should not have redirected to changelist - self.assertEqual(response.url, f"/admin/market/item/{self.item.id + 1}/change/") - - # Should not have changed existing item - item.refresh_from_db() - self.assertEqual(item.name, "Not name") - self.assertEqual(item.file.name.count("test_file"), 1) - self.assertEqual(item.image.name.count("test_image2"), 0) - self.assertEqual(item.image.name.count("test_image"), 1) - - # Should have saved new item - self.assertEqual(Item.objects.count(), 2) - new_item = Item.objects.filter(id=item.id + 1).first() - self.assertIsNotNone(new_item) - self.assertEqual(new_item.name, data["name"]) - self.assertEqual(new_item.price, data["price"]) - self.assertEqual(new_item.currency, data["currency"]) - # In Django (by default), the save as new does not transfer over the files - self.assertFalse(new_item.file) - self.assertFalse(new_item.image) - - # Should have cleared cache - for key in CACHE_KEYS.values(): - self.assertIsNone(cache.get(key)) - - def test_add_with_upload_file_should_save_new_instance_with_files(self): - # Request.POST - data = { - "name": "name", - "price": 2.0, - "image": "", - "currency": Item.VALID_CURRENCIES[0][0], - "_confirm_add": True, - "_save": True, - } - - # Upload new file - f2 = SimpleUploadedFile( - name="test_file2.jpg", - content=open(self.image_path, "rb").read(), - content_type="image/jpeg", - ) - # Set cache - cache_item = Item( - name=data["name"], price=data["price"], currency=data["currency"], file=f2 - ) - - cache.set(CACHE_KEYS["object"], cache_item) - cache.set(CACHE_KEYS["post"], data) - - # Click "Yes, I'm Sure" - del data["_confirm_add"] - data[CONFIRMATION_RECEIVED] = True - - with mock.patch.object(ItemAdmin, "message_user") as message_user: - response = self.client.post("/admin/market/item/add/", data=data) - # Should show message to user with correct obj and path - message_user.assert_called_once() - message = message_user.call_args[0][1] - self.assertIn("/admin/market/item/2/change/", message) - self.assertIn(data["name"], message) - self.assertNotIn("You may edit it again below.", message) - - # Should not have redirected to changelist - self.assertEqual(response.url, "/admin/market/item/") - - # Should not have changed existing item - self.item.refresh_from_db() - self.assertEqual(self.item.name, "Not name") - self.assertEqual(self.item.file.name.count("test_file"), 1) - self.assertEqual(self.item.image.name.count("test_image2"), 0) - self.assertEqual(self.item.image.name.count("test_image"), 1) - - # Should have saved new item - self.assertEqual(Item.objects.count(), 2) - new_item = Item.objects.filter(id=self.item.id + 1).first() - self.assertIsNotNone(new_item) - self.assertEqual(new_item.name, data["name"]) - self.assertEqual(new_item.price, data["price"]) - self.assertEqual(new_item.currency, data["currency"]) - self.assertIn("test_file2", new_item.file.name) - self.assertFalse(new_item.image) - - # Should have cleared cache - for key in CACHE_KEYS.values(): - self.assertIsNone(cache.get(key)) - - def test_add_without_cached_post_should_save_new_instance_with_file(self): - # Request.POST - data = { - "name": "name", - "price": 2.0, - "image": "", - "currency": Item.VALID_CURRENCIES[0][0], - "_confirm_add": True, - "_save": True, - } - - # Upload new file - f2 = SimpleUploadedFile( - name="test_file2.jpg", - content=open(self.image_path, "rb").read(), - content_type="image/jpeg", - ) - # Set cache - cache_item = Item( - name=data["name"], price=data["price"], currency=data["currency"], file=f2 - ) - - cache.set(CACHE_KEYS["object"], cache_item) - # Make sure there's no post cached post - cache.delete(CACHE_KEYS["post"]) - - # Click "Yes, I'm Sure" - del data["_confirm_add"] - data[CONFIRMATION_RECEIVED] = True - - with mock.patch.object(ItemAdmin, "message_user") as message_user: - response = self.client.post("/admin/market/item/add/", data=data) - # Should show message to user with correct obj and path - message_user.assert_called_once() - message = message_user.call_args[0][1] - self.assertIn("/admin/market/item/2/change/", message) - self.assertIn(data["name"], message) - self.assertNotIn("You may edit it again below.", message) - - # Should not have redirected to changelist - self.assertEqual(response.url, "/admin/market/item/") - - # Should not have changed existing item - self.item.refresh_from_db() - self.assertEqual(self.item.name, "Not name") - self.assertEqual(self.item.file.name.count("test_file"), 1) - self.assertEqual(self.item.image.name.count("test_image2"), 0) - self.assertEqual(self.item.image.name.count("test_image"), 1) - - # Should have saved new item - self.assertEqual(Item.objects.count(), 2) - new_item = Item.objects.filter(id=self.item.id + 1).first() - self.assertIsNotNone(new_item) - self.assertEqual(new_item.name, data["name"]) - self.assertEqual(new_item.price, data["price"]) - self.assertEqual(new_item.currency, data["currency"]) - self.assertFalse(new_item.image) - - # Able to save the cached file since cached object was there even though cached post was not - self.assertIn("test_file2", new_item.file.name) - - # Should have cleared cache - for key in CACHE_KEYS.values(): - self.assertIsNone(cache.get(key)) - - def test_add_without_cached_object_should_save_new_instance_but_not_have_file(self): - # Request.POST - data = { - "name": "name", - "price": 2.0, - "image": "", - "currency": Item.VALID_CURRENCIES[0][0], - "_confirm_add": True, - "_save": True, - } - - # Make sure there's no post cached obj - cache.delete(CACHE_KEYS["object"]) - cache.set(CACHE_KEYS["post"], data) - - # Click "Yes, I'm Sure" - del data["_confirm_add"] - data[CONFIRMATION_RECEIVED] = True - - with mock.patch.object(ItemAdmin, "message_user") as message_user: - response = self.client.post("/admin/market/item/add/", data=data) - # Should show message to user with correct obj and path - message_user.assert_called_once() - message = message_user.call_args[0][1] - self.assertIn("/admin/market/item/2/change/", message) - self.assertIn(data["name"], message) - self.assertNotIn("You may edit it again below.", message) - - # Should not have redirected to changelist - self.assertEqual(response.url, "/admin/market/item/") - - # Should not have changed existing item - self.item.refresh_from_db() - self.assertEqual(self.item.name, "Not name") - self.assertEqual(self.item.file.name.count("test_file"), 1) - self.assertEqual(self.item.image.name.count("test_image2"), 0) - self.assertEqual(self.item.image.name.count("test_image"), 1) - - # Should have saved new item - self.assertEqual(Item.objects.count(), 2) - new_item = Item.objects.filter(id=self.item.id + 1).first() - self.assertIsNotNone(new_item) - self.assertEqual(new_item.name, data["name"]) - self.assertEqual(new_item.price, data["price"]) - self.assertEqual(new_item.currency, data["currency"]) - self.assertFalse(new_item.image) - - # FAILED to save the file, because cached item was not there - self.assertFalse(new_item.file) - - # Should have cleared cache - for key in CACHE_KEYS.values(): - self.assertIsNone(cache.get(key)) - - def test_add_without_any_cache_should_save_new_instance_but_not_have_file(self): - # Request.POST - data = { - "name": "name", - "price": 2.0, - "image": "", - "currency": Item.VALID_CURRENCIES[0][0], - "_confirm_add": True, - "_save": True, - } - - # Make sure there's no cache - cache.delete(CACHE_KEYS["object"]) - cache.delete(CACHE_KEYS["post"]) - - # Click "Yes, I'm Sure" - del data["_confirm_add"] - data[CONFIRMATION_RECEIVED] = True - - with mock.patch.object(ItemAdmin, "message_user") as message_user: - response = self.client.post("/admin/market/item/add/", data=data) - # Should show message to user with correct obj and path - message_user.assert_called_once() - message = message_user.call_args[0][1] - self.assertIn("/admin/market/item/2/change/", message) - self.assertIn(data["name"], message) - self.assertNotIn("You may edit it again below.", message) - - # Should not have redirected to changelist - self.assertEqual(response.url, "/admin/market/item/") - - # Should not have changed existing item - self.item.refresh_from_db() - self.assertEqual(self.item.name, "Not name") - self.assertEqual(self.item.file.name.count("test_file"), 1) - self.assertEqual(self.item.image.name.count("test_image2"), 0) - self.assertEqual(self.item.image.name.count("test_image"), 1) - - # Should have saved new item - self.assertEqual(Item.objects.count(), 2) - new_item = Item.objects.filter(id=self.item.id + 1).first() - self.assertIsNotNone(new_item) - self.assertEqual(new_item.name, data["name"]) - self.assertEqual(new_item.price, data["price"]) - self.assertEqual(new_item.currency, data["currency"]) - self.assertFalse(new_item.image) - - # FAILED to save the file, because cached item was not there - self.assertFalse(new_item.file) - - # Should have cleared cache - for key in CACHE_KEYS.values(): - self.assertIsNone(cache.get(key)) - - def test_change_without_cached_post_should_save_file_changes(self): - item = self.item - # Load the Change Item Page - ItemAdmin.save_as_continue = False - - # Upload new image and remove file - i2 = SimpleUploadedFile( - name="test_image2.jpg", - content=open(self.image_path, "rb").read(), - content_type="image/jpeg", - ) - # Request.POST - data = { - "id": item.id, - "name": "name", - "price": 2.0, - "image": i2, - "file": "", - "file-clear": "on", - "currency": Item.VALID_CURRENCIES[0][0], - "_confirm_change": True, - "_saveasnew": True, - } - - # Set cache - cache_item = Item( - name=data["name"], - price=data["price"], - currency=data["currency"], - image=i2, - ) - - cache.set(CACHE_KEYS["object"], cache_item) - # Ensure no cached post - cache.delete(CACHE_KEYS["post"]) - - # Click "Yes, I'm Sure" - del data["_confirm_change"] - # Image would have been in FILES and not in POST - del data["image"] - data[CONFIRMATION_RECEIVED] = True - - with mock.patch.object(ItemAdmin, "message_user") as message_user: - response = self.client.post( - f"/admin/market/item/{self.item.id}/change/", data=data - ) - # Should show message to user with correct obj and path - message_user.assert_called_once() - message = message_user.call_args[0][1] - self.assertIn("/admin/market/item/2/change/", message) - self.assertIn(data["name"], message) - self.assertNotIn("You may edit it again below.", message) - - # Should have redirected to changelist - self.assertEqual(response.url, "/admin/market/item/") - - # Should not have changed existing item - item.refresh_from_db() - self.assertEqual(item.name, "Not name") - self.assertEqual(item.file.name.count("test_file"), 1) - self.assertEqual(item.image.name.count("test_image2"), 0) - self.assertEqual(item.image.name.count("test_image"), 1) - - # Should have saved new item - self.assertEqual(Item.objects.count(), 2) - new_item = Item.objects.filter(id=item.id + 1).first() - self.assertIsNotNone(new_item) - self.assertEqual(new_item.name, data["name"]) - self.assertEqual(new_item.price, data["price"]) - self.assertEqual(new_item.currency, data["currency"]) - # Should have cleared `file` since clear was selected - self.assertFalse(new_item.file) - # Saved cached file from cached obj even if cached post was missing - self.assertIn("test_image2", new_item.image.name) - - # Should have cleared cache - for key in CACHE_KEYS.values(): - self.assertIsNone(cache.get(key)) - - def test_change_without_cached_object_should_save_but_without_file_changes(self): - item = self.item - # Load the Change Item Page - ItemAdmin.save_as_continue = False - - # Request.POST - data = { - "id": item.id, - "name": "name", - "price": 2.0, - "file": "", - "file-clear": "on", - "currency": Item.VALID_CURRENCIES[0][0], - "_confirm_change": True, - "_saveasnew": True, - } - - # Ensure no cached obj - cache.delete(CACHE_KEYS["object"]) - cache.set(CACHE_KEYS["post"], data) - - # Click "Yes, I'm Sure" - del data["_confirm_change"] - data[CONFIRMATION_RECEIVED] = True - - with mock.patch.object(ItemAdmin, "message_user") as message_user: - response = self.client.post( - f"/admin/market/item/{self.item.id}/change/", data=data - ) - # Should show message to user with correct obj and path - message_user.assert_called_once() - message = message_user.call_args[0][1] - self.assertIn("/admin/market/item/2/change/", message) - self.assertIn(data["name"], message) - self.assertNotIn("You may edit it again below.", message) - - # Should have redirected to changelist - self.assertEqual(response.url, "/admin/market/item/") - - # Should not have changed existing item - item.refresh_from_db() - self.assertEqual(item.name, "Not name") - self.assertEqual(item.file.name.count("test_file"), 1) - self.assertEqual(item.image.name.count("test_image2"), 0) - self.assertEqual(item.image.name.count("test_image"), 1) - - # Should have saved new item - self.assertEqual(Item.objects.count(), 2) - new_item = Item.objects.filter(id=item.id + 1).first() - self.assertIsNotNone(new_item) - self.assertEqual(new_item.name, data["name"]) - self.assertEqual(new_item.price, data["price"]) - self.assertEqual(new_item.currency, data["currency"]) - self.assertFalse(new_item.file) - # FAILED to save image - self.assertFalse(new_item.image) - - # Should have cleared cache - for key in CACHE_KEYS.values(): - self.assertIsNone(cache.get(key)) - - def test_change_without_any_cache_should_save_but_not_have_file_changes(self): - item = self.item - # Load the Change Item Page - ItemAdmin.save_as_continue = False - - # Request.POST - data = { - "id": item.id, - "name": "name", - "price": 2.0, - "file": "", - "file-clear": "on", - "currency": Item.VALID_CURRENCIES[0][0], - "_confirm_change": True, - "_saveasnew": True, - } - - # Ensure no cache - cache.delete(CACHE_KEYS["object"]) - cache.delete(CACHE_KEYS["post"]) - - # Click "Yes, I'm Sure" - del data["_confirm_change"] - data[CONFIRMATION_RECEIVED] = True - - with mock.patch.object(ItemAdmin, "message_user") as message_user: - response = self.client.post( - f"/admin/market/item/{self.item.id}/change/", data=data - ) - # Should show message to user with correct obj and path - message_user.assert_called_once() - message = message_user.call_args[0][1] - self.assertIn("/admin/market/item/2/change/", message) - self.assertIn(data["name"], message) - self.assertNotIn("You may edit it again below.", message) - - # Should have redirected to changelist - self.assertEqual(response.url, "/admin/market/item/") - - # Should not have changed existing item - item.refresh_from_db() - self.assertEqual(item.name, "Not name") - self.assertEqual(item.file.name.count("test_file"), 1) - self.assertEqual(item.image.name.count("test_image2"), 0) - self.assertEqual(item.image.name.count("test_image"), 1) - - # Should have saved new item - self.assertEqual(Item.objects.count(), 2) - new_item = Item.objects.filter(id=item.id + 1).first() - self.assertIsNotNone(new_item) - self.assertEqual(new_item.name, data["name"]) - self.assertEqual(new_item.price, data["price"]) - self.assertEqual(new_item.currency, data["currency"]) - self.assertFalse(new_item.file) - # FAILED to save image - self.assertFalse(new_item.image) - - # Should have cleared cache - for key in CACHE_KEYS.values(): - self.assertIsNone(cache.get(key)) - - def test_change_without_changing_file_should_save_changes(self): - item = self.item - # Load the Change Item Page - ItemAdmin.save_as_continue = False - - # Request.POST - data = { - "id": item.id, - "name": "name", - "price": 2.0, - "file": "", - "image": "", - "file-clear": "on", - "currency": Item.VALID_CURRENCIES[0][0], - "_confirm_change": True, - "_save": True, - } - - # Set cache - cache_item = Item( - name=data["name"], - price=data["price"], - currency=data["currency"], - ) - - cache.get(CACHE_KEYS["object"], cache_item) - cache.get(CACHE_KEYS["post"], data) - - # Click "Yes, I'm Sure" - del data["_confirm_change"] - data[CONFIRMATION_RECEIVED] = True - - with mock.patch.object(ItemAdmin, "message_user") as message_user: - response = self.client.post( - f"/admin/market/item/{self.item.id}/change/", data=data - ) - # Should show message to user with correct obj and path - message_user.assert_called_once() - message = message_user.call_args[0][1] - self.assertIn("/admin/market/item/1/change/", message) - self.assertIn(data["name"], message) - self.assertNotIn("You may edit it again below.", message) - - # Should have redirected to changelist - self.assertEqual(response.url, "/admin/market/item/") - - # Should have changed existing item - self.assertEqual(Item.objects.count(), 1) - item.refresh_from_db() - self.assertEqual(item.name, "name") - # Should have cleared if requested - self.assertFalse(item.file.name) - self.assertEqual(item.image.name.count("test_image2"), 0) - self.assertEqual(item.image.name.count("test_image"), 1) - - # Should have cleared cache - for key in CACHE_KEYS.values(): - self.assertIsNone(cache.get(key)) - - @mock.patch("admin_confirm.admin.CACHE_TIMEOUT", 1) - def test_old_cache_should_not_be_used(self): - item = self.item - - # Upload new image and remove file - i2 = SimpleUploadedFile( - name="test_image2.jpg", - content=open(self.image_path, "rb").read(), - content_type="image/jpeg", - ) - # Click "Save And Continue" - data = { - "id": item.id, - "name": "name", - "price": 2.0, - "image": i2, - "file": "", - "file-clear": "on", - "currency": Item.VALID_CURRENCIES[0][0], - "_confirm_change": True, - "_continue": True, - } - response = self.client.post(f"/admin/market/item/{item.id}/change/", data=data) - - # Should be shown confirmation page - self._assertSubmitHtml( - rendered_content=response.rendered_content, - save_action="_continue", - multipart_form=True, - ) - - # Should have cached the unsaved item - cached_item = cache.get(CACHE_KEYS["object"]) - self.assertIsNotNone(cached_item) - self.assertIsNone(cached_item.id) - self.assertEqual(cached_item.name, data["name"]) - self.assertEqual(cached_item.price, data["price"]) - self.assertEqual(cached_item.currency, data["currency"]) - self.assertFalse(cached_item.file.name) - self.assertEqual(cached_item.image, i2) - - # Should not have saved the changes yet - self.assertEqual(Item.objects.count(), 1) - item.refresh_from_db() - self.assertEqual(item.name, "Not name") - self.assertIsNotNone(item.file) - self.assertIsNotNone(item.image) - - # Wait for cache to time out - - time.sleep(1) - - # Check that it did time out - cached_item = cache.get(CACHE_KEYS["object"]) - self.assertIsNone(cached_item) - - # Click "Yes, I'm Sure" - del data["_confirm_change"] - data["image"] = "" - data[CONFIRMATION_RECEIVED] = True - response = self.client.post(f"/admin/market/item/{item.id}/change/", data=data) - - # Should not have redirected to changelist - self.assertEqual(response.url, f"/admin/market/item/{item.id}/change/") - - # Should have saved item - self.assertEqual(Item.objects.count(), 1) - saved_item = Item.objects.all().first() - self.assertEqual(saved_item.name, data["name"]) - self.assertEqual(saved_item.price, data["price"]) - self.assertEqual(saved_item.currency, data["currency"]) - self.assertFalse(saved_item.file) - - # SHOULD not have saved image since it was in the old cache - self.assertNotIn("test_image2", saved_item.image) - - # Should have cleared cache - for key in CACHE_KEYS.values(): - self.assertIsNone(cache.get(key)) - - def test_cache_with_incorrect_model_should_not_be_used(self): - item = self.item - # Load the Change Item Page - ItemAdmin.save_as_continue = False - - # Request.POST - data = { - "id": item.id, - "name": "name", - "price": 2.0, - "file": "", - "file-clear": "on", - "currency": Item.VALID_CURRENCIES[0][0], - "_confirm_change": True, - "_save": True, - } - - # Set cache to incorrect model - cache_obj = Shop(name="ShopName") - - cache.set(CACHE_KEYS["object"], cache_obj) - cache.set(CACHE_KEYS["post"], data) - - # Click "Yes, I'm Sure" - del data["_confirm_change"] - data[CONFIRMATION_RECEIVED] = True - - with mock.patch.object(ItemAdmin, "message_user") as message_user: - response = self.client.post( - f"/admin/market/item/{self.item.id}/change/", data=data - ) - # Should show message to user with correct obj and path - message_user.assert_called_once() - message = message_user.call_args[0][1] - self.assertIn("/admin/market/item/1/change/", message) - self.assertIn(data["name"], message) - self.assertNotIn("You may edit it again below.", message) - - # Should have redirected to changelist - self.assertEqual(response.url, "/admin/market/item/") - - # Should have changed existing item - self.assertEqual(Item.objects.count(), 1) - item.refresh_from_db() - self.assertEqual(item.name, "name") - # Should have cleared if requested - self.assertFalse(item.file.name) - self.assertEqual(item.image.name.count("test_image2"), 0) - self.assertEqual(item.image.name.count("test_image"), 1) - - # Should have cleared cache - for key in CACHE_KEYS.values(): - self.assertIsNone(cache.get(key)) - - def test_form_without_files_should_not_use_cache(self): - cache.delete_many(CACHE_KEYS.values()) - shop = ShopFactory() - # Click "Save And Continue" - data = { - "id": shop.id, - "name": "name", - "_confirm_change": True, - "_continue": True, - } - response = self.client.post(f"/admin/market/shop/{shop.id}/change/", data=data) - - # Should be shown confirmation page - self._assertSubmitHtml( - rendered_content=response.rendered_content, save_action="_continue" - ) - - # Should not have set cache since not multipart form - for key in CACHE_KEYS.values(): - self.assertIsNone(cache.get(key)) +from admin_confirm.file_cache import FileCache + +file = SimpleUploadedFile( + name="test_file.jpg", + content=open("screenshot.png", "rb").read(), + content_type="image/jpeg", +) + + +def test_should_set_file_cache(): + file_cache = FileCache() + file_cache.set("key", file) + assert "key" in file_cache.cached_keys + assert file_cache.get("key") is not None + + +def test_should_delete_file_cache(): + file_cache = FileCache() + file_cache.set("key", file) + file_cache.delete("key") + assert "key" not in file_cache.cached_keys + assert file_cache.get("key") is None + + +def test_should_delete_all_file_cache(): + file_cache = FileCache() + file_cache.set("key", file) + file_cache.set("key2", file) + file_cache.delete_all() + assert len(file_cache.cached_keys) == 0 + assert file_cache.get("key") is None + assert file_cache.get("key2") is None diff --git a/admin_confirm/tests/unit/test_with_validators.py b/admin_confirm/tests/unit/test_with_validators.py index 8874bb7..0c203a5 100644 --- a/admin_confirm/tests/unit/test_with_validators.py +++ b/admin_confirm/tests/unit/test_with_validators.py @@ -1,7 +1,11 @@ """ Ensures that confirmations work with validators on the Model and on the Modelform. + +NOTE: These unit tests are not enough to ensure that confirmations work with validators. + Please ensure to add integration tests too! """ +from admin_confirm.constants import CONFIRM_ADD from unittest import mock from django.urls import reverse from django.utils import timezone @@ -80,27 +84,8 @@ class TestWithValidators(AdminConfirmTestCase): "_save": True, } response = self.client.post(reverse("admin:market_itemsale_add"), data) - # Should not have been added yet - self.assertEqual(ItemSale.objects.count(), 0) - # Ensure not redirected (confirmation page does not redirect) - self.assertEqual(response.status_code, 200) - expected_templates = [ - "admin/market/itemsale/change_confirmation.html", - "admin/market/change_confirmation.html", - "admin/change_confirmation.html", - ] - self.assertEqual(response.template_name, expected_templates) - - self._assertSubmitHtml(rendered_content=response.rendered_content) - - # Confirmation page would not have the _confirm_add sent on submit - del data["_confirm_add"] - # Selecting to "Yes, I'm sure" on the confirmation page - # Would post to the same endpoint - response = self.client.post(reverse("admin:market_itemsale_add"), data) - - # Should not have redirected, since there was an error + # Should show form with error and not confirmation page self.assertEqual(response.status_code, 200) expected_templates = [ "admin/market/itemsale/change_form.html", @@ -109,8 +94,10 @@ class TestWithValidators(AdminConfirmTestCase): ] self.assertEqual(response.template_name, expected_templates) self.assertEqual(ItemSale.objects.count(), 0) - self.assertTrue("error" in str(response.content)) - self.assertTrue("Invalid Currency" in str(response.content)) + self.assertIn("error", str(response.rendered_content)) + self.assertIn("Invalid Currency", str(response.rendered_content)) + # Should still ask for confirmation + self.assertIn(CONFIRM_ADD, response.rendered_content) def test_can_confirm_for_models_with_clean_overridden(self): shop = ShopFactory() @@ -162,6 +149,7 @@ class TestWithValidators(AdminConfirmTestCase): item = ItemFactory() InventoryFactory(shop=shop, item=item, quantity=1) transaction = TransactionFactory(shop=shop) + # Asking to buy more than the shop has in stock data = { "transaction": transaction.id, "item": item.id, @@ -176,6 +164,28 @@ class TestWithValidators(AdminConfirmTestCase): # Should not have been added yet self.assertEqual(ItemSale.objects.count(), 0) + # Ensure it shows the form and not the confirmation page + self.assertEqual(response.status_code, 200) + expected_templates = [ + "admin/market/itemsale/change_form.html", + "admin/market/change_form.html", + "admin/change_form.html", + ] + self.assertEqual(response.template_name, expected_templates) + self.assertTrue("error" in str(response.content)) + self.assertTrue( + "Shop does not have enough of the item stocked" in str(response.content) + ) + + # Should still be asking for confirmation + self.assertIn(CONFIRM_ADD, response.rendered_content) + + # Fix the issue by buying only what shop has in stock + data["quantity"] = 1 + # _confirm_add would still be in the POST data + response = self.client.post(reverse("admin:market_itemsale_add"), data) + + # Should show confirmation page # Ensure not redirected (confirmation page does not redirect) self.assertEqual(response.status_code, 200) expected_templates = [ @@ -187,26 +197,6 @@ class TestWithValidators(AdminConfirmTestCase): self._assertSubmitHtml(rendered_content=response.rendered_content) - # Confirmation page would not have the _confirm_add sent on submit - del data["_confirm_add"] - # Selecting to "Yes, I'm sure" on the confirmation page - # Would post to the same endpoint - response = self.client.post(reverse("admin:market_itemsale_add"), data) - - # Should not have redirected, since there was an error - self.assertEqual(response.status_code, 200) - expected_templates = [ - "admin/market/itemsale/change_form.html", - "admin/market/change_form.html", - "admin/change_form.html", - ] - self.assertEqual(response.template_name, expected_templates) - self.assertEqual(ItemSale.objects.count(), 0) - self.assertTrue("error" in str(response.content)) - self.assertTrue( - "Shop does not have enough of the item stocked" in str(response.content) - ) - def test_can_confirm_for_modelform_with_clean_field_and_clean_overridden(self): shop = ShopFactory() data = { @@ -270,27 +260,19 @@ class TestWithValidators(AdminConfirmTestCase): # Should not have been added yet self.assertEqual(Checkout.objects.count(), 0) - # Ensure not redirected (confirmation page does not redirect) + # Should show form with error and not confirmation page self.assertEqual(response.status_code, 200) expected_templates = [ - "admin/market/checkout/change_confirmation.html", - "admin/market/change_confirmation.html", - "admin/change_confirmation.html", + "admin/market/checkout/change_form.html", + "admin/market/change_form.html", + "admin/change_form.html", ] self.assertEqual(response.template_name, expected_templates) - - self._assertSubmitHtml(rendered_content=response.rendered_content) - - # Confirmation page would not have the _confirm_add sent on submit - del data["_confirm_add"] - # Selecting to "Yes, I'm sure" on the confirmation page - # Would post to the same endpoint - response = self.client.post(reverse("admin:market_checkout_add"), data) - print(response.content) - self.assertEqual(response.status_code, 200) self.assertEqual(Checkout.objects.count(), 0) - self.assertIn("error", str(response.content)) - self.assertIn("Invalid Total 111", str(response.content)) + self.assertIn("error", str(response.rendered_content)) + self.assertIn("Invalid Total 111", str(response.rendered_content)) + # Should still ask for confirmation + self.assertIn(CONFIRM_ADD, response.rendered_content) def test_cannot_confirm_for_modelform_with_clean_overridden_if_validation_fails( self, @@ -311,24 +293,19 @@ class TestWithValidators(AdminConfirmTestCase): # Should not have been added yet self.assertEqual(Checkout.objects.count(), 0) - # Ensure not redirected (confirmation page does not redirect) + # Should not have been added yet + self.assertEqual(Checkout.objects.count(), 0) + + # Should show form with error and not confirmation page self.assertEqual(response.status_code, 200) expected_templates = [ - "admin/market/checkout/change_confirmation.html", - "admin/market/change_confirmation.html", - "admin/change_confirmation.html", + "admin/market/checkout/change_form.html", + "admin/market/change_form.html", + "admin/change_form.html", ] self.assertEqual(response.template_name, expected_templates) - - self._assertSubmitHtml(rendered_content=response.rendered_content) - - # Confirmation page would not have the _confirm_add sent on submit - del data["_confirm_add"] - # Selecting to "Yes, I'm sure" on the confirmation page - # Would post to the same endpoint - response = self.client.post(reverse("admin:market_checkout_add"), data) - print(response.content) - self.assertEqual(response.status_code, 200) self.assertEqual(Checkout.objects.count(), 0) - self.assertIn("error", str(response.content)) - self.assertIn("Invalid Total 222", str(response.content)) + self.assertIn("error", str(response.rendered_content)) + self.assertIn("Invalid Total 222", str(response.rendered_content)) + # Should still ask for confirmation + self.assertIn(CONFIRM_ADD, response.rendered_content) diff --git a/admin_confirm/utils.py b/admin_confirm/utils.py index 39f637b..0a67ba6 100644 --- a/admin_confirm/utils.py +++ b/admin_confirm/utils.py @@ -1,12 +1,27 @@ from django.urls import reverse +from admin_confirm.constants import CACHE_KEY_PREFIX, DEBUG def snake_to_title_case(string: str) -> str: return " ".join(string.split("_")).title() -def get_admin_change_url(obj): +def get_admin_change_url(obj: object) -> str: return reverse( "admin:%s_%s_change" % (obj._meta.app_label, obj._meta.model_name), args=(obj.pk,), ) + + +def format_cache_key(model: str, field: str) -> str: + return f"{CACHE_KEY_PREFIX}__{model}__{field}" + + +def log(message: str): # pragma: no cover + if DEBUG: + print(message) + + +def inspect(obj: object): # pragma: no cover + if DEBUG: + print(f"{str(obj): type(obj) - dir(obj)}") diff --git a/setup.cfg b/setup.cfg index aed3ba5..e07435d 100644 --- a/setup.cfg +++ b/setup.cfg @@ -5,11 +5,13 @@ paths = admin_confirm exclude = admin_confirm/tests/* tests/* -ignore = - C812 # missing trailing comma - I001 # isort found an import in the wrong position - I004 # sisort found an unexpected blank line in imports - Q000 # Remove bad quotes +ignore = + D107 # Missing docstring in init + D400 # Doc-string: First line should end with a period + C812 # missing trailing comma + I001 # isort found an import in the wrong position + I004 # sisort found an unexpected blank line in imports + Q000 # Remove bad quotes WPS110 # Seems to require no one word variable names WPS305 # Found f string WPS336 # Explicit string concatination @@ -22,5 +24,5 @@ branch = True [tool:pytest] DJANGO_SETTINGS_MODULE=tests.test_project.settings.test -addopts = --doctest-modules -ra -l --tb=short --show-capture=stdout --color=yes +addopts = --doctest-modules -ra -l --tb=short --show-capture=all --color=yes testpaths = admin_confirm diff --git a/tests/market/admin/checkout_admin.py b/tests/market/admin/checkout_admin.py index 50ec65f..ec5bd50 100644 --- a/tests/market/admin/checkout_admin.py +++ b/tests/market/admin/checkout_admin.py @@ -8,6 +8,9 @@ from ..models import Checkout class CheckoutForm(ModelForm): + search_fields = ["shop", "date"] + confirm_change = True + class Meta: model = Checkout fields = [ diff --git a/tests/market/admin/shop_admin.py b/tests/market/admin/shop_admin.py index e6d492a..1589504 100644 --- a/tests/market/admin/shop_admin.py +++ b/tests/market/admin/shop_admin.py @@ -5,6 +5,7 @@ from admin_confirm.admin import AdminConfirmMixin, confirm_action class ShopAdmin(AdminConfirmMixin, ModelAdmin): confirmation_fields = ["name"] actions = ["show_message", "show_message_no_confirmation"] + search_fields = ["name"] @confirm_action def show_message(modeladmin, request, queryset): diff --git a/tests/market/migrations/0010_checkout_itemsale_transaction.py b/tests/market/migrations/0010_checkout_itemsale_transaction.py index f30bcb4..b766779 100644 --- a/tests/market/migrations/0010_checkout_itemsale_transaction.py +++ b/tests/market/migrations/0010_checkout_itemsale_transaction.py @@ -7,39 +7,89 @@ import django.db.models.deletion class Migration(migrations.Migration): dependencies = [ - ('market', '0009_auto_20210304_0355'), + ("market", "0009_auto_20210304_0355"), ] operations = [ migrations.CreateModel( - name='Transaction', + name="Transaction", fields=[ - ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), - ('timestamp', models.DateField(auto_created=True)), - ('total', models.DecimalField(decimal_places=2, editable=False, max_digits=5)), - ('currency', models.CharField(choices=[('CAD', 'CAD'), ('USD', 'USD')], max_length=3)), - ('shop', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='market.shop')), + ( + "id", + models.AutoField( + auto_created=True, + primary_key=True, + serialize=False, + verbose_name="ID", + ), + ), + ("timestamp", models.DateField(auto_created=True)), + ( + "total", + models.DecimalField(decimal_places=2, editable=False, max_digits=5), + ), + ( + "currency", + models.CharField( + choices=[("CAD", "CAD"), ("USD", "USD")], max_length=3 + ), + ), + ( + "shop", + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, to="market.shop" + ), + ), ], ), migrations.CreateModel( - name='ItemSale', + name="ItemSale", fields=[ - ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), - ('total', models.DecimalField(decimal_places=2, editable=False, max_digits=5)), - ('currency', models.CharField(choices=[('CAD', 'CAD'), ('USD', 'USD')], max_length=3)), - ('item', models.ForeignKey(null=True, on_delete=django.db.models.deletion.SET_NULL, to='market.item')), - ('transaction', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='item_sales', to='market.transaction')), + ( + "id", + models.AutoField( + auto_created=True, + primary_key=True, + serialize=False, + verbose_name="ID", + ), + ), + ( + "total", + models.DecimalField(decimal_places=2, editable=False, max_digits=5), + ), + ( + "currency", + models.CharField( + choices=[("CAD", "CAD"), ("USD", "USD")], max_length=3 + ), + ), + ( + "item", + models.ForeignKey( + null=True, + on_delete=django.db.models.deletion.SET_NULL, + to="market.item", + ), + ), + ( + "transaction", + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + related_name="item_sales", + to="market.transaction", + ), + ), ], ), migrations.CreateModel( - name='Checkout', - fields=[ - ], + name="Checkout", + fields=[], options={ - 'proxy': True, - 'indexes': [], - 'constraints': [], + "proxy": True, + "indexes": [], + "constraints": [], }, - bases=('market.transaction',), + bases=("market.transaction",), ), ] diff --git a/tests/market/migrations/0012_auto_20210326_0240.py b/tests/market/migrations/0012_auto_20210326_0240.py index b43b2c0..2240e90 100644 --- a/tests/market/migrations/0012_auto_20210326_0240.py +++ b/tests/market/migrations/0012_auto_20210326_0240.py @@ -6,18 +6,18 @@ from django.db import migrations, models class Migration(migrations.Migration): dependencies = [ - ('market', '0011_auto_20210326_0130'), + ("market", "0011_auto_20210326_0130"), ] operations = [ migrations.AlterField( - model_name='transaction', - name='date', + model_name="transaction", + name="date", field=models.DateField(), ), migrations.AlterField( - model_name='transaction', - name='timestamp', + model_name="transaction", + name="timestamp", field=models.DateTimeField(auto_created=True), ), ] diff --git a/tests/market/models.py b/tests/market/models.py index 29c66a7..1b112cd 100644 --- a/tests/market/models.py +++ b/tests/market/models.py @@ -52,7 +52,7 @@ class Town(models.Model): class ShoppingMall(models.Model): name = models.CharField(max_length=120) - shops = models.ManyToManyField(Shop, blank=True, null=True) + shops = models.ManyToManyField(Shop, blank=True) general_manager = models.OneToOneField( GeneralManager, on_delete=models.CASCADE, null=True, blank=True ) diff --git a/tests/test_project/settings/base.py b/tests/test_project/settings/base.py index 682a8c8..c046859 100644 --- a/tests/test_project/settings/base.py +++ b/tests/test_project/settings/base.py @@ -23,11 +23,12 @@ SECRET_KEY = "=yddl-40388w3e2hl$e8)revce=n67_idi8pfejtn3!+2%!_qt" # SECURITY WARNING: don't run with debug turned on in production! DEBUG = True +ADMIN_CONFIRM_DEBUG = True -USE_DCOKER = os.environ.get("USE_DOCKER", "").lower() == "true" +USE_DOCKER = os.environ.get("USE_DOCKER", "").lower() == "true" ALLOWED_HOSTS = ["127.0.0.1", "localhost"] -if USE_DCOKER: +if USE_DOCKER: import socket ALLOWED_HOSTS += [socket.gethostbyname(socket.gethostname())]