Laravel Event Filtering Controller

Question

So I am currently writing a function in laravel that handles URL parameter filters.

How the code works:
So we have the following two URL parameters:

timeframe and category

an URL would look like this:

/events?timeframe=3&category=6

but it can also look like this

/events?timeframe=0&category=0

the range for the timeframe is from 0 to 6 and for the category, 0 to 8 These parameters are created using this blade template:

Filter Blade:

@php

if (Request::is('courses')) {
    $timeframe = '0';
    $category = '0';

} else if (Request::is('events')) {
    $timeframe = '0';
    $category = '1';

}


if (request()->has('category')) {
    $requestCategory = request()->get('category');
    if (in_array((int)$requestCategory, range(0, 8))) {
        $category = $requestCategory;
    }
}

if (request()->has('timeframe')) {
    $timeframe = request()->get('timeframe');
}

@endphp

<form class="container @if (Request::is('events')) -mt-24 @endif !pb-0">
    <div class="pt-6 md:pt-0">
        <div class="bg-white shadow p-6 rounded-lg">
            <div class="grid grid-cols-1 md:grid-cols-2 gap-6">
                <div>
                    <label class="mb-3 inline-block">Timeframe:</label>
                    <select aria-label="Timeframe" name="timeframe" class="w-full bg-dark-gray p-3 rounded-lg">
                        <option value="0" @if ($timeframe === '0') selected @endif>Upcoming</option>
                        <option value="1" @if ($timeframe === '1') selected @endif>Today</option>
                        <option value="2" @if ($timeframe === '2') selected @endif>Tomorrow</option>
                        <option value="3" @if ($timeframe === '3') selected @endif>This Week</option>
                        <option value="4" @if ($timeframe === '4') selected @endif>This Month</option>
                        <option value="5" @if ($timeframe === '5') selected @endif>Next Week</option>
                        <option value="6" @if ($timeframe === '6') selected @endif>Next Month</option>
                    </select>
                </div>
                <div>
                    <label class="mb-3 inline-block">Category:</label>
                    <select aria-label="Category" name="category" class="w-full bg-dark-gray p-3 rounded-lg">
                        <option value="0" @if ($category === '0') selected @endif>All Courses and Events</option>
                        <option value="1" @if ($category === '1') selected @endif>Events</option>
                        <option value="2" @if ($category === '2') selected @endif>Example 1</option>
                        <option value="3" @if ($category === '3') selected @endif>Example 2</option>
                        <option value="4" @if ($category === '4') selected @endif>Example 2</option>
                        <option value="5" @if ($category === '5') selected @endif>Example 3</option>
                        <option value="6" @if ($category === '6') selected @endif>Example 4</option>
                        <option value="7" @if ($category === '7') selected @endif>Example 5</option>
                        <option value="8" @if ($category === '8') selected @endif>Example 6</option>
                    </select>
                </div>
            </div>
            <div class="mt-3 flex items-end">
                <x-button type="submit" class="w-full">Search</x-button>
            </div>
        </div>
    </div>
</form>

Controller: I first set the $category with the $request->query('category'). This is only done if the URL has parameters, and is not 0 Because an URL can also just be like /events and the 0 for the category should show all events from 1 to 8.

After this I am setting the variables for the specific routes. /courses and /events.

Now I am filtering the timeframe with the carbon dates. The values are the following:

0 = Upcoming
1 = Today
2 = Tomorrow
3 = This Week
4 = This Month
5 = Next Week
6 = Next Month
default = Upcoming (all events after now())

I am not sure if this is the correct way to do this, because I am just adding 999 years to get all events from the DB for the upcoming.

Now I am returning the view. This is done dynamically, based on the all variable from the route. I am also checking if the $category is set. If it is true I am checking the category in the query. If not it will check all events on the database.

   public function coursesList(Request $request)
    {

    // check if category is zero
    if ($categoryParameter === '0') {
        $category = false;
    }

    if ($categoryParameter !== '0' && $categoryParameter >= '9') {
        $category = false;
    }

    // check against known list of categories
    if ($categoryParameter !== '0' && $categoryParameter <= '8') {
        $category = $request->query('category');
    }
    

    // setting for /kursangebote page
    if ($all === 1) {
        $view = 'pages.all-events';
        if (!$request->filled('category')) {
            $category = false;
        }
    }

    // setting for /events page
    if ($all === 0) {
        $view = 'pages.course.event-list';
        if (!$request->filled('category')) {
            $category = '1';
        }
    }

        if ($request->has('timeframe')) {
            $now = CarbonImmutable::now();
            [$startDate, $endDate] = match ($request->get('timeframe')) {
                '1' => [$now , Carbon::today()->endOfDay()],
                '2' => [Carbon::tomorrow()->startOfDay(), Carbon::tomorrow()->endOfDay()],
                '3' => [$now, Carbon::now()->endOfWeek()->endOfDay()],
                '4' => [$now, Carbon::now()->endOfMonth()->endOfDay()],
                '5' => [$now->next(Carbon::MONDAY)->startOfDay(), Carbon::now()->next(Carbon::MONDAY)->endOfWeek()->endOfDay()],
                '6' => [$now->startOfMonth()->addMonth()->startOfMonth(), Carbon::now()->startOfMonth()->addMonth()->endOfMonth()->endOfDay()],
                default => [$now, Carbon::now()->startOfMonth()->addYears('999')->endOfMonth()->endOfDay()],
            };
        } else {
            // default if no timeframe is set as URL parameter
            $startDate = Carbon::now();
            $endDate = Carbon::now()->startOfMonth()->addYears('999')->endOfMonth()->endOfDay();
        }

        return view($view, [
            'events' => Events::withCount('tickets')
                ->withMin('tickets', 'price')
                ->orderBy('start')
                ->when($category, function ($query, $category) {
                    $query->where('category', $category);
                })
                ->whereBetween('start', [$startDate, $endDate])
                // add pagination
                ->paginate(16)

        ]);
    }

