1

I'm querying a database based on variables and I'm a bit confused about how to get my function is not working. If there is only one string in the SQL criteria array, I want it to say WHERE and if there is more then one, I want the first one to be WHERE and the rest to be AND. Currently if I have one string and the rets are blank then this what the query looks like: SELECT * FROM tb_visitors AND date BETWEEN '2017-08-01' AND '2017-08-31'. I want it to be WHERE first, is there a better way to do this?

        $sites =  $data['sites'];
        $attendence =  $data['filter'];
        $time = $data['time'];
        $department = $data['department'];
        $staff = $data['staff'];

        $sql = 'SELECT * FROM tb_visitors' ;
        $sql_criteria= "";

        if(is_numeric($department) == true){
           $sql_criteria[] = "dept_id = "."'".$department."'"; 
        } else if($staff !== 'Staff Member') {
             $sql_criteria[] = 'staff='.'"'.$staff.'"'; 
        } 

        if($department == 'Department' || $staff == 'Staff Member' ){
           $sql_criteria[] = ""; 
        }

        // Time
        if($time == 'Date Range' || $time == 'Custom' || $time == 'Today'){
           $sql_criteria[] = ""; 
        } else if($time == 'Today'){
           $today_date = date("Y-m-d"); 
           $sql_criteria[] = "date ="."'".$today_date."'";  
        } else if($time == 'Tomorrow'){
           $d= strtotime("tomorrow");
           $date =  date("Y-m-d", $d);
           $sql_criteria[] = "date ="."'".$date."'"; 
        }  else if($time == 'Yesterday'){
           $date = date('Y-m-d',strtotime("-1 days"));
           $sql_criteria[] = "date ="."'".$date."'"; 
        } else if($time == 'Last Month'){
           $start_date =  date("Y-n-j", strtotime("first day of previous month"));
            $end_date =  date("Y-n-j", strtotime("last day of previous month"));
           $sql_criteria[] = "date BETWEEN '".$start_date."' AND '".$end_date."'"; 
        } else if($time == 'Next Month'){
           $start_date =  date("Y-n-j", strtotime("first day of next month"));
           $end_date =  date("Y-n-j", strtotime("last day of next month"));
           $sql_criteria[] = "date BETWEEN '".$start_date."' AND '".$end_date."'"; 
        } else if($time == 'This Week'){
           $start_date = date("Y-m-d", strtotime('monday this week'));
           $end_date =  date("Y-m-d", strtotime('sunday this week'));
           $sql_criteria[] = "date BETWEEN '".$start_date."' AND '".$end_date."'";  
        } else if($time == 'This Month'){
           $start_date = date('Y-m-01');
            $end_date = date('Y-m-t');
            $sql_criteria[] = "date BETWEEN '".$start_date."' AND '".$end_date."'";
        } else if($time == 'Last Week'){
           $start_date = date("Y-m-d", strtotime("last week monday"));
           $end_date =  date("Y-m-d", strtotime("last week sunday"));
           $sql_criteria[] = "date BETWEEN '".$start_date."' AND '".$end_date."'"; 
        } else if($time == 'Next Week'){
           $start_date = date("Y-m-d", strtotime("next monday"));
           $end_date =    date("Y-m-d", strtotime('+1 week sunday'));
           $sql_criteria[] = "date BETWEEN '".$start_date."' AND '".$end_date."'";  

        } 

        // Attendence 
        if($attendence == 'Status' || $attendence == 'All Visits'){
            $sql_criteria[] = ""; 
        } else if($attendence == 'No Show'){
            $sql_criteria[] = "no_show = 'Y'";  
        } else if($attendence == 'Completed Visits'){
            $sql_criteria[] = "seen IS NOT NULL";  
        } else if($attendence == 'Upcoming Visits'){
           $today_date = date("Y-m-d"); 
           $sql_criteria[] = "date > '".$today_date."'"; 
        } 

        //Sites

        if($sites == 'All Sites' || $sites == 'Sites' ){
            $sql_criteria[] = ""; 
        } else if($sites == 'Twickenham'){
            $sql_criteria[] = "dept_id NOT IN ('28',  '29',  '30',  '32',  '41',  '44')";
        } else if($sites == 'Hammersmith'){
            $sql_criteria[] = "dept_id IN ('36')"; 
        } else if($sites == 'Heatherwood'){
           $sql_criteria[] = "dept_id IN ('37')";  
        } else if($sites == 'Hillingdon'){
          $sql_criteria[] = "dept_id IN ('38')";   
        } else if($sites == 'Nuffeild'){
            $sql_criteria[] = "dept_id IN ('39')"; 
        } else if($sites == 'St Georges'){
            $sql_criteria[] = "dept_id IN ('41')"; 
        } else if($sites == 'Queen Victoria'){
            $sql_criteria[] = "dept_id IN ('40')"; 
        } else if($sites == 'Stoke Mandeville'){
           $sql_criteria[] = "dept_id IN ('42')";  
        }


    $i=0;
    foreach ($sql_criteria as $a => $b){

       if($b == ""){
            $join = "";   
        } else if($i == 0){
            $join = "WHERE";
        } else {
            $join = "AND";  
        }


    $sql_final = $join." ".$b;
    $i++;
    $sql = $sql." ".$sql_final;
    }
   $result = $db->db_num("$sql");
   echo $sql;
