Skip to content

Add checkbox for showing Trip IDs in operator edit form#220

Open
shay-coding wants to merge 3 commits intomainfrom
hide-trip-ids
Open

Add checkbox for showing Trip IDs in operator edit form#220
shay-coding wants to merge 3 commits intomainfrom
hide-trip-ids

Conversation

@shay-coding
Copy link
Collaborator

@shay-coding shay-coding commented Jan 25, 2026

Summary by CodeRabbit

  • New Features

    • Added a "Show Trip IDs" toggle in operator settings; saved preference controls whether Trip IDs appear in trip listings.
    • Trip list layout now adjusts dynamically: rows and headers reorganize depending on the toggle to show either Trip ID or Time columns.
  • Chores

    • Minor formatting/whitespace cleanup.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 25, 2026

Walkthrough

Added a "Show Trip IDs" checkbox to operator edit UI, persisted via operator.show_trip_id in the operator edit view, and adjusted vehicle listing template to conditionally display trip ID vs time columns. Minor newline formatting fixes in templates.

Changes

Cohort / File(s) Change Summary
Templates — Operator edit & Vehicle detail
fleet/templates/edit_operator.html, fleet/templates/vehicle_detail.html
edit_operator.html: added show_trip_id checkbox input bound to operator.show_trip_id and EOF newline. vehicle_detail.html: conditional rendering of table header and row cells so Trip ID column is shown when operator.show_trip_id is true; otherwise renders Time column and adjusts cell alignment.
View logic — Operator update
fleet/views.py
In operator_edit, read request.POST.get('show_trip_id') == 'on' and assign to operator.show_trip_id before saving.

Sequence Diagram(s)

sequenceDiagram
  participant Browser as Browser
  participant Server as Server (Django view)
  participant DB as Database
  participant Template as Template renderer

  Browser->>Server: GET /operator/<id>/edit (renders form)
  Note right of Server: form includes "Show Trip IDs" checkbox
  Browser->>Server: POST /operator/<id>/edit (show_trip_id=on|off)
  Server->>DB: update operator.show_trip_id and save
  DB-->>Server: save confirmation
  Server->>Template: render vehicle_detail with operator.show_trip_id
  Template-->>Server: rendered HTML
  Server-->>Browser: HTML response (vehicle list shows Trip ID or Time)
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested reviewers

  • Kai-codin
  • Brooklynbull321

Poem

