Skip to content

Commit f477c95

Browse files
Merge branch 'dev' into 960-notifications-add-a-dropdown-with-options-on-the-feedback-form
2 parents f6db0f9 + 8dd24ba commit f477c95

File tree

15 files changed

+1133
-13
lines changed

15 files changed

+1133
-13
lines changed

.github/workflows/run_full_test_suite.yml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,5 +33,10 @@ jobs:
3333
DJANGO_ENV: test
3434
run: docker-compose -f local.yml run --rm django bash ./init.sh
3535

36+
- name: Generate Coverage Report
37+
env:
38+
DJANGO_ENV: test
39+
run: docker-compose -f local.yml run --rm django bash -c "coverage report"
40+
3641
- name: Cleanup
3742
run: docker-compose -f local.yml down --volumes

CHANGELOG.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,3 +23,10 @@ For each PR made, an entry should be added to this changelog. It should contain
2323
- Added a new API endpoint `feedback-form-dropdown-options-api/` where the list is going to be accesible
2424
- Added a list view called `FeedbackFormDropdownListView`
2525
- Added tests
26+
27+
- 1217-add-data-validation-to-the-feedback-form-api-to-restrict-html-content
28+
- Description: The feedback form API does not currently have any form of data validation on the backend which makes it easy for the user with the endpoint to send in data with html tags. We need to have a validation scheme on the backend to protect this from happening.
29+
- Changes:
30+
- Defined a class `HTMLFreeCharField` which inherits `serializers.CharField`
31+
- Used regex to catch any HTML content comming in as an input to form fields
32+
- Called this class within the serializer for necessary fields

