Generate sitemap#859
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #859 +/- ##
==========================================
+ Coverage 75.99% 76.04% +0.04%
==========================================
Files 35 36 +1
Lines 8194 8211 +17
==========================================
+ Hits 6227 6244 +17
Misses 1967 1967 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR implements sitemap generation for the Penn Clubs website using the next-sitemap package. The sitemap includes both static Next.js routes and dynamically generated paths for publicly accessible club and event pages fetched from a new backend API endpoint.
Key changes:
- Added
next-sitemappackage and configured it to generate sitemap and robots.txt during build - Created a new backend API endpoint (
/api/sitemap-paths/) that returns paths for public clubs and events - Added comprehensive test coverage for the sitemap API endpoint
Reviewed changes
Copilot reviewed 5 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| frontend/package.json | Added next-sitemap dependency (^4.2.3) and postbuild script to generate sitemap after build |
| frontend/next-sitemap.config.js | Configuration for sitemap generation including site URL, excluded routes, robots.txt rules, and dynamic path fetching from backend API |
| frontend/bun.lock | Lock file updates for new dependencies and version bumps |
| backend/clubs/sitemap.py | New API view that returns JSON list of dynamic paths for publicly accessible clubs and events |
| backend/clubs/urls.py | Registered the new sitemap-paths API endpoint |
| backend/tests/clubs/test_views.py | Comprehensive test suite for the sitemap API covering various club and event visibility scenarios |
| .gitignore | Added entries to ignore auto-generated sitemap and robots.txt files |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| events = ( | ||
| Event.objects.filter( | ||
| Q(club__in=visible_clubs) | Q(club__isnull=True), | ||
| ) | ||
| .filter( | ||
| Q(club__isnull=True) | ||
| | Q(club__approved=True) | ||
| | Q(type=Event.FAIR) | ||
| | Q(club__ghost=True) | ||
| ) | ||
| .values_list("id", flat=True) |
There was a problem hiding this comment.
The event filtering logic doesn't fully match the EventViewSet.get_queryset() behavior for public viewers. Specifically, events with type=Event.FAIR from clubs that don't meet the visibility criteria (e.g., not visible_to_public) would be included by EventViewSet but excluded here because they fail the first filter at line 68 (club__in=visible_clubs). The second filter's Q(type=Event.FAIR) condition becomes unreachable for such events. Consider restructuring the query to match the EventViewSet logic more closely, potentially by removing the club__in restriction for FAIR events.
| '/apply/', | ||
| '/create', | ||
| '/renew', | ||
| '/invite/', |
There was a problem hiding this comment.
The /health endpoint is excluded from the sitemap (line 21) but not disallowed in robots.txt (robotsTxtOptions.policies). If the health endpoint should not be crawled by search engines, consider adding it to the disallow list. Similarly, /applications, /user/, and /wharton/ paths are excluded from the sitemap but not disallowed in robots.txt. Ensure this is intentional.
| '/invite/', | |
| '/invite/', | |
| '/applications', | |
| '/user/', | |
| '/wharton/', | |
| '/health', |
| const apiUrl = process.env.DOMAIN | ||
| ? process.env.DOMAIN.startsWith('http') | ||
| ? process.env.DOMAIN | ||
| : `https://${process.env.DOMAIN}` | ||
| : 'https://pennclubs.com' |
There was a problem hiding this comment.
The logic for constructing the API URL (lines 28-32) is duplicated from the siteUrl construction (lines 3-7). Consider extracting this into a shared constant or using the config.siteUrl parameter that's passed to the additionalPaths function to avoid code duplication and ensure consistency.
| const apiUrl = process.env.DOMAIN | |
| ? process.env.DOMAIN.startsWith('http') | |
| ? process.env.DOMAIN | |
| : `https://${process.env.DOMAIN}` | |
| : 'https://pennclubs.com' | |
| const apiUrl = config.siteUrl |
Generates a sitemap for static routes as well as publicly-accessible club and event pages.