Learn how to a build a cloud-first strategyRegister Now

x
?
Solved

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

Posted on 2016-07-28
9
Medium Priority
?
309 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 60

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 111

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 60

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
Concerto Cloud for Software Providers & ISVs

Can Concerto Cloud Services help you focus on evolving your application offerings, while delivering the best cloud experience to your customers? From DevOps to revenue models and customer support, the answer is yes!

Learn how Concerto can help you.

 
LVL 111

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 60

Accepted Solution

by:
Julian Hansen earned 2000 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 60

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

What does it mean to be "Always On"?

Is your cloud always on? With an Always On cloud you won't have to worry about downtime for maintenance or software application code updates, ensuring that your bottom line isn't affected.

Question has a verified solution.

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

When table data gets too large to manage or queries take too long to execute the solution is often to buy bigger hardware or assign more CPUs and memory resources to the machine to solve the problem. However, the best, cheapest and most effective so…
The title says it all. Writing any type of PHP Application or API code that provides high throughput, while under a heavy load, seems to be an arcane art form (Black Magic). This article aims to provide some general guidelines for producing this typ…
The viewer will learn how to create and use a small PHP class to apply a watermark to an image. This video shows the viewer the setup for the PHP watermark as well as important coding language. Continue to Part 2 to learn the core code used in creat…
In this video, Percona Solutions Engineer Barrett Chambers discusses some of the basic syntax differences between MySQL and MongoDB. To learn more check out our webinar on MongoDB administration for MySQL DBA: https://www.percona.com/resources/we…
Suggested Courses
Course of the Month21 days, 6 hours left to enroll

810 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