compose/local/django/Dockerfile

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ RUN apt-get update && apt-get install --no-install-recommends -y \
5252
&& wget --quiet -O - https://www.postgresql.org/media/keys/ACCC4CF8.asc | apt-key add - \
5353
&& apt-get update \
5454
&& apt-get install -y postgresql-15 postgresql-client-15 \
55+
&& apt-get install -y chromium chromium-driver \
5556
# cleaning up unused files
5657
&& apt-get purge -y --auto-remove -o APT::AutoRemove::RecommendsImportant=false \
5758
&& rm -rf /var/lib/apt/lists/*

docs/architecture-decisions/testing_strategy.md

Lines changed: 184 additions & 0 deletions
Large diffs are not rendered by default.
Lines changed: 283 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,283 @@
1+
# Frontend Testing Methodologies for Django Projects
2+
## Overview
3+
4+
This document outlines testing methodologies for Django projects with HTML forms and JavaScript enhancements, focusing on Python-based testing solutions. While going through the codebase, I can see it is primarily a JavaScript-heavy frontend that uses plain HTML forms enhanced with JavaScript/jQuery rather than server-rendered Django forms. Django forms are being used only in the admin panel of the project.
5+
6+
## Primary Testing Tools
7+
8+
### 1. Selenium with Python (Chosen)
9+
10+
#### Capabilities
11+
- Full browser automation
12+
- JavaScript execution support
13+
- Real DOM interaction
14+
- Cross-browser testing
15+
- Modal dialog handling
16+
- AJAX request testing
17+
- File upload testing
18+
- DataTables interaction
19+
20+
#### Implementation
21+
```python
22+
from selenium import webdriver
23+
from selenium.webdriver.common.by import By
24+
from selenium.webdriver.support.ui import WebDriverWait
25+
from selenium.webdriver.support import expected_conditions as EC
26+
27+
class TestCollectionDetail:
28+
def setup_method(self):
29+
self.driver = webdriver.Chrome()
30+
self.wait = WebDriverWait(self.driver, 10)
31+
32+
def test_title_change_modal(self):
33+
# Example test for title change modal
34+
self.driver.get("/collections/1/")
35+
title_button = self.wait.until(
36+
EC.element_to_be_clickable((By.ID, "change-title-btn"))
37+
)
38+
title_button.click()
39+
40+
modal = self.wait.until(
41+
EC.visibility_of_element_located((By.ID, "title-modal"))
42+
)
43+
44+
form = modal.find_element(By.TAG_NAME, "form")
45+
input_field = form.find_element(By.NAME, "title")
46+
input_field.send_keys("New Title")
47+
form.submit()
48+
49+
# Wait for AJAX completion
50+
self.wait.until(
51+
EC.text_to_be_present_in_element((By.ID, "collection-title"), "New Title")
52+
)
53+
```
54+
55+
#### Pros
56+
- Complete end-to-end testing
57+
- Real browser interaction
58+
- JavaScript support
59+
- Comprehensive API
60+
- Strong community support
61+
62+
#### Drawbacks
63+
- Slower execution
64+
- Browser dependencies
65+
- More complex setup
66+
- Can be flaky with timing issues
67+
68+
### 2. pytest-django with django-test-client
69+
70+
#### Capabilities
71+
- Form submission testing
72+
- Response validation
73+
- Header verification
74+
- Status code checking
75+
- Session handling
76+
- Template rendering testing
77+
78+
#### Implementation
79+
```python
80+
import pytest
81+
from django.urls import reverse
82+
83+
@pytest.mark.django_db
84+
class TestCollectionForms:
85+
def test_collection_create(self, client):
86+
url = reverse('collection_create')
87+
data = {
88+
'title': 'Test Collection',
89+
'division': 'division1',
90+
'workflow_status': 'active'
91+
}
92+
response = client.post(url, data)
93+
assert response.status_code == 302 # Redirect after success
94+
95+
# Verify creation
96+
response = client.get(reverse('collection_detail', kwargs={'pk': 1}))
97+
assert 'Test Collection' in response.content.decode()
98+
```
99+
100+
#### Pros
101+
- Fast execution
102+
- No browser dependency
103+
- Simpler setup
104+
- Integrated with Django
105+
106+
#### Drawbacks
107+
- **No JavaScript support (Dealbreaker)**
108+
- Limited DOM interaction
109+
- Can't test real user interactions
110+
111+
### 3. Playwright for Python
112+
113+
#### Capabilities
114+
- Modern browser automation
115+
- Async/await support
116+
- Network interception
117+
- Mobile device emulation
118+
- Automatic waiting
119+
- Screenshot and video capture
120+
121+
#### Implementation
122+
```python
123+
from playwright.sync_api import sync_playwright
124+
125+
def test_modal_form_submission():
126+
with sync_playwright() as p:
127+
browser = p.chromium.launch()
128+
page = browser.new_page()
129+
130+
page.goto("/collections/")
131+
132+
# Click button to open modal
133+
page.click("#add-collection-btn")
134+
135+
# Fill form in modal
136+
page.fill("#title-input", "New Collection")
137+
page.fill("#division-input", "Division A")
138+
139+
# Submit form
140+
page.click("#submit-btn")
141+
142+
# Wait for success message
143+
success_message = page.wait_for_selector(".toast-success")
144+
assert "Collection created" in success_message.text_content()
145+
146+
browser.close()
147+
```
148+
149+
#### Pros
150+
- Modern API design
151+
- Better stability than Selenium
152+
- Built-in async support
153+
- Powerful debugging tools
154+
155+
#### Drawbacks
156+
- **Newer tool, smaller community (Dealbreaker)**
157+
- Additional system dependencies
158+
- Learning curve for async features
159+
160+
161+
### 4. Beautiful Soup with Requests
162+
A combination for testing HTML structure and content.
163+
164+
**Capabilities:**
165+
- HTML parsing and validation
166+
- Content extraction
167+
- Structure verification
168+
- Link checking
169+
- Form field validation
170+
- Template testing
171+
172+
**Pros:**
173+
- Lightweight solution
174+
- Flexible HTML parsing
175+
- No browser dependency
176+
- Fast execution
177+
- Simple API
178+
- Low resource usage
179+
180+
**Drawbacks:**
181+
- **No JavaScript support (Dealbreaker)**
182+
- Limited interaction testing
183+
- No visual testing
184+
- Basic functionality only
185+
- No real browser simulation
186+
187+
## Feature Comparison Table
188+
189+
| Feature | Selenium | Django Test Client | Playwright | Beautiful Soup |
190+
|---------------------------|----------|-------------------|------------|----------------|
191+
| JavaScript Support | ✅ Yes | ❌ No | ✅ Yes | ❌ No |
192+
| Setup Complexity | 🟡 Medium | 🟢 Low | 🟡 Medium | 🟢 Low |
193+
| Execution Speed | 🔴 Slow | 🟢 Fast | 🟡 Medium | 🟢 Fast |
194+
| Modal Testing | ✅ Yes | ❌ No | ✅ Yes | ❌ No |
195+
| AJAX Testing | ✅ Yes | ❌ No | ✅ Yes | ❌ No |
196+
| Cross-browser Testing | ✅ Yes | ❌ No | ✅ Yes | ❌ No |
197+
| Real User Interaction | ✅ Yes | ❌ No | ✅ Yes | ❌ No |
198+
| Documentation Quality | ✅ Excellent| ✅ Good | ✅ Good | ✅ Good |
199+
| Community Support | ✅ Large | ✅ Large | 🟡 Growing | ✅ Large |
200+
201+
## Testing Strategy Recommendations
202+
203+
1. **Primary Testing Tool**: Selenium with Python
204+
- Best suited for your JavaScript-heavy interface
205+
- Handles modals and AJAX naturally
206+
- Extensive documentation and community support
207+
208+
2. **Test Coverage Areas**:
209+
- Modal form interactions
210+
- AJAX submissions
211+
- DataTables functionality
212+
- Form validation
213+
- Success/error messages
214+
- URL routing
215+
- DOM updates
216+
217+
## Implementation Steps
218+
219+
1. Add testing dependencies to requirements file `requirements/local.txt`:
220+
```text
221+
# Testing Dependencies
222+
selenium>=4.15.2
223+
pytest-xdist>=3.3.1
224+
pytest-cov>=4.1.0
225+
```
226+
227+
2. Update Dockerfile `compose/local/django/Dockerfile` to install Chrome and ChromeDriver:
228+
```dockerfile
229+
# Install Chrome and ChromeDriver for Selenium tests
230+
RUN apt-get update && apt-get install -y \
231+
chromium \
232+
chromium-driver \
233+
&& rm -rf /var/lib/apt/lists/*
234+
```
235+
236+
3. Rebuild Docker container to apply changes:
237+
```bash
238+
docker-compose -f local.yml build django
239+
```
240+
241+
4. Create test directory structure:
242+
```bash
243+
mkdir -p tests/frontend
244+
touch tests/frontend/__init__.py
245+
touch tests/frontend/base.py
246+
touch tests/frontend/test_setup.py
247+
```
248+
249+
5. Create base test classes:
250+
```python
251+
import pytest
252+
from selenium import webdriver
253+
254+
class BaseUITest:
255+
@pytest.fixture(autouse=True)
256+
def setup_class(self):
257+
self.driver = webdriver.Chrome()
258+
yield
259+
self.driver.quit()
260+
261+
def login(self):
262+
# Common login logic
263+
pass
264+
```
265+
266+
6. Organize tests by feature:
267+
```python
268+
class TestCollectionManagement(BaseUITest):
269+
def test_create_collection(self):
270+
pass
271+
272+
def test_edit_collection(self):
273+
pass
274+
275+
class TestURLPatterns(BaseUITest):
276+
def test_add_include_pattern(self):
277+
pass
278+
```
279+
280+
7. Run tests:
281+
```bash
282+
docker-compose -f local.yml run --rm django pytest tests/frontend/test_setup.py -v
283+
```

feedback/serializers.py

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import re
2+
13
from rest_framework import serializers
24

35
from .models import ContentCurationRequest, Feedback, FeedbackFormDropdown
@@ -9,7 +11,23 @@ class Meta:
911
fields = ["id", "name"]
1012

1113

14+
class HTMLFreeCharField(serializers.CharField):
15+
def to_internal_value(self, data):
16+
value = super().to_internal_value(data)
17+
18+
if re.search(r"<[^>]+>", value):
19+
raise serializers.ValidationError("HTML tags are not allowed in this field")
20+
21+
return value
22+
23+
1224
class FeedbackSerializer(serializers.ModelSerializer):
25+
26+
name = HTMLFreeCharField()
27+
subject = HTMLFreeCharField()
28+
comments = HTMLFreeCharField()
29+
source = HTMLFreeCharField()
30+
1331
class Meta:
1432
model = Feedback
1533
fields = [

init.sh

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
#!/bin/bash
2-
echo "Running all test cases across the project..."
2+
echo "Running all test cases across the project with coverage analysis..."
33

44
# Initialize a failure counter
55
failure_count=0
@@ -10,10 +10,20 @@ excluded_dirs="document_classifier functional_tests"
1010
# Find all test files except those in excluded directories
1111
test_files=$(find . -type f -name "test_*.py" | grep -Ev "$(echo $excluded_dirs | sed 's/ /|/g')")
1212

13-
# Run each test file
13+
coverage erase # Clear any existing coverage data
14+
15+
# Setup .coveragerc configuration to include all Python files
16+
echo "[run]
17+
source = .
18+
include = */*.py
19+
20+
[report]
21+
show_missing = True" > .coveragerc
22+
23+
# Run each test file with coverage (without generating report yet)
1424
for test_file in $test_files; do
1525
echo "Running $test_file..."
16-
pytest "$test_file"
26+
coverage run --append -m pytest "$test_file" # Collect coverage data
1727

