Add CI workflow for backend validation and webapp build #1057
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Reviewer's GuideThis PR adds a multi-stage GitHub Actions workflow to validate the Python/Django backend (dependency sync, system checks, migrations, integration tests, smoke test) and build the Vue/Vite webapp, and it enhances the pytest suite with reusable fixtures and comprehensive tests for control panel, public presale, common utilities, and healthcheck endpoints. Sequence diagram for backend smoke test healthcheck validationsequenceDiagram
participant CI as "GitHub Actions Runner"
participant Postgres as "PostgreSQL 16"
participant Redis as "Redis 7"
participant Django as "Django Dev Server"
CI->>Postgres: Start service
CI->>Redis: Start service
CI->>Django: Run migrations
CI->>Django: Start server
CI->>Django: Request /healthcheck/
Django->>Postgres: Check DB connection
Django->>Redis: Check Redis connection
Django-->>CI: Return healthcheck response
CI->>Django: Stop server
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- Consider DRYing up your GitHub Actions workflow by extracting repeated steps (e.g., checkout, setup-python/node, pip install uv) and service definitions into reusable anchors or composite actions to reduce duplication.
- Many tests currently assert a broad set of status codes (e.g. [200, 302, 404]), which can hide real failures—narrow those down to the exact expected responses to catch unintended breaks.
- Prefer using Django's reverse() (or named URL patterns) in tests instead of hard-coded URL strings to improve maintainability and reduce brittleness if routes change.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider DRYing up your GitHub Actions workflow by extracting repeated steps (e.g., checkout, setup-python/node, pip install uv) and service definitions into reusable anchors or composite actions to reduce duplication.
- Many tests currently assert a broad set of status codes (e.g. [200, 302, 404]), which can hide real failures—narrow those down to the exact expected responses to catch unintended breaks.
- Prefer using Django's reverse() (or named URL patterns) in tests instead of hard-coded URL strings to improve maintainability and reduce brittleness if routes change.
## Individual Comments
### Comment 1
<location> `app/tests/test_control.py:20-38` </location>
<code_context>
+ # Should redirect to login
+ assert response.status_code == 302
+
+ def test_organizer_dashboard_with_auth(self, organizer_client, organizer):
+ """Test organizer dashboard accessible for team members."""
+ url = f'/orga/organizer/{organizer.slug}/'
+ response = organizer_client.get(url)
+ # Should load or redirect to appropriate page
+ assert response.status_code in [200, 302]
+
+ def test_event_list_with_auth(self, organizer_client):
</code_context>
<issue_to_address>
**suggestion (testing):** Add assertions to verify page content or expected redirects, not just status codes.
Add checks for the rendered template, redirect URL, or specific page content to improve test coverage.
```suggestion
def test_organizer_dashboard_requires_auth(self, client, organizer):
"""Test that organizer dashboard requires authentication."""
url = f'/orga/organizer/{organizer.slug}/'
response = client.get(url)
# Should redirect to login
assert response.status_code == 302
assert response.url.startswith('/orga/login') or '/login' in response.url
def test_organizer_dashboard_with_auth(self, organizer_client, organizer):
"""Test organizer dashboard accessible for team members."""
url = f'/orga/organizer/{organizer.slug}/'
response = organizer_client.get(url)
# Should load or redirect to appropriate page
assert response.status_code in [200, 302]
if response.status_code == 302:
# Should redirect to event list or dashboard
assert '/orga/event/' in response.url or '/orga/dashboard/' in response.url
else:
# Should contain dashboard content
assert b"Dashboard" in response.content or b"Events" in response.content
def test_event_list_with_auth(self, organizer_client):
"""Test event list page for authenticated organizer."""
response = organizer_client.get('/orga/event/')
# Should show event list
assert response.status_code == 200
assert b"Event List" in response.content or b"Events" in response.content
```
</issue_to_address>
### Comment 2
<location> `app/tests/test_presale.py:26-41` </location>
<code_context>
+class TestEventPages:
+ """Test event-specific public pages."""
+
+ def test_event_landing_page(self, client, organizer, event):
+ """Test that event landing page loads."""
+ url = f'/{organizer.slug}/{event.slug}/'
+ response = client.get(url)
+ # May return 200 or redirect depending on event setup
+ assert response.status_code in [200, 301, 302, 404]
+
+ def test_robots_txt(self, client):
</code_context>
<issue_to_address>
**suggestion (testing):** Test for invalid event/organizer slugs and ensure proper error handling.
Please add tests for invalid or non-existent organizer/event slugs to verify that the application returns the correct error codes and handles these cases gracefully.
```suggestion
@pytest.mark.django_db
class TestEventPages:
"""Test event-specific public pages."""
def test_event_landing_page(self, client, organizer, event):
"""Test that event landing page loads."""
url = f'/{organizer.slug}/{event.slug}/'
response = client.get(url)
# May return 200 or redirect depending on event setup
assert response.status_code in [200, 301, 302, 404]
def test_invalid_organizer_slug(self, client, event):
"""Test that an invalid organizer slug returns 404."""
url = f'/nonexistent-organizer/{event.slug}/'
response = client.get(url)
assert response.status_code == 404
def test_invalid_event_slug(self, client, organizer):
"""Test that an invalid event slug returns 404."""
url = f'/{organizer.slug}/nonexistent-event/'
response = client.get(url)
assert response.status_code == 404
def test_invalid_organizer_and_event_slug(self, client):
"""Test that both invalid organizer and event slugs return 404."""
url = '/nonexistent-organizer/nonexistent-event/'
response = client.get(url)
assert response.status_code == 404
def test_robots_txt(self, client):
"""Test that robots.txt is accessible."""
response = client.get('/robots.txt')
assert response.status_code == 200
assert 'text/plain' in response['Content-Type']
```
</issue_to_address>
### Comment 3
<location> `app/tests/test_common.py:23-27` </location>
<code_context>
+ # Should handle redirect requests
+ assert response.status_code in [200, 302, 400]
+
+ def test_csp_report_endpoint(self, client):
+ """Test CSP report endpoint accepts POST."""
+ response = client.post('/csp_report/', data='{}', content_type='application/json')
+ # Should accept CSP reports
+ assert response.status_code in [200, 204]
+
+ def test_metrics_endpoint(self, client):
</code_context>
<issue_to_address>
**suggestion (testing):** Consider testing CSP report endpoint with malformed or missing data.
Add tests for malformed or missing JSON in requests to the CSP report endpoint and check for appropriate error responses, such as 400 Bad Request.
```suggestion
def test_csp_report_endpoint_malformed_json(self, client):
"""Test CSP report endpoint with malformed JSON."""
response = client.post('/csp_report/', data='{bad json}', content_type='application/json')
# Should reject malformed JSON
assert response.status_code == 400
def test_csp_report_endpoint_missing_data(self, client):
"""Test CSP report endpoint with missing data."""
response = client.post('/csp_report/', data='', content_type='application/json')
# Should reject missing data
assert response.status_code == 400
def test_metrics_endpoint(self, client):
"""Test metrics endpoint (may require auth)."""
response = client.get('/metrics')
# May require auth or return metrics
assert response.status_code in [200, 401, 403]
```
</issue_to_address>
### Comment 4
<location> `app/tests/test_healthcheck.py:12-15` </location>
<code_context>
+class TestHealthcheck:
+ """Test the /healthcheck/ endpoint."""
+
+ def test_healthcheck_returns_200(self, client):
+ """Healthcheck should return 200 OK when services are available."""
+ response = client.get('/healthcheck/')
+ assert response.status_code == 200
+
+ def test_healthcheck_empty_body(self, client):
</code_context>
<issue_to_address>
**suggestion (testing):** Add tests for healthcheck endpoint when dependencies (DB, Redis) are unavailable.
Please add tests that simulate DB and Redis outages to confirm the healthcheck endpoint returns the expected error status and message.
Suggested implementation:
```python
def test_healthcheck_content_type(self, client):
"""Healthcheck returns text/html content type."""
response = client.get('/healthcheck/')
assert 'text/html' in response['Content-Type']
def test_healthcheck_db_unavailable(self, client, mocker):
"""Healthcheck returns 503 if DB is unavailable."""
# Patch the DB check used in the healthcheck view to raise an exception
mocker.patch('app.views.healthcheck.check_database', side_effect=Exception("DB unavailable"))
response = client.get('/healthcheck/')
assert response.status_code == 503
assert b"DB unavailable" in response.content
def test_healthcheck_redis_unavailable(self, client, mocker):
"""Healthcheck returns 503 if Redis is unavailable."""
# Patch the Redis check used in the healthcheck view to raise an exception
mocker.patch('app.views.healthcheck.check_redis', side_effect=Exception("Redis unavailable"))
response = client.get('/healthcheck/')
assert response.status_code == 503
assert b"Redis unavailable" in response.content
```
- Ensure that your healthcheck view in `app/views/healthcheck.py` uses `check_database` and `check_redis` functions for dependency checks, and that it returns a 503 status code and error message when those checks fail.
- If the healthcheck view uses different function names or locations for DB/Redis checks, update the patch targets accordingly.
- The `mocker` fixture is provided by pytest-mock. If not already present, add `pytest-mock` to your test dependencies.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+20
to
+38
| def test_organizer_dashboard_requires_auth(self, client, organizer): | ||
| """Test that organizer dashboard requires authentication.""" | ||
| url = f'/orga/organizer/{organizer.slug}/' | ||
| response = client.get(url) | ||
| # Should redirect to login | ||
| assert response.status_code == 302 | ||
|
|
||
| def test_organizer_dashboard_with_auth(self, organizer_client, organizer): | ||
| """Test organizer dashboard accessible for team members.""" | ||
| url = f'/orga/organizer/{organizer.slug}/' | ||
| response = organizer_client.get(url) | ||
| # Should load or redirect to appropriate page | ||
| assert response.status_code in [200, 302] | ||
|
|
||
| def test_event_list_with_auth(self, organizer_client): | ||
| """Test event list page for authenticated organizer.""" | ||
| response = organizer_client.get('/orga/event/') | ||
| # Should show event list | ||
| assert response.status_code == 200 |
There was a problem hiding this comment.
suggestion (testing): Add assertions to verify page content or expected redirects, not just status codes.
Add checks for the rendered template, redirect URL, or specific page content to improve test coverage.
Suggested change
| def test_organizer_dashboard_requires_auth(self, client, organizer): | |
| """Test that organizer dashboard requires authentication.""" | |
| url = f'/orga/organizer/{organizer.slug}/' | |
| response = client.get(url) | |
| # Should redirect to login | |
| assert response.status_code == 302 | |
| def test_organizer_dashboard_with_auth(self, organizer_client, organizer): | |
| """Test organizer dashboard accessible for team members.""" | |
| url = f'/orga/organizer/{organizer.slug}/' | |
| response = organizer_client.get(url) | |
| # Should load or redirect to appropriate page | |
| assert response.status_code in [200, 302] | |
| def test_event_list_with_auth(self, organizer_client): | |
| """Test event list page for authenticated organizer.""" | |
| response = organizer_client.get('/orga/event/') | |
| # Should show event list | |
| assert response.status_code == 200 | |
| def test_organizer_dashboard_requires_auth(self, client, organizer): | |
| """Test that organizer dashboard requires authentication.""" | |
| url = f'/orga/organizer/{organizer.slug}/' | |
| response = client.get(url) | |
| # Should redirect to login | |
| assert response.status_code == 302 | |
| assert response.url.startswith('/orga/login') or '/login' in response.url | |
| def test_organizer_dashboard_with_auth(self, organizer_client, organizer): | |
| """Test organizer dashboard accessible for team members.""" | |
| url = f'/orga/organizer/{organizer.slug}/' | |
| response = organizer_client.get(url) | |
| # Should load or redirect to appropriate page | |
| assert response.status_code in [200, 302] | |
| if response.status_code == 302: | |
| # Should redirect to event list or dashboard | |
| assert '/orga/event/' in response.url or '/orga/dashboard/' in response.url | |
| else: | |
| # Should contain dashboard content | |
| assert b"Dashboard" in response.content or b"Events" in response.content | |
| def test_event_list_with_auth(self, organizer_client): | |
| """Test event list page for authenticated organizer.""" | |
| response = organizer_client.get('/orga/event/') | |
| # Should show event list | |
| assert response.status_code == 200 | |
| assert b"Event List" in response.content or b"Events" in response.content |
Comment on lines
+26
to
+41
| @pytest.mark.django_db | ||
| class TestEventPages: | ||
| """Test event-specific public pages.""" | ||
|
|
||
| def test_event_landing_page(self, client, organizer, event): | ||
| """Test that event landing page loads.""" | ||
| url = f'/{organizer.slug}/{event.slug}/' | ||
| response = client.get(url) | ||
| # May return 200 or redirect depending on event setup | ||
| assert response.status_code in [200, 301, 302, 404] | ||
|
|
||
| def test_robots_txt(self, client): | ||
| """Test that robots.txt is accessible.""" | ||
| response = client.get('/robots.txt') | ||
| assert response.status_code == 200 | ||
| assert 'text/plain' in response['Content-Type'] |
There was a problem hiding this comment.
suggestion (testing): Test for invalid event/organizer slugs and ensure proper error handling.
Please add tests for invalid or non-existent organizer/event slugs to verify that the application returns the correct error codes and handles these cases gracefully.
Suggested change
| @pytest.mark.django_db | |
| class TestEventPages: | |
| """Test event-specific public pages.""" | |
| def test_event_landing_page(self, client, organizer, event): | |
| """Test that event landing page loads.""" | |
| url = f'/{organizer.slug}/{event.slug}/' | |
| response = client.get(url) | |
| # May return 200 or redirect depending on event setup | |
| assert response.status_code in [200, 301, 302, 404] | |
| def test_robots_txt(self, client): | |
| """Test that robots.txt is accessible.""" | |
| response = client.get('/robots.txt') | |
| assert response.status_code == 200 | |
| assert 'text/plain' in response['Content-Type'] | |
| @pytest.mark.django_db | |
| class TestEventPages: | |
| """Test event-specific public pages.""" | |
| def test_event_landing_page(self, client, organizer, event): | |
| """Test that event landing page loads.""" | |
| url = f'/{organizer.slug}/{event.slug}/' | |
| response = client.get(url) | |
| # May return 200 or redirect depending on event setup | |
| assert response.status_code in [200, 301, 302, 404] | |
| def test_invalid_organizer_slug(self, client, event): | |
| """Test that an invalid organizer slug returns 404.""" | |
| url = f'/nonexistent-organizer/{event.slug}/' | |
| response = client.get(url) | |
| assert response.status_code == 404 | |
| def test_invalid_event_slug(self, client, organizer): | |
| """Test that an invalid event slug returns 404.""" | |
| url = f'/{organizer.slug}/nonexistent-event/' | |
| response = client.get(url) | |
| assert response.status_code == 404 | |
| def test_invalid_organizer_and_event_slug(self, client): | |
| """Test that both invalid organizer and event slugs return 404.""" | |
| url = '/nonexistent-organizer/nonexistent-event/' | |
| response = client.get(url) | |
| assert response.status_code == 404 | |
| def test_robots_txt(self, client): | |
| """Test that robots.txt is accessible.""" | |
| response = client.get('/robots.txt') | |
| assert response.status_code == 200 | |
| assert 'text/plain' in response['Content-Type'] |
Comment on lines
+23
to
+27
| def test_metrics_endpoint(self, client): | ||
| """Test metrics endpoint (may require auth).""" | ||
| response = client.get('/metrics') | ||
| # May require auth or return metrics | ||
| assert response.status_code in [200, 401, 403] |
There was a problem hiding this comment.
suggestion (testing): Consider testing CSP report endpoint with malformed or missing data.
Add tests for malformed or missing JSON in requests to the CSP report endpoint and check for appropriate error responses, such as 400 Bad Request.
Suggested change
| def test_metrics_endpoint(self, client): | |
| """Test metrics endpoint (may require auth).""" | |
| response = client.get('/metrics') | |
| # May require auth or return metrics | |
| assert response.status_code in [200, 401, 403] | |
| def test_csp_report_endpoint_malformed_json(self, client): | |
| """Test CSP report endpoint with malformed JSON.""" | |
| response = client.post('/csp_report/', data='{bad json}', content_type='application/json') | |
| # Should reject malformed JSON | |
| assert response.status_code == 400 | |
| def test_csp_report_endpoint_missing_data(self, client): | |
| """Test CSP report endpoint with missing data.""" | |
| response = client.post('/csp_report/', data='', content_type='application/json') | |
| # Should reject missing data | |
| assert response.status_code == 400 | |
| def test_metrics_endpoint(self, client): | |
| """Test metrics endpoint (may require auth).""" | |
| response = client.get('/metrics') | |
| # May require auth or return metrics | |
| assert response.status_code in [200, 401, 403] |
Comment on lines
+12
to
+15
| def test_healthcheck_returns_200(self, client): | ||
| """Healthcheck should return 200 OK when services are available.""" | ||
| response = client.get('/healthcheck/') | ||
| assert response.status_code == 200 |
There was a problem hiding this comment.
suggestion (testing): Add tests for healthcheck endpoint when dependencies (DB, Redis) are unavailable.
Please add tests that simulate DB and Redis outages to confirm the healthcheck endpoint returns the expected error status and message.
Suggested implementation:
def test_healthcheck_content_type(self, client):
"""Healthcheck returns text/html content type."""
response = client.get('/healthcheck/')
assert 'text/html' in response['Content-Type']
def test_healthcheck_db_unavailable(self, client, mocker):
"""Healthcheck returns 503 if DB is unavailable."""
# Patch the DB check used in the healthcheck view to raise an exception
mocker.patch('app.views.healthcheck.check_database', side_effect=Exception("DB unavailable"))
response = client.get('/healthcheck/')
assert response.status_code == 503
assert b"DB unavailable" in response.content
def test_healthcheck_redis_unavailable(self, client, mocker):
"""Healthcheck returns 503 if Redis is unavailable."""
# Patch the Redis check used in the healthcheck view to raise an exception
mocker.patch('app.views.healthcheck.check_redis', side_effect=Exception("Redis unavailable"))
response = client.get('/healthcheck/')
assert response.status_code == 503
assert b"Redis unavailable" in response.content- Ensure that your healthcheck view in
app/views/healthcheck.pyusescheck_databaseandcheck_redisfunctions for dependency checks, and that it returns a 503 status code and error message when those checks fail. - If the healthcheck view uses different function names or locations for DB/Redis checks, update the patch targets accordingly.
- The
mockerfixture is provided by pytest-mock. If not already present, addpytest-mockto your test dependencies.
There was a problem hiding this comment.
Pull Request Overview
Introduces comprehensive GitHub Actions CI workflow for automated validation of both Python backend and Vue/Vite webapp when targeting the enext branch.
- Adds CI workflow with backend validation, webapp build verification, and integration testing
- Implements comprehensive test suite covering healthcheck, public pages, authenticated pages, and admin functionality
- Provides reusable pytest fixtures for users, organizers, events, and authenticated clients
Reviewed Changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
.github/workflows/webapp-backend-ci.yml |
CI workflow with uv package management, Django checks, Vue build, PostgreSQL/Redis integration tests, and server smoke testing |
app/tests/conftest.py |
Pytest configuration with fixtures for users, organizers, events, teams, and authenticated test clients |
app/tests/test_healthcheck.py |
Tests for /healthcheck/ endpoint validating response status, content, and type |
app/tests/test_presale.py |
Tests for public event pages including landing, locale, robots.txt, schedule, and speaker pages |
app/tests/test_control.py |
Tests for authenticated organizer control panel, event management, and admin interface pages |
app/tests/test_common.py |
Tests for utility endpoints including redirects, CSP reporting, metrics, and static content |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| POSTGRES_USER: eventyay | ||
| POSTGRES_PASSWORD: eventyay | ||
| POSTGRES_DB: eventyay-db | ||
| POSTGRES_HOST: postgres |
There was a problem hiding this comment.
The POSTGRES_HOST should be 'localhost' when connecting from the runner to the service container, not 'postgres'. Service containers are accessible on localhost with the mapped port.
Suggested change
| POSTGRES_HOST: postgres | |
| POSTGRES_HOST: localhost |
| POSTGRES_USER: eventyay | ||
| POSTGRES_PASSWORD: eventyay | ||
| POSTGRES_DB: eventyay-db | ||
| POSTGRES_HOST: postgres |
There was a problem hiding this comment.
The POSTGRES_HOST should be 'localhost' when connecting from the runner to the service container, not 'postgres'. Service containers are accessible on localhost with the mapped port.
Suggested change
| POSTGRES_HOST: postgres | |
| POSTGRES_HOST: localhost |
| POSTGRES_USER: eventyay | ||
| POSTGRES_PASSWORD: eventyay | ||
| POSTGRES_DB: eventyay-db | ||
| POSTGRES_HOST: postgres |
There was a problem hiding this comment.
The POSTGRES_HOST should be 'localhost' when connecting from the runner to the service container, not 'postgres'. Service containers are accessible on localhost with the mapped port.
Comment on lines
+163
to
+165
| if (echo > /dev/tcp/postgres/5432) >/dev/null 2>&1; then echo "Postgres is up"; break; fi; sleep 2; done | ||
| for i in {1..30}; do | ||
| if (echo > /dev/tcp/redis/6379) >/dev/null 2>&1; then echo "Redis is up"; break; fi; sleep 2; done |
There was a problem hiding this comment.
The TCP health checks should use 'localhost' instead of 'postgres' and 'redis' since service containers are accessible on localhost from the runner.
Suggested change
| if (echo > /dev/tcp/postgres/5432) >/dev/null 2>&1; then echo "Postgres is up"; break; fi; sleep 2; done | |
| for i in {1..30}; do | |
| if (echo > /dev/tcp/redis/6379) >/dev/null 2>&1; then echo "Redis is up"; break; fi; sleep 2; done | |
| if (echo > /dev/tcp/localhost/5432) >/dev/null 2>&1; then echo "Postgres is up"; break; fi; sleep 2; done | |
| for i in {1..30}; do | |
| if (echo > /dev/tcp/localhost/6379) >/dev/null 2>&1; then echo "Redis is up"; break; fi; sleep 2; done |
norbusan
reviewed
Oct 16, 2025
|
|
||
| - uses: actions/setup-python@v5 | ||
| with: | ||
| python-version: "3.11" |
There was a problem hiding this comment.
The Dockerfile for prod/dev now use Python 3.12.
Maybe to do this here, too.
mariobehling
requested changes
Oct 21, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This pull request introduces a comprehensive GitHub Actions CI workflow to validate both the Python backend and the Vue/Vite webapp for pull requests targeting the enext branch.
Changes
.github/workflows/webapp-backend-ci.yml):Backend Test Suite Enhancements (app/tests/):
Rationale
This CI workflow helps prevent broken merges and regressions by automatically validating both backend and frontend changes before integration.
It ensures the backend starts successfully, dependencies are properly synchronized, and the webapp builds correctly, providing a more reliable development and review process.
Testing
Resolves #1050