Router:

// events list view
Route::get('/events', [
    'all' => 0,
    'uses' => '\App\Http\Controllers\EventsController@coursesList'
])->name('event-list');

Route::get('/courses', [
    'all' => 1,
    'uses' => '\App\Http\Controllers\EventsController@coursesList'
])->name('all-events');

My question:
Are there better ways to handle the first four if statements in the controller? And how would be the correct way to filter the db query for the upcoming setting? Maybe there are also other ways to improve this code, I would be very happy for some suggestions! :)

Answer 1

I think overall your code is pretty good. It was a little hard to tell what your view would call if the form was submitted (maybe you aren't using that part??)

Anyway, I took a stab.

I think, based on my limited knowledge of your app, you can boil your controller down to something like this

public function events(Request $request)
{
    $category = $request->get('category') ?? 1;
    $timeframe = $request->get('timeframe') ?? false;
    return $this->coursesList($category, $timeframe, 'pages.all-events');
}

public function courses(Request $request)
{
    $category = $request->get('category') ?? false;
    $timeframe = $request->get('timeframe') ?? false;
    return $this->coursesList($category, $timeframe, 'pages.course.event-list');
}

public function coursesList($category, $timeframe, $view)
{
    // if the category is set as URL parameter, check if it is range 1-8
    if($category !== false){
        // cast category to int, prevents bogus data from users
        $category = (int)$category;
        if($category <= 0 || $category >= 9){
            $category = false;
        }
    }

    if ($timeframe) {
        // cast timeframe to int, prevents bogus data from users
        $timeframe = (int)$timeframe;
        $now = CarbonImmutable::now();
        [$startDate, $endDate] = match ($timeframe) {
            1 => [$now, Carbon::today()->endOfDay()],
            2 => [Carbon::tomorrow()->startOfDay(), Carbon::tomorrow()->endOfDay()],
            3 => [$now, Carbon::now()->endOfWeek()->endOfDay()],
            4 => [$now, Carbon::now()->endOfMonth()->endOfDay()],
            5 => [$now->next(Carbon::MONDAY)->startOfDay(), Carbon::now()->next(Carbon::MONDAY)->endOfWeek()->endOfDay()],
            6 => [$now->startOfMonth()->addMonth()->startOfMonth(), Carbon::now()->startOfMonth()->addMonth()->endOfMonth()->endOfDay()],
            default => [$now, Carbon::now()->startOfMonth()->addYears('999')->endOfMonth()->endOfDay()],
        };
    } else {
        // default if no timeframe is set as URL parameter
        $startDate = Carbon::now();
        $endDate = Carbon::now()->startOfMonth()->addYears('999')->endOfMonth()->endOfDay();
    }

    return view($view, [
        'events' => Events::withCount('tickets')
            ->withMin('tickets', 'price')
            ->orderBy('start')
            ->when($category, function ($query, $category) {
                $query->where('category', $category);
            })
            ->whereBetween('start', [$startDate, $endDate])
            // add pagination
            ->paginate(16)

    ]);
}

Which would allow you to remove the "all" from your routes since they are going to separate controller methods.

Route::get('/courses', [\App\Http\Controllers\EventsController::class, 'courses']);
Route::get('/events', [\App\Http\Controllers\EventsController::class, 'events']);

Your query looks good to me.

Again I'm not super familiar with the architecture of your app.

Answer 2

I agree with the advice in the answer by Rager. Specifying certain controller methods per route, as was done in your first question is a common way to have more succinct methods.

Indentation levels are inconsistent

Perhaps there may have been issues with pasting code into the editor here but it appears that some of the indentation levels are inconsistent- For example in the controller method coursesList these lines appear in the middle of the method:

// setting for /events page
    if ($all === 0) {
        $view = 'pages.course.event-list';
        if (!$request->filled('category')) {
            $category = '1';
        }
    }

        if ($request->has('timeframe')) {

For readability it is best to keep indentation levels consistent.

Blade template loop can be used to simplify repeated lines

As I explained at the end of this review blade templates support loops and those can simplify redundant lines of options. The template in the code above has repetitive lines for the <option> elements in the select list for the category select list:

<option value="2" @if ($category === '2') selected @endif>Example 1</option>
<option value="3" @if ($category === '3') selected @endif>Example 2</option>
<option value="4" @if ($category === '4') selected @endif>Example 2</option>
<option value="5" @if ($category === '5') selected @endif>Example 3</option>
<option value="6" @if ($category === '6') selected @endif>Example 4</option>
<option value="7" @if ($category === '7') selected @endif>Example 5</option>
<option value="8" @if ($category === '8') selected @endif>Example 6</option>

This is very repetitive and does not comply with the Don't repeat yourself (DRY) principle. It also has two options with “Example 2”. Presuming there should only be one option with “Example 2”, it could be simplified using a loop with the @selected directive mentioned in the Additional Attributes section

<option value="0" @if ($category === '0') selected @endif>All Courses and Events</option>
<option value="1" @if ($category === '1') selected @endif>Events</option>
@for ($i = 2; $i < 9; $i++)
    <option value="{{ $i }}" @selected($category == $i)>Example {{ $i - 1 }}</option>
@endfor

The range() function could also be used with a @foreach loop:

<option value="0" @if ($category === '0') selected @endif>All Courses and Events</option>
<option value="1" @if ($category === '1') selected @endif>Events</option>
@foreach (range(2, 8) as $i)
    <option value="{{ $i }}" @selected($category == $i)>Example {{ $i - 1 }}</option>
@endfor