?
Solved

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

Posted on 2016-07-28
9
Medium Priority
?
216 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
[X]
Welcome to Experts Exchange

Add your voice to the tech community where 5M+ people just like you are talking about what matters.

  • Help others & share knowledge
  • Earn cash & points
  • Learn & ask questions
  • 4
  • 2
  • 2
  • +1
9 Comments
 
LVL 58

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 58

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
WordPress Tutorial 1: Installation & Setup

WordPress is a very popular option for running your web site and can be used to get your content online quickly for the world to see. This guide will walk you through installing the WordPress server software and the initial setup process.

 
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 58

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 58

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

Independent Software Vendors: We Want Your Opinion

We value your feedback.

Take our survey and automatically be enter to win anyone of the following:
Yeti Cooler, Amazon eGift Card, and Movie eGift Card!

Question has a verified solution.

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

3 proven steps to speed up Magento powered sites. The article focus is on optimizing time to first byte (TTFB), full page caching and configuring server for optimal performance.
Many old projects have bad code, but the budget doesn't exist to rewrite the codebase. You can update this code to be safer by introducing contemporary input validation, sanitation, and safer database queries.
The viewer will learn how to count occurrences of each item in an array.
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…
Suggested Courses

770 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