Solved

Trying to add conditional 'where' clause into query builder laravel

Posted on 2016-07-28
9
141 Views
Last Modified: 2016-07-28
Hi,
I have  a where clause I'm trying to insert that is conditional. I get a syntax error: "unexpected 'if' (T_IF)".
Not sure how to write this?
$vehicles = DB::table('used_vehicles')
        ->select('used_vehicles.id','used_vehicles.dealer_id','used_vehicles.category_id','used_vehicles.make_id','used_vehicles.model_id','used_vehicles.construction_year','used_vehicles.cab_style','used_vehicles.stock_number','used_vehicles.vehicle_price','used_vehicles.discount','used_vehicles.vehicle_price - used_vehicles.discount AS used_vehicles.vehicle_price','dealerships.name','makes.name as make_name','models.model_name as model_name','used_vehicles_categories.category_title as category_name')
        ->join('dealerships', 'used_vehicles.dealer_id', '=', 'dealerships.id')
        ->join('makes', 'used_vehicles.make_id', '=', 'makes.id')
        ->join('models', 'used_vehicles.model_id', '=', 'models.id')
        ->join('used_vehicles_categories', 'used_vehicles.category_id', '=', 'used_vehicles_categories.id')
        	if (isset(Auth::user()->dealership->name))
        	{
        		$vehicles->where('used_vehicles.dealer_id', Auth::user()->dealership_id);
        	}
       	->where('used_vehicles.archived', 0)
        ->orderBy( 'added', 'DESC') 
	->get();

Open in new window

0
Comment
Question by:tjyoung
  • 4
  • 2
  • 2
  • +1
9 Comments
 
LVL 55

Expert Comment

by:Julian Hansen
ID: 41732843
You can't put an if in a chain - you can do this
$vehicles = DB::table('used_vehicles')
        ->select('used_vehicles.id','used_vehicles.dealer_id','used_vehicles.category_id','used_vehicles.make_id','used_vehicles.model_id','used_vehicles.construction_year','used_vehicles.cab_style','used_vehicles.stock_number','used_vehicles.vehicle_price','used_vehicles.discount','used_vehicles.vehicle_price - used_vehicles.discount AS used_vehicles.vehicle_price','dealerships.name','makes.name as make_name','models.model_name as model_name','used_vehicles_categories.category_title as category_name')
        ->join('dealerships', 'used_vehicles.dealer_id', '=', 'dealerships.id')
        ->join('makes', 'used_vehicles.make_id', '=', 'makes.id')
        ->join('models', 'used_vehicles.model_id', '=', 'models.id')
        ->join('used_vehicles_categories', 'used_vehicles.category_id', '=', 'used_vehicles_categories.id');
// Terminate the chain 
// Now do the if    
if (isset(Auth::user()->dealership->name)) {
  $vehicles->where('used_vehicles.dealer_id', Auth::user()->dealership_id);
}
// Finally do the get
$vehicles->get();

Open in new window

1
 
LVL 109

Expert Comment

