Skip to content

Add server-side column sorting to the Tickets page#72

Open
NanakiEmi wants to merge 5 commits into
mintopia:developfrom
NanakiEmi:tickets-column-sorting
Open

Add server-side column sorting to the Tickets page#72
NanakiEmi wants to merge 5 commits into
mintopia:developfrom
NanakiEmi:tickets-column-sorting

Conversation

@NanakiEmi

Copy link
Copy Markdown

Problem

The user Tickets page (TicketController@index) paginates with no orderBy, so tickets are shown in database insertion order. With 15 rows per page a user's most recent tickets can land on page 2, and the Reference / Type / Seat / Event columns can't be reordered.

Change

Adds server-side sorting to /tickets, mirroring the existing Admin tickets convention (same order / order_direction query params and the shared partials/_sortheader.blade.php partial):

  • Sensible default order — newest event first (order=event, order_direction=desc), so recent tickets appear on page 1.
  • Sortable columns — Reference, Type, Seat, Event; click a header to sort ascending/descending. Sort keys are whitelisted, so only known columns ever reach the query builder.
  • Pagination preserves the sort via ->appends().
  • Joins are applied only for the active sort column; ->select('tickets.*') avoids column collisions and a tickets.id tiebreaker keeps ordering deterministic. Existing eager loads and the non-admin draft filter are unchanged.

Tests

  • Unit tests for TicketController@index: default order, each column ascending/descending, invalid order/direction fall back to defaults, pagination preserves the sort, and seat sort keeps unseated tickets.
  • Feature test that the Tickets page renders sortable header links and rows in the requested order.
  • Full suite passes; phpcs clean.

🤖 Generated with Claude Code

NanakiEmi added 3 commits July 6, 2026 10:31
Adds whitelisted order/order_direction query param handling to
TicketController@index, mirroring the existing admin tickets sort
convention (default: newest event first). Covers reference, type,
seat, and event columns with join-based sorting and a stable
tickets.id tiebreaker; invalid inputs fall back to the default.
Reuse the existing partials/_sortheader partial for the Reference,
Type, Seat, and Event column headers so they become clickable sort
links (Event defaults to desc on first click for newest-first).
Adds an HTTP feature test proving the headers render sort links and
that rows are ordered accordingly.
@codecov

codecov Bot commented Jul 6, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (dbcf2c2) to head (42b450a).

Additional details and impacted files
@@             Coverage Diff             @@
##             develop       #72   +/-   ##
===========================================
  Coverage     100.00%   100.00%           
- Complexity      1155      1166   +11     
===========================================
  Files            136       137    +1     
  Lines           4053      4046    -7     
===========================================
- Hits            4053      4046    -7     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Comment thread app/Http/Controllers/TicketController.php Outdated
Comment thread app/Http/Controllers/TicketController.php Outdated
Comment thread app/Http/Controllers/TicketController.php Outdated
Comment thread app/Http/Controllers/TicketController.php Outdated
Comment thread app/Http/Controllers/TicketController.php Outdated
Comment thread app/Http/Controllers/TicketController.php Outdated
Address PR review feedback:
- Move sort param validation into TicketIndexRequest FormRequest, so the
  controller just reads the validated value. Invalid/missing params are
  coerced to defaults in prepareForValidation so a bad query string still
  renders the page rather than 422ing.
- Replace explicit joins and select('tickets.*') with withAggregate()
  correlated subqueries; the ORM selects tickets.* and orders by the
  related column without manual joins.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
public function index(Request $request)
public function index(TicketIndexRequest $request)
{
$order = $request->validated('order');

@mintopia mintopia Jul 11, 2026

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Follow pattern from other pages, just use $request->input('order', 'default');

public function rules(): array
{
return [
'order' => ['required', Rule::in(self::SORTABLE)],

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

This will force the page to require it, even though by default it doesn't need them.

* Coerce missing or unrecognised sort parameters to sensible defaults, so a
* bad query string still renders the page rather than failing validation.
*/
protected function prepareForValidation(): void

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

prepareForValidation() is an antipattern, and not used by the rest of the site. Remove this, make the parameters optional. Then just access them with a default value in the controller.

Comment thread app/Http/Controllers/TicketController.php
- Make order/order_direction optional (drop required) and remove the
  prepareForValidation() hook.
- Read sort params in the controller with $request->input(key, default),
  matching the pattern used by the other index pages.

Invalid values are now rejected by the FormRequest rather than silently
coerced; the fallback unit tests are replaced by feature tests asserting
the validation errors.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.

2 participants