🐰 A checkbox twinkles on the page,
Tick or untick to set the stage,
Server nods, the operator knows,
Trips reveal IDs or keep their close.
🥕✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: adding a checkbox in the operator edit form to toggle trip ID visibility.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@fleet/templates/edit_operator.html`:
- Around line 95-99: The template checkbox for show_trip_id needs an id and its
label should use for="show_trip_id" (do the same for the show_livery_border
checkbox to ensure labels are associated), and in the view add the POST handler
line operator.show_trip_id = request.POST.get('show_trip_id') == 'on' next to
where operator.show_livery_border is set so the checkbox value is persisted
after form submit.

Comment on lines +95 to +99
<label for="show_trip_id">
<input type="checkbox" name="show_trip_id" {% if operator.show_trip_id %}checked{% endif %}>
Show Trip IDs
</label>

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Find all references to show_trip_id in the codebase
rg -n -C3 '\bshow_trip_id\b'

Repository: NextStopLabs/MyBusTimes

Length of output: 3693


🏁 Script executed:

#!/bin/bash
# Check the edit_operator.html template for other checkbox patterns
rg -n "type=\"checkbox\"" fleet/templates/edit_operator.html -B2 -A2

# Find view or form handling for operator edits
fd -e py . fleet | xargs rg -l "show_trip_id|edit.*operator" | head -5

Repository: NextStopLabs/MyBusTimes

Length of output: 593


🏁 Script executed:

#!/bin/bash
# Find the operator edit view in fleet/views.py
rg -n "def.*operator\|show_trip_id\|show_livery_border" fleet/views.py -A5 -B2 | head -60

Repository: NextStopLabs/MyBusTimes

Length of output: 49


🏁 Script executed:

#!/bin/bash
# Check the size and content of views.py
wc -l fleet/views.py
# Look for class-based views or functions handling operators
rg -n "class.*Operator\|def.*operator\|def post\|def patch\|def put" fleet/views.py | head -20

Repository: NextStopLabs/MyBusTimes

Length of output: 86


🏁 Script executed:

#!/bin/bash
# Search for operator-related API views/viewsets
rg -n "class.*Operator.*View" fleet/views.py

# Also check if there's a separate forms.py
fd -name "forms.py" fleet/

Repository: NextStopLabs/MyBusTimes

Length of output: 297


🏁 Script executed:

#!/bin/bash
# Search for operator-related API views/viewsets
rg "class.*Operator.*View" fleet/views.py

# Check if there's a separate forms.py
find fleet -name "forms.py"

# Look for how form/request data is handled for operators
rg -n "request.POST\|request.data" fleet/views.py | grep -i operator | head -10

Repository: NextStopLabs/MyBusTimes

Length of output: 81


🏁 Script executed:

#!/bin/bash
# Check fleet/forms.py for operator form definition
wc -l fleet/forms.py
rg -n "class.*Operator.*Form\|show_trip_id\|show_livery_border" fleet/forms.py -B3 -A3

Repository: NextStopLabs/MyBusTimes

Length of output: 85


🏁 Script executed:

#!/bin/bash
# Read the entire forms.py file
cat -n fleet/forms.py

Repository: NextStopLabs/MyBusTimes

Length of output: 15873


🏁 Script executed:

#!/bin/bash
# Search for operator update/edit handlers in views.py
rg -n "MBTOperator\|def.*operator" fleet/views.py | head -30

# Also check if there's a URLs pattern that might give a clue
rg -n "edit.*operator\|operator.*edit" fleet/urls.py

Repository: NextStopLabs/MyBusTimes

Length of output: 49


🏁 Script executed:

#!/bin/bash
# Read the edit_operator.html template to understand the form submission
head -150 fleet/templates/edit_operator.html | tail -60

# Also look for the form action
rg -n "<form\|action=" fleet/templates/edit_operator.html | head -10

Repository: NextStopLabs/MyBusTimes

Length of output: 2559


🏁 Script executed:

#!/bin/bash
# Find the opening form tag
head -100 fleet/templates/edit_operator.html | grep -n "<form" -A5

# Also check if this is a method POST or PUT
rg -n "<form" fleet/templates/edit_operator.html -A5

Repository: NextStopLabs/MyBusTimes

Length of output: 800


🏁 Script executed:

#!/bin/bash
# Find the view that renders this template
rg -n "edit_operator.html" fleet/

# Find the corresponding view in urls.py
rg -n "edit.*operator\|operator.*edit" fleet/urls.py

Repository: NextStopLabs/MyBusTimes

Length of output: 148


🏁 Script executed:

#!/bin/bash
# Read the view function that renders edit_operator.html (around line 3964)
sed -n '3920,4050p' fleet/views.py

Repository: NextStopLabs/MyBusTimes

Length of output: 4641


🏁 Script executed:

#!/bin/bash
# Find the function definition - search backwards from line 3964
sed -n '3800,3970p' fleet/views.py | head -100

Repository: NextStopLabs/MyBusTimes

Length of output: 4423


🏁 Script executed:

#!/bin/bash
# Search for show_trip_id in the POST handler section (around line 3830-3920)
sed -n '3820,3920p' fleet/views.py | grep -n "show_trip_id"

# Also do a broader search in views.py
rg -n "show_trip_id" fleet/views.py

Repository: NextStopLabs/MyBusTimes

Length of output: 49


🏁 Script executed:

#!/bin/bash
# Final confirmation - search entire codebase for any assignment to show_trip_id field
rg "show_trip_id\s*=" --type py

# Also check if there's any auto-save mechanism or middleware that might handle it
rg "request.POST\|request.data" fleet/views.py | grep -i "show_trip_id\|setattr\|__dict__"

Repository: NextStopLabs/MyBusTimes

Length of output: 135


Add missing POST handler and fix label association for the checkbox.

The checkbox field won't persist because the view's POST handler is missing the line to save show_trip_id from the form submission. The show_livery_border checkbox (line 91) has the same missing id attribute issue and is correctly saved with operator.show_livery_border = request.POST.get('show_livery_border') == 'on', but no equivalent code exists for show_trip_id.

Required fixes

In the template (fleet/templates/edit_operator.html, lines 95-99), add the missing id attribute:

     <label for="show_trip_id">
-    <input type="checkbox" name="show_trip_id" {% if operator.show_trip_id %}checked{% endif %}>
+    <input type="checkbox" id="show_trip_id" name="show_trip_id" {% if operator.show_trip_id %}checked{% endif %}>
     Show Trip IDs
     </label>

In the POST handler (fleet/views.py, near where show_livery_border is set), add:

operator.show_trip_id = request.POST.get('show_trip_id') == 'on'
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<label for="show_trip_id">
<input type="checkbox" name="show_trip_id" {% if operator.show_trip_id %}checked{% endif %}>
Show Trip IDs
</label>
<label for="show_trip_id">
<input type="checkbox" id="show_trip_id" name="show_trip_id" {% if operator.show_trip_id %}checked{% endif %}>
Show Trip IDs
</label>
🤖 Prompt for AI Agents
In `@fleet/templates/edit_operator.html` around lines 95 - 99, The template
checkbox for show_trip_id needs an id and its label should use
for="show_trip_id" (do the same for the show_livery_border checkbox to ensure
labels are associated), and in the view add the POST handler line
operator.show_trip_id = request.POST.get('show_trip_id') == 'on' next to where
operator.show_livery_border is set so the checkbox value is persisted after form
submit.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@fleet/views.py`:
- Around line 3858-3859: The change-tracking loop that builds the `changes` list
doesn't include `show_trip_id`, so toggles to that field aren't recorded; update
the logic that compares old vs new operator attributes (the same place where
`show_livery_border` is handled) to include `show_trip_id` in the `changes` list
when its value changes so the audit log/Discord message reflects toggles to
`operator.show_trip_id`.

