Conversation
Added a new view to export fleet data as a CSV file.
WalkthroughA CSV export function for fleet data has been added to fleet/views.py. The new endpoint Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @fleet/views.py:
- Around line 1123-1171: export_fleet_csv writes user-controlled text directly
to CSV and assumes v.features is always a list; add a small sanitizer function
(e.g., safe_csv_cell) and apply it to all string fields written (fleet_number,
reg, type_details, livery.name, branding, depot, name, prev_reg, and the
features cell) to prefix a quote when values start with "=", "+", "-", "@", or a
tab, and make the features cell robust by normalizing v.features before joining
(handle list, str, and other types safely and convert elements to strings).
Locate export_fleet_csv and wrap each text value passed into writer.writerow
with the sanitizer and replace the direct ', '.join(v.features) usage with a
defensive normalization that yields a single string for the features column.
🧹 Nitpick comments (1)
fleet/views.py (1)
1-50: Imports: csv is fine; consider makingQimport explicit
- Adding
import csvhere is appropriate for the new CSV view and consistent with the rest of the module.export_fleet_csv(and other views) rely onQ, but this module does not explicitly import it; instead it likely leaks in via a*import. That’s brittle and is what Ruff is warning about (F405). Consider adding an explicit import, e.g.from django.db.models import Q, and removing reliance on transitive star‑imports for core ORM utilities.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
fleet/views.py
🧰 Additional context used
🧬 Code graph analysis (1)
fleet/views.py (1)
fleet/models.py (3)
MBTOperator(129-169)fleet(226-289)vehicleType(72-87)
🪛 Ruff (0.14.10)
fleet/views.py
1124-1124: MBTOperator may be undefined, or defined from star imports
(F405)
1130-1130: fleet may be undefined, or defined from star imports
(F405)
1130-1130: Q may be undefined, or defined from star imports
(F405)
1130-1130: Q may be undefined, or defined from star imports
(F405)
| def export_fleet_csv(request, operator_slug): | ||
| operator = get_object_or_404(MBTOperator, operator_slug=operator_slug) | ||
|
|
||
| withdrawn = request.GET.get('withdrawn', '').lower() == 'true' | ||
| depot = request.GET.get('depot') | ||
|
|
||
| # Base queryset | ||
| qs = fleet.objects.filter(Q(operator=operator) | Q(loan_operator=operator)) | ||
| if not withdrawn: | ||
| qs = qs.filter(in_service=True) | ||
| if depot: | ||
| qs = qs.filter(depot=depot) | ||
|
|
||
| # Order by fleet number | ||
| vehicles = qs.order_by('fleet_number_sort').select_related( | ||
| 'operator', 'loan_operator', 'vehicleType', 'livery' | ||
| ) | ||
|
|
||
| # Create the HttpResponse object with CSV header | ||
| response = HttpResponse(content_type='text/csv') | ||
| response['Content-Disposition'] = f'attachment; filename="{operator_slug}_fleet.csv"' | ||
|
|
||
| writer = csv.writer(response) | ||
|
|
||
| # Write header row | ||
| headers = ['Fleet Number', 'Registration', 'Type', 'Type Details', 'Livery', | ||
| 'Branding', 'Depot', 'Name', 'Previous Reg', 'Features', 'In Service', | ||
| 'Open Top', 'Preserved'] | ||
| writer.writerow(headers) | ||
|
|
||
| # Write data rows | ||
| for v in vehicles: | ||
| writer.writerow([ | ||
| v.fleet_number or '', | ||
| v.reg or '', | ||
| v.vehicleType.type_name if v.vehicleType else '', | ||
| v.type_details or '', | ||
| v.livery.name if v.livery else '', | ||
| v.branding or '', | ||
| v.depot or '', | ||
| v.name or '', | ||
| v.prev_reg or '', | ||
| ', '.join(v.features) if v.features else '', | ||
| 'Yes' if v.in_service else 'No', | ||
| 'Yes' if v.open_top else 'No', | ||
| 'Yes' if v.preserved else 'No', | ||
| ]) | ||
|
|
||
| return response |
There was a problem hiding this comment.
Overall CSV export logic looks good; consider hardening against CSV injection and legacy features types
The view mirrors vehicles_api’s filtering and ordering, and the selected columns/boolean formatting are all consistent with the data model, so functionally this is solid.
A couple of points worth tightening:
-
User‑controlled text (e.g.
fleet_number,reg,name,branding,depot,prev_reg,features) is written directly to CSV. If an attacker can put a value starting with=,+,-,@or a leading tab into any of those fields, opening the CSV in Excel/Sheets can trigger formula execution (“CSV injection”). Consider a small sanitizer applied to each string field before writing, for example:def safe_csv_cell(value): if not isinstance(value, str): return value if value and value[0] in ("=", "+", "-", "@", "\t"): return "'" + value return value
and wrap the text fields (
fleet_number,reg,type_details,livery.name,branding,depot,name,prev_reg,features) withsafe_csv_cell(...)before passing them towriter.writerow. This keeps the export usable while mitigating formula injection in common spreadsheet tools. -
featuresis aJSONField; most of the code paths now store a list of strings, but older rows might still contain a bare string or some other shape.', '.join(v.features)will misbehave (or even raise) in those cases. A defensive pattern like:if isinstance(v.features, list): features_str = ", ".join(map(str, v.features)) elif isinstance(v.features, str): features_str = v.features else: features_str = ""
would make the export more robust against legacy data.
These are incremental hardening steps; the core view implementation is otherwise in good shape.
🧰 Tools
🪛 Ruff (0.14.10)
1124-1124: MBTOperator may be undefined, or defined from star imports
(F405)
1130-1130: fleet may be undefined, or defined from star imports
(F405)
1130-1130: Q may be undefined, or defined from star imports
(F405)
1130-1130: Q may be undefined, or defined from star imports
(F405)
🤖 Prompt for AI Agents
In @fleet/views.py around lines 1123 - 1171, export_fleet_csv writes
user-controlled text directly to CSV and assumes v.features is always a list;
add a small sanitizer function (e.g., safe_csv_cell) and apply it to all string
fields written (fleet_number, reg, type_details, livery.name, branding, depot,
name, prev_reg, and the features cell) to prefix a quote when values start with
"=", "+", "-", "@", or a tab, and make the features cell robust by normalizing
v.features before joining (handle list, str, and other types safely and convert
elements to strings). Locate export_fleet_csv and wrap each text value passed
into writer.writerow with the sanitizer and replace the direct ',
'.join(v.features) usage with a defensive normalization that yields a single
string for the features column.
1 participant
Added a new view to export fleet data as a CSV file.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.