1828
# Check the exit status of pytest
1929
if [ $? -ne 0 ]; then
@@ -22,10 +32,11 @@ for test_file in $test_files; do
2232
fi
2333
done
2434

25-
# Report the results
35+
# Report the results without generating the coverage report
2636
if [ $failure_count -ne 0 ]; then
27-
echo "$failure_count test(s) failed."
37+
echo "$failure_count test(s) failed. Refer to the terminal output for details."
2838
exit 1
2939
else
3040
echo "All tests passed successfully!"
41+
echo "Coverage data collected. Coverage report will be generated separately."
3142
fi

requirements/local.txt

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,10 @@ pytest==8.0.0 # https://github.com/pytest-dev/pytest
1313
pytest-sugar==1.0.0 # https://github.com/Frozenball/pytest-sugar
1414
types-requests # maybe instead, we should add `mypy --install-types` to the dockerfile?
1515
types-xmltodict
16+
pytest-xdist>=3.3.1
17+
pytest-cov>=4.1.0
18+
selenium>=4.15.2 # Selenium (Frontend Testing)
19+
coverage==7.4.1
1620

1721
# Documentation
1822
# ------------------------------------------------------------------------------

sde_collections/tests/frontend/__init__.py

Whitespace-only changes.

0 commit comments

Comments
 (0)