by:Ray Paseur
ID: 41732848
I would write it in two completely separate calls to the DB::table() facade.  Putting the if() statement into the middle of the chained methods seems like a logically good idea, but it bumps into PHP syntax.  Maybe something like this (I don't have a Laravel instance to test, but it seems OK).
/**
 * IF THE DEALERSHIP IS LOGGED IN
 */
if (isset(Auth::user()->dealership->name))
{
    $vehicles = DB::table('used_vehicles')
        ->select('used_vehicles.id','used_vehicles.dealer_id','used_vehicles.category_id','used_vehicles.make_id','used_vehicles.model_id','used_vehicles.construction_year','used_vehicles.cab_style','used_vehicles.stock_number','used_vehicles.vehicle_price','used_vehicles.discount','used_vehicles.vehicle_price - used_vehicles.discount AS used_vehicles.vehicle_price','dealerships.name','makes.name as make_name','models.model_name as model_name','used_vehicles_categories.category_title as category_name')
        ->join('dealerships', 'used_vehicles.dealer_id', '=', 'dealerships.id')
        ->join('makes', 'used_vehicles.make_id', '=', 'makes.id')
        ->join('models', 'used_vehicles.model_id', '=', 'models.id')
        ->join('used_vehicles_categories', 'used_vehicles.category_id', '=', 'used_vehicles_categories.id')
        
        // ADDITIONAL WHERE CLAUSE TO RESTRICT RESULTS ACCORDING TO DEALERSHIP ID
        ->where('used_vehicles.dealer_id', Auth::user()->dealership_id)
        ->where('used_vehicles.archived', 0)
        ->orderBy( 'added', 'DESC') 
    ->get();
}

/**
 * IF THE DEALERSHIP IS _NOT_ LOGGED IN
 */
else
{
    $vehicles = DB::table('used_vehicles')
        ->select('used_vehicles.id','used_vehicles.dealer_id','used_vehicles.category_id','used_vehicles.make_id','used_vehicles.model_id','used_vehicles.construction_year','used_vehicles.cab_style','used_vehicles.stock_number','used_vehicles.vehicle_price','used_vehicles.discount','used_vehicles.vehicle_price - used_vehicles.discount AS used_vehicles.vehicle_price','dealerships.name','makes.name as make_name','models.model_name as model_name','used_vehicles_categories.category_title as category_name')
        ->join('dealerships', 'used_vehicles.dealer_id', '=', 'dealerships.id')
        ->join('makes', 'used_vehicles.make_id', '=', 'makes.id')
        ->join('models', 'used_vehicles.model_id', '=', 'models.id')
        ->join('used_vehicles_categories', 'used_vehicles.category_id', '=', 'used_vehicles_categories.id')
        
        // NO ADDITIONAL WHERE CLAUSE
        ->where('used_vehicles.archived', 0)
        ->orderBy( 'added', 'DESC') 
    ->get();
}

Open in new window

0
 
LVL 55

Expert Comment

by:Julian Hansen
ID: 41732858
@Ray - I see where you are going and under less complex conditions I would agree with you - however I see a potential for time bomb in your implementation.

If the structure of the query changes in the future - you now have two points of change to the query logic - forget to update one or update it incorrectly and you have inconsistent results.

In this case the two conditions are mostly the same - so I would keep like code common and simply add the where in based on the condition.
0
PRTG Network Monitor: Intuitive Network Monitoring

Network Monitoring is essential to ensure that computer systems and network devices are running. Use PRTG to monitor LANs, servers, websites, applications and devices, bandwidth, virtual environments, remote systems, IoT, and many more. PRTG is easy to set up & use.

 
LVL 109

Expert Comment

by:Ray Paseur
ID: 41732870
@Julian: I certainly take your point.  I was thinking about this as I was writing the if() statement.  If() adds an element of cyclomatic complexity that makes it necessary to keep the unit tests organized in parallel, too.  I was assuming, maybe incorrectly, that this would be the only place in the script that sets $vehicles.  If so, a single if() doesn't seem too problematic.  If this is not the only code point to set $vehicles, it certainly argues for refactoring this DB call out to a common code block.

My instinct would be to keep the logged-in experience completely separate from the not-logged-in experience.  But without seeing the rest of the application, I'm not sure whether the addition of an if() statement represents a step toward spaghetti code, or just a blip in an otherwise well-structured application.

It's not 100% clear to me whether the order of chained methods matters to the execution of the query.  Would be nice if we had that freedom of movement!
https://laravel.com/docs/5.1/queries
0
 
LVL 7

Expert Comment

by:Gauthier
ID: 41732902
Julian answer is good, he just forget 2 lines before the get()
if you get a lot of these conditional, it may be worth to write a scope function to the UsedVehicles Model.

function scopeAuthDealer($query) {
    if (isset(Auth::user()->dealership->name)) {
        $query->where('dealer_id', Auth::user()->dealership_id);
    }
    return $query;
}

Open in new window


which you can use in the query chain as:
...->authDealer()->...
0
 
LVL 55

Accepted Solution

by:
Julian Hansen earned 500 total points
ID: 41732927
@Gauthier - nice catch

My Laravel is a bit rusty but I seem to remember you can pass an Array to the where() method - (assuming you are AND'ing your where clauses) in which case you could do this
$vehicles = DB::table('used_vehicles')

// Construct the Where Clause array
// Using conditional to add additional
// AND clause

$whereClause = array(
  'used_vehicles.archived' => 0,
);
if (isset(Auth::user()->dealership->name)) {
   $whereClause['used_vehicles.dealer_id'] = Auth::user()->dealership_id;
}
// Now proceed as before
        ->select('used_vehicles.id','used_vehicles.dealer_id','used_vehicles.category_id','used_vehicles.make_id','used_vehicles.model_id','used_vehicles.construction_year','used_vehicles.cab_style','used_vehicles.stock_number','used_vehicles.vehicle_price','used_vehicles.discount','used_vehicles.vehicle_price - used_vehicles.discount AS used_vehicles.vehicle_price','dealerships.name','makes.name as make_name','models.model_name as model_name','used_vehicles_categories.category_title as category_name')
        ->join('dealerships', 'used_vehicles.dealer_id', '=', 'dealerships.id')
        ->join('makes', 'used_vehicles.make_id', '=', 'makes.id')
        ->join('models', 'used_vehicles.model_id', '=', 'models.id')
        ->join('used_vehicles_categories', 'used_vehicles.category_id', '=', 'used_vehicles_categories.id')
        ->where($whereClause)
        ->orderBy( 'added', 'DESC') 
  ->get();

Open in new window

0
 
LVL 1

Author Comment

by:tjyoung
ID: 41732967
Hi,
Trying with Julian's code above but getting:

FatalErrorException in InventoryController.php line 30: syntax error, unexpected '$whereClause' (T_VARIABLE)

My exact query:
	public function readAll(ReadAllInventoryRequest $request)
	{

		$vehicles = DB::table('used_vehicles')
	
		$whereClause = array(
			'used_vehicles.archived' => 0,
		);

		if (isset(Auth::user()->dealership->name)) {
			$whereClause['used_vehicles.dealer_id'] = Auth::user()->dealership_id;
		}
	
        ->select('used_vehicles.id','used_vehicles.dealer_id','used_vehicles.category_id','used_vehicles.make_id','used_vehicles.model_id','used_vehicles.construction_year','used_vehicles.cab_style','used_vehicles.stock_number','used_vehicles.vehicle_price','used_vehicles.discount','used_vehicles.vehicle_price - used_vehicles.discount AS used_vehicles.vehicle_price','dealerships.name','makes.name as make_name','models.model_name as model_name','used_vehicles_categories.category_title as category_name')
        ->join('dealerships', 'used_vehicles.dealer_id', '=', 'dealerships.id')
        ->join('makes', 'used_vehicles.make_id', '=', 'makes.id')
        ->join('models', 'used_vehicles.model_id', '=', 'models.id')
        ->join('used_vehicles_categories', 'used_vehicles.category_id', '=', 'used_vehicles_categories.id')
        ->where($whereClause)
        ->orderBy( 'added', 'DESC') 
		->get();
			
		return response()->json($vehicles, 200);
	}

Open in new window

0
 
LVL 1

Author Comment

by:tjyoung
ID: 41732979
Hi,
Just moved the condition before the query and all is fine:

	public function readAll(ReadAllInventoryRequest $request)
	{
	
		$whereClause = array(
			'used_vehicles.archived' => 0,
		);

		if (isset(Auth::user()->dealership->name)) {
			$whereClause['used_vehicles.dealer_id'] = Auth::user()->dealership_id;
		}
		

		$vehicles = DB::table('used_vehicles')
		->select('used_vehicles.id','used_vehicles.dealer_id','used_vehicles.category_id','used_vehicles.make_id','used_vehicles.model_id','used_vehicles.construction_year','used_vehicles.cab_style','used_vehicles.stock_number','used_vehicles.vehicle_price','used_vehicles.discount','used_vehicles.vehicle_price - used_vehicles.discount AS used_vehicles.vehicle_price','dealerships.name','makes.name as make_name','models.model_name as model_name','used_vehicles_categories.category_title as category_name')
        ->join('dealerships', 'used_vehicles.dealer_id', '=', 'dealerships.id')
        ->join('makes', 'used_vehicles.make_id', '=', 'makes.id')
        ->join('models', 'used_vehicles.model_id', '=', 'models.id')
        ->join('used_vehicles_categories', 'used_vehicles.category_id', '=', 'used_vehicles_categories.id')
        ->where($whereClause)
        ->orderBy( 'added', 'DESC') 
		->get();
			
		return response()->json($vehicles, 200);
	}

Open in new window

0
 
LVL 55

Expert Comment

by:Julian Hansen
ID: 41732993
Apologies - typo on my  part - cut and paste was supposed to go before the chain. EE not the best code editor ...
0

Featured Post

Comprehensive Backup Solutions for Microsoft

Acronis protects the complete Microsoft technology stack: Windows Server, Windows PC, laptop and Surface data; Microsoft business applications; Microsoft Hyper-V; Azure VMs; Microsoft Windows Server 2016; Microsoft Exchange 2016 and SQL Server 2016.

Question has a verified solution.

If you are experiencing a similar issue, please ask a related question

Suggested Solutions

Introduction Since I wrote the original article about Handling Date and Time in PHP and MySQL (http://www.experts-exchange.com/articles/201/Handling-Date-and-Time-in-PHP-and-MySQL.html) several years ago, it seemed like now was a good time to updat…
Creating and Managing Databases with phpMyAdmin in cPanel.
Learn how to match and substitute tagged data using PHP regular expressions. Demonstrated on Windows 7, but also applies to other operating systems. Demonstrated technique applies to PHP (all versions) and Firefox, but very similar techniques will w…
The viewer will learn how to look for a specific file type in a local or remote server directory using PHP.

840 members asked questions and received personalized solutions in the past 7 days.

Join the community of 500,000 technology professionals and ask your questions.

Join & Ask a Question