Conversation
- update `competition_by_slug` decorator for `venue` and `ground` - extra qualification in the `venue` and `ground` url patterns to avoid conflicts - simplify test case
There was a problem hiding this comment.
Pull Request Overview
Adds detailed venue and ground listing pages to the tournament control site by extending routing, decorators, views, templates, and tests.
- Enhanced
competition_by_slugdecorator to extract and validatevenueandgroundslugs. - Introduced
venue_urlsand new view methods for venue and ground listings, then hooked them into season routing. - Added
venue.html,ground.html, and corresponding tests to verify page rendering and date filtering.
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_competition_site.py | New tests for venue and ground views, including date filtering |
| templates/tournamentcontrol/competition/venue.html | New template for listing matches by date at a venue |
| templates/tournamentcontrol/competition/ground.html | New template for listing matches by date at a ground |
| tournamentcontrol/competition/sites.py | Added venue_urls, integrated into season_urls, and implemented venue/ground views |
| tournamentcontrol/competition/decorators.py | Extended decorator to handle venue_slug and ground_slug parameters |
Comments suppressed due to low confidence (4)
tournamentcontrol/competition/sites.py:446
- You’ve registered two URL patterns both named
venue(with and withoutdatestr). This can confuse reverse lookups—consider using distinct names or a single pattern with an optional parameter.
path("<slug:venue>/", self.venue, name="venue"),
tournamentcontrol/competition/sites.py:450
- Similarly here, the
groundview is registered twice under the same name (ground) for date-specific and non-date-specific paths. Provide unique names or consolidate into a single route with an optionaldatestr.
path("<slug:venue>/ground/<slug:ground>/", self.ground, name="ground"),
tournamentcontrol/competition/tests/test_competition_site.py:168
- We have positive tests for valid venue and ground slugs, but no negative tests for invalid slugs. Adding tests asserting a 404 for non-existent
venueandgroundslugs would improve coverage.
def test_venue(self):
tournamentcontrol/competition/templates/tournamentcontrol/competition/ground.html:21
- [nitpick] The ground template omits the field/venue column present in the venue template (
<td class="field">). If the table markup or CSS expects that column, consider adding it for consistency or updating styles accordingly.
<tr><td class="time">{{ match.datetime|time }}</td><td>{{ match.get_home_team.title }}</td><td>{{ match.get_away_team.title }}</td></tr>
| <h1>{% trans competition.title %} - {% trans season.title %}</h1> | ||
| <h2>{% trans venue.title %}</h2> |
There was a problem hiding this comment.
The {% trans %} tag is intended for string literals; wrapping template variables in it may not behave as expected. Use {{ competition.title }} (and similarly for season.title) or a {% blocktrans %} block if you need variable interpolation with translations.
| <h1>{% trans competition.title %} - {% trans season.title %}</h1> | |
| <h2>{% trans venue.title %}</h2> | |
| <h1>{{ competition.title }} - {{ season.title }}</h1> | |
| <h2>{{ venue.title }}</h2> |
|
|
||
| {% block content %} | ||
| <h1>{% trans competition.title %} - {% trans season.title %}</h1> | ||
| <h2>{% trans venue.title %} - {% trans ground.title %}</h2> |
There was a problem hiding this comment.
Same as above: avoid using {% trans %} on variables. Replace with {{ venue.title }} and {{ ground.title }}, or use a {% blocktrans %} if translation of dynamic text is required.
| <h2>{% trans venue.title %} - {% trans ground.title %}</h2> | |
| <h2>{{ venue.title }} - {{ ground.title }}</h2> |
| date="2025-03-13", | ||
| time="10:00", | ||
| datetime="2025-03-13 10:00", |
There was a problem hiding this comment.
[nitpick] The test factory is given date, time, and datetime as strings; for clarity and to avoid implicit parsing, consider passing actual date and datetime objects.
| date="2025-03-13", | |
| time="10:00", | |
| datetime="2025-03-13 10:00", | |
| date=datetime.date(2025, 3, 13), | |
| time=datetime.time(10, 0), | |
| datetime=datetime.datetime(2025, 3, 13, 10, 0), |
This pull request introduces functionality to handle venues and grounds within the tournament control system. The changes include updates to decorators, site URL routing, templates, and tests to support venue and ground-related features.
Venue and Ground Handling in Decorators:
venue_slugandground_slugparameters tocompetition_by_slugintournamentcontrol/competition/decorators.py. These parameters enable filtering and retrieval of venues and grounds associated with a season.URL Routing for Venues and Grounds:
venue_urlsmethod intournamentcontrol/competition/sites.pyto define URL patterns for venue and ground views, including support for date-specific filtering.season_urlsmethod to include venue-related URLs, integrating venue and ground functionality into the broader season routing.Venue and Ground Templates:
venue.htmlandground.htmltemplates to display venue and ground details, including matches grouped by date. These templates provide user-friendly interfaces for exploring venue and ground data.Tests for Venue and Ground Features:
tournamentcontrol/competition/tests/test_competition_site.pyto verify venue and ground views, including scenarios with date-specific filtering. These tests ensure the correctness of the new functionality.https://chatgpt.com/codex/tasks/task_e_68624a03239c8324a60e40ddb5bfa143