user3053151
  • 137
  • 1
  • 14
  • 4
    this is horrendously vulnerable to injection attacks. You need to sanitise your inputs really carefully, and use parameters wherever possible. For anything that can't be parameterised, use whitelisting. – ADyson Aug 29 '17 at 15:12
  • 2
    @user3053151, I highly recommend you look at this documentation for php prepared statements http://php.net/manual/en/class.pdostatement.php – HumbleWebDev Aug 29 '17 at 15:20
  • Hey, Thanks for the advice, don't worry, this project is not going to go online, its for a private visitors book I'm trying to make. Thanks for the link, I'll have a read. – user3053151 Aug 29 '17 at 15:25
  • it'll have to go online _somewhere_ if people are going to use it (unless you mean you will be the only user, on localhost). Even if it's on a local network / intranet, you can't make any assumptions about who / what is going to try and send requests to it, and what their intentions are. Plus it's good practise for the future. – ADyson Aug 29 '17 at 16:07

1 Answers1

2

Since you've already got your where clauses in an array, you can utilize count and implode.

$sql = 'SELECT * FROM tb_visitors';
$sql_criteria = array();

/*
* Build the $sql_criteria array
* [...]
*/

if (count($sql_criteria)) // Should the where clause be built?
{
    $sql .= " WHERE " . implode(" AND ", $sql_criteria);
}

Note: As a commenter has already stated, you're vulnerable to SQL injections. This is important to consider, because injections can occur "accidentally", and are not always due to a targeted attack. For example, if your $staff had a value of "John O'Sullivan", the single quote in the name would break your query.

RToyo
  • 2,877
  • 1
  • 15
  • 22
  • With this I now get the follow if one of the variables are an empty string: "SELECT * FROM tb_visitors WHERE AND AND AND dept_id IN ('36') – user3053151 Aug 29 '17 at 15:31
  • @user3053151 The simple answer is to not have an empty string, haha. But if you want to keep an empty string in your array, you can sanitize the `$sql_criteria` through `array_filter()` to remove the blank values. If you intend to keep empty strings in your `$sql_criteria`, I can update the answer to demonstrate that. Edit: [this question](https://stackoverflow.com/questions/3654295/remove-empty-array-elements) is about removing blanks from an array. – RToyo Aug 29 '17 at 15:34
  • I got rid of the empty strings, I think you have cracked the case :) Many thanks! – user3053151 Aug 29 '17 at 15:39