Skip to content

Commit df9e6e8

Browse files
TrangPhamTrangPham
authored
CHORE: Clean up code smells and other minor stuff (#68)
* CHORE: Clean up code smells and other minor stuff * ISSUE-52: Fix localstack dev container create s3 bucket * Some more minor review dr stuff * Adding related_name for all foreign keys
1 parent 064aab4 commit df9e6e8

17 files changed

+143
-191
lines changed

‎admin_confirm/admin.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ def _get_changed_data(self, form: ModelForm, model: Model, obj: object, add: boo
145145
"""
146146

147147
def _display_for_changed_data(field, initial_value, new_value):
148-
if not (isinstance(field, FileField) or isinstance(field, ImageField)):
148+
if not isinstance(field, (FileField, ImageField)):
149149
return [initial_value, new_value]
150150

151151
if initial_value:
@@ -238,7 +238,7 @@ def _reconstruct_request_files():
238238
query_dict = request.POST
239239

240240
for field in self.model._meta.get_fields():
241-
if not (isinstance(field, FileField) or isinstance(field, ImageField)):
241+
if not isinstance(field, (FileField, ImageField)):
242242
continue
243243

244244
cached_file = self._file_cache.get(

‎admin_confirm/file_cache.py‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
""" FileCache - caches files for ModelAdmins with confirmations.
1+
"""FileCache - caches files for ModelAdmins with confirmations.
22
33
Code modified from: https://github.com/MaistrenkoAnton/filefield-cache/blob/master/filefield_cache/cache.py
44
Original copy date: April 22, 2021
@@ -27,6 +27,7 @@
2727
OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
2828
SOFTWARE.
2929
"""
30+
3031
from django.core.files.uploadedfile import InMemoryUploadedFile
3132

3233
try:
@@ -40,7 +41,7 @@
4041
from admin_confirm.utils import log
4142

4243

43-
class FileCache(object):
44+
class FileCache:
4445
"Cache file data and retain the file upon confirmation."
4546

4647
timeout = CACHE_TIMEOUT

‎admin_confirm/tests/integration/test_with_cache.py‎

Lines changed: 26 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
Tests confirmation of add/change
33
on ModelAdmin that utilize caches
44
"""
5+
56
import os
67
import pytest
78
import pkg_resources
@@ -30,9 +31,7 @@ def tearDown(self):
3031

3132
def test_models_without_files_should_not_have_confirmation_received(self):
3233
mall = ShoppingMall.objects.create(name="mall")
33-
self.selenium.get(
34-
self.live_server_url + f"/admin/market/shoppingmall/{mall.id}/change/"
35-
)
34+
self.selenium.get(self.live_server_url + f"/admin/market/shoppingmall/{mall.id}/change/")
3635
# Should ask for confirmation of change
3736
self.assertIn(CONFIRM_CHANGE, self.selenium.page_source)
3837

@@ -59,9 +58,7 @@ def test_models_without_files_should_not_have_confirmation_received(self):
5958

6059
def test_models_with_files_should_have_confirmation_received(self):
6160
item = Item.objects.create(name="item", price=1)
62-
self.selenium.get(
63-
self.live_server_url + f"/admin/market/item/{item.id}/change/"
64-
)
61+
self.selenium.get(self.live_server_url + f"/admin/market/item/{item.id}/change/")
6562
# Should ask for confirmation of change
6663
self.assertIn(CONFIRM_CHANGE, self.selenium.page_source)
6764

@@ -90,23 +87,17 @@ def test_should_save_file_additions(self):
9087
"Known issue `https://github.com/SeleniumHQ/selenium/issues/8762` with this selenium version."
9188
)
9289

93-
item = Item.objects.create(
94-
name="item", price=1, currency=Item.VALID_CURRENCIES[0][0]
95-
)
90+
item = Item.objects.create(name="item", price=1, currency=Item.VALID_CURRENCIES[0][0])
9691

97-
self.selenium.get(
98-
self.live_server_url + f"/admin/market/item/{item.id}/change/"
99-
)
92+
self.selenium.get(self.live_server_url + f"/admin/market/item/{item.id}/change/")
10093
self.assertIn(CONFIRM_CHANGE, self.selenium.page_source)
10194

10295
# Make a change to trigger confirmation page
10396
price = self.selenium.find_element(By.NAME, "price")
10497
price.send_keys(2)
10598

10699
# Upload a new file
107-
self.selenium.find_element(By.ID, "id_file").send_keys(
108-
os.getcwd() + "/screenshot.png"
109-
)
100+
self.selenium.find_element(By.ID, "id_file").send_keys(os.getcwd() + "/screenshot.png")
110101

111102
self.selenium.find_element(By.NAME, "_continue").click()
112103

@@ -130,28 +121,26 @@ def test_should_save_file_changes(self):
130121
"Known issue `https://github.com/SeleniumHQ/selenium/issues/8762` with this selenium version."
131122
)
132123

133-
file = SimpleUploadedFile(
134-
name="old_file.jpg",
135-
content=open("screenshot.png", "rb").read(),
136-
content_type="image/jpeg",
137-
)
124+
with open("screenshot.png", "rb") as f:
125+
file = SimpleUploadedFile(
126+
name="old_file.jpg",
127+
content=f.read(),
128+
content_type="image/jpeg",
129+
)
130+
138131
item = Item.objects.create(
139132
name="item", price=1, currency=Item.VALID_CURRENCIES[0][0], file=file
140133
)
141134

142-
self.selenium.get(
143-
self.live_server_url + f"/admin/market/item/{item.id}/change/"
144-
)
135+
self.selenium.get(self.live_server_url + f"/admin/market/item/{item.id}/change/")
145136
self.assertIn(CONFIRM_CHANGE, self.selenium.page_source)
146137

147138
# Make a change to trigger confirmation page
148139
price = self.selenium.find_element(By.NAME, "price")
149140
price.send_keys(2)
150141

151142
# Upload a new file
152-
self.selenium.find_element(By.ID, "id_file").send_keys(
153-
os.getcwd() + "/screenshot.png"
154-
)
143+
self.selenium.find_element(By.ID, "id_file").send_keys(os.getcwd() + "/screenshot.png")
155144

156145
self.selenium.find_element(By.NAME, "_continue").click()
157146

@@ -169,18 +158,18 @@ def test_should_save_file_changes(self):
169158
self.assertRegex(item.file.name, r"screenshot.*\.png$")
170159

171160
def test_should_remove_file_if_clear_selected(self):
172-
file = SimpleUploadedFile(
173-
name="old_file.jpg",
174-
content=open("screenshot.png", "rb").read(),
175-
content_type="image/jpeg",
176-
)
161+
with open("screenshot.png", "rb") as f:
162+
file = SimpleUploadedFile(
163+
name="old_file.jpg",
164+
content=f.read(),
165+
content_type="image/jpeg",
166+
)
167+
177168
item = Item.objects.create(
178169
name="item", price=1, currency=Item.VALID_CURRENCIES[0][0], file=file
179170
)
180171

181-
self.selenium.get(
182-
self.live_server_url + f"/admin/market/item/{item.id}/change/"
183-
)
172+
self.selenium.get(self.live_server_url + f"/admin/market/item/{item.id}/change/")
184173
self.assertIn(CONFIRM_CHANGE, self.selenium.page_source)
185174

186175
# Make a change to trigger confirmation page
@@ -190,9 +179,9 @@ def test_should_remove_file_if_clear_selected(self):
190179
# Choose to clear the existing file
191180
self.selenium.find_element(By.ID, "file-clear_id").click()
192181
self.assertTrue(
193-
self.selenium.find_element(
194-
By.XPATH, ".//*[@id='file-clear_id']"
195-
).get_attribute("checked")
182+
self.selenium.find_element(By.XPATH, ".//*[@id='file-clear_id']").get_attribute(
183+
"checked"
184+
)
196185
)
197186

198187
self.selenium.find_element(By.NAME, "_continue").click()

‎admin_confirm/tests/integration/test_with_form_input_types.py‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
"""
22
Tests with different form input types
33
"""
4+
45
from datetime import timedelta
56
from django.utils import timezone
67
from importlib import reload
@@ -36,7 +37,7 @@ def tearDownClass(cls):
3637
return super().tearDownClass()
3738

3839
def test_radio_input_should_work(self):
39-
self.selenium.get(self.live_server_url + f"/admin/market/item/add/")
40+
self.selenium.get(self.live_server_url + "/admin/market/item/add/")
4041
self.assertIn("radio", self.selenium.page_source)
4142

4243
# Should ask for confirmation of add
@@ -76,9 +77,7 @@ def test_raw_id_fields_should_work(self):
7677
mall = ShoppingMall.objects.create(name="mall", general_manager=gm1, town=town)
7778
mall.shops.set(shops)
7879

79-
self.selenium.get(
80-
self.live_server_url + f"/admin/market/shoppingmall/{mall.id}/change/"
81-
)
80+
self.selenium.get(self.live_server_url + f"/admin/market/shoppingmall/{mall.id}/change/")
8281
self.assertIn(CONFIRM_CHANGE, self.selenium.page_source)
8382

8483
# Make a change to trigger confirmation page

‎admin_confirm/tests/integration/test_with_s3_storage.py‎

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,13 @@ def setUp(self):
3737
# Delete all current files
3838
for obj in self.bucket.objects.all():
3939
obj.delete()
40+
41+
with open("screenshot.png", "rb") as f:
42+
self.file = SimpleUploadedFile(
43+
name="old_file.jpg",
44+
content=f.read(),
45+
content_type="image/jpeg",
46+
)
4047
super().setUp()
4148

4249
def tearDown(self):
@@ -109,13 +116,8 @@ def test_should_save_file_changes(self):
109116
"Known issue `https://github.com/SeleniumHQ/selenium/issues/8762` with this selenium version."
110117
)
111118

112-
file = SimpleUploadedFile(
113-
name="old_file.jpg",
114-
content=open("screenshot.png", "rb").read(),
115-
content_type="image/jpeg",
116-
)
117119
item = Item.objects.create(
118-
name="item", price=1, currency=Item.VALID_CURRENCIES[0][0], file=file
120+
name="item", price=1, currency=Item.VALID_CURRENCIES[0][0], file=self.file
119121
)
120122

121123
self.selenium.get(self.live_server_url + f"/admin/market/item/{item.id}/change/")
@@ -152,13 +154,8 @@ def test_should_save_file_changes(self):
152154
self.assertRegex(objects_by_last_modified[0].key, r"old_file.*\.jpg$")
153155

154156
def test_should_remove_file_if_clear_selected(self):
155-
file = SimpleUploadedFile(
156-
name="old_file.jpg",
157-
content=open("screenshot.png", "rb").read(),
158-
content_type="image/jpeg",
159-
)
160157
item = Item.objects.create(
161-
name="item", price=1, currency=Item.VALID_CURRENCIES[0][0], file=file
158+
name="item", price=1, currency=Item.VALID_CURRENCIES[0][0], file=self.file
162159
)
163160

164161
self.selenium.get(self.live_server_url + f"/admin/market/item/{item.id}/change/")

‎admin_confirm/tests/integration/test_with_validators_integration.py‎

Lines changed: 11 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -2,34 +2,22 @@
22
Ensures that confirmations work with validators on the Model and on the Modelform.
33
"""
44

5-
from logging import currentframe
5+
from importlib import reload
6+
from selenium.webdriver.common.by import By
67
from unittest import mock
7-
from django.urls import reverse
8-
from django.utils import timezone
98

10-
from admin_confirm.tests.helpers import AdminConfirmTestCase
11-
from tests.market.models import Checkout, ItemSale
9+
from admin_confirm.constants import CONFIRM_ADD
10+
from admin_confirm.tests.helpers import AdminConfirmIntegrationTestCase
11+
12+
from tests.market.models import ItemSale
1213
from tests.factories import (
1314
InventoryFactory,
1415
ItemFactory,
1516
ShopFactory,
1617
TransactionFactory,
1718
)
18-
19-
20-
import pytest
21-
import pkg_resources
22-
from importlib import reload
23-
from tests.factories import ShopFactory
24-
from tests.market.models import GeneralManager, ShoppingMall, Town
25-
26-
from admin_confirm.tests.helpers import AdminConfirmIntegrationTestCase
2719
from tests.market.admin import shoppingmall_admin
2820

29-
from admin_confirm.constants import CONFIRM_ADD, CONFIRM_CHANGE
30-
from selenium.webdriver.support.ui import Select
31-
from selenium.webdriver.common.by import By
32-
3321

3422
class ConfirmWithValidatorsTests(AdminConfirmIntegrationTestCase):
3523
def setUp(self):
@@ -47,7 +35,7 @@ def test_can_confirm_for_models_with_validator_on_model_field(self, _mock_clean)
4735
ItemFactory()
4836
TransactionFactory()
4937

50-
self.selenium.get(self.live_server_url + f"/admin/market/itemsale/add/")
38+
self.selenium.get(self.live_server_url + "/admin/market/itemsale/add/")
5139
# Should ask for confirmation of change
5240
self.assertIn(CONFIRM_ADD, self.selenium.page_source)
5341

@@ -81,7 +69,7 @@ def test_cannot_confirm_for_models_with_validator_on_model_field_if_validator_fa
8169
InventoryFactory(shop=shop, item=item, quantity=10)
8270
TransactionFactory(shop=shop)
8371

84-
self.selenium.get(self.live_server_url + f"/admin/market/itemsale/add/")
72+
self.selenium.get(self.live_server_url + "/admin/market/itemsale/add/")
8573
# Should ask for confirmation of change
8674
self.assertIn(CONFIRM_ADD, self.selenium.page_source)
8775

@@ -124,7 +112,7 @@ def test_can_confirm_for_models_with_clean_overridden(self):
124112
InventoryFactory(shop=shop, item=item, quantity=10)
125113
transaction = TransactionFactory(shop=shop)
126114

127-
self.selenium.get(self.live_server_url + f"/admin/market/itemsale/add/")
115+
self.selenium.get(self.live_server_url + "/admin/market/itemsale/add/")
128116
# Should ask for confirmation of change
129117
self.assertIn(CONFIRM_ADD, self.selenium.page_source)
130118

@@ -163,7 +151,7 @@ def test_cannot_confirm_for_models_with_clean_overridden_if_clean_fails(self):
163151
InventoryFactory(shop=shop, item=item, quantity=1)
164152
TransactionFactory(shop=shop)
165153

166-
self.selenium.get(self.live_server_url + f"/admin/market/itemsale/add/")
154+
self.selenium.get(self.live_server_url + "/admin/market/itemsale/add/")
167155
# Should ask for confirmation of change
168156
self.assertIn(CONFIRM_ADD, self.selenium.page_source)
169157

@@ -177,9 +165,7 @@ def test_cannot_confirm_for_models_with_clean_overridden_if_clean_fails(self):
177165

178166
# Should show errors and not confirmation page
179167
self.assertNotIn("Confirm", self.selenium.page_source)
180-
self.assertIn(
181-
"Shop does not have enough of the item stocked", self.selenium.page_source
182-
)
168+
self.assertIn("Shop does not have enough of the item stocked", self.selenium.page_source)
183169
self.assertIn(CONFIRM_ADD, self.selenium.page_source)
184170

185171
# Should not have been added yet

0 commit comments

Comments
 (0)