Solved

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

Posted on 2016-07-28
9
81 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 51

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 108

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 51

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
 
LVL 108

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
Threat Intelligence Starter Resources

Integrating threat intelligence can be challenging, and not all companies are ready. These resources can help you build awareness and prepare for defense.

 
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 51

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 51

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

How your wiki can always stay up-to-date

Quip doubles as a “living” wiki and a project management tool that evolves with your organization. As you finish projects in Quip, the work remains, easily accessible to all team members, new and old.
- Increase transparency
- Onboard new hires faster
- Access from mobile/offline

Join & Write a Comment

Author Note: Since this E-E article was originally written, years ago, formal testing has come into common use in the world of PHP.  PHPUnit (http://en.wikipedia.org/wiki/PHPUnit) and similar technologies have enjoyed wide adoption, making it possib…
These days socially coordinated efforts have turned into a critical requirement for enterprises.
The viewer will learn how to look for a specific file type in a local or remote server directory using PHP.
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…

706 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

Need Help in Real-Time?

Connect with top rated Experts

17 Experts available now in Live!

Get 1:1 Help Now