Comment on lines 3858 to +3859
operator.show_livery_border = request.POST.get('show_livery_border') == 'on'
operator.show_trip_id = request.POST.get('show_trip_id') == 'on'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Include show_trip_id in the operator change audit log.

You now update operator.show_trip_id, but the change-tracking loop later doesn’t include this field, so toggles won’t be logged to Discord. Consider adding it to the changes list to keep the audit trail complete.

🤖 Prompt for AI Agents
In `@fleet/views.py` around lines 3858 - 3859, The change-tracking loop that
builds the `changes` list doesn't include `show_trip_id`, so toggles to that
field aren't recorded; update the logic that compares old vs new operator
attributes (the same place where `show_livery_border` is handled) to include
`show_trip_id` in the `changes` list when its value changes so the audit
log/Discord message reflects toggles to `operator.show_trip_id`.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
fleet/templates/vehicle_detail.html (1)

293-296: Hardcoded colspan="5" does not account for dynamic column count.

The column count varies based on operator.show_trip_id, show_board, user permissions, and superuser status. With show_trip_id=false and minimal permissions, there could be fewer than 5 columns; with all options enabled, there could be more.

Consider calculating the colspan dynamically or setting it to a value that spans all possible columns (e.g., using a large number like colspan="10" or omitting it to let the cell span naturally).

🔧 Suggested fix
             {% empty %}
             <tr>
-                <td colspan="5">No trips found</td>
+                <td colspan="10">No trips found</td>
             </tr>
🧹 Nitpick comments (2)
fleet/templates/vehicle_detail.html (2)

242-254: Header indentation is inconsistent with surrounding template code.

The header section starting at line 242 uses no indentation, while the rest of the file (e.g., lines 276-292) uses significant indentation within the table. This inconsistency hurts readability.

Additionally, the header lacks columns for Map, Edit, and Admin actions that appear in the body rows (lines 285-291).

♻️ Suggested fix for consistent indentation
-<thead>
-    <tr>
-        <th scope="col">Route</th>
-        {% if operator.show_trip_id %}
-            <th scope="col" colspan="2">Trip</th>
-        {% else %}
-            <th scope="col">Time</th>
-        {% endif %}
-        <th scope="col">To</th>
-        {% if show_board %}
-            <th scope="col"></th>
-        {% endif %}
-    </tr>
-</thead>
+        <thead>
+            <tr>
+                <th scope="col">Route</th>
+                {% if operator.show_trip_id %}
+                    <th scope="col" colspan="2">Trip</th>
+                {% else %}
+                    <th scope="col">Time</th>
+                {% endif %}
+                <th scope="col">To</th>
+                {% if show_board %}
+                    <th scope="col"></th>
+                {% endif %}
+            </tr>
+        </thead>

255-275: Body indentation is inconsistent with surrounding code (lines 276-292).

The body section introduced by this change uses minimal indentation, while the subsequent rows (starting at line 276) use heavy indentation. Consider aligning the indentation for consistency.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant