0

I have a users page, where all the users in the system are displayed by default.

I am trying to apply filters so that users can refine their search for a user. For example, view all users who are male.

By default, to display all users on users.php, I am running this query by default:

$get_all_users = "SELECT * FROM users";

But when a user for example selected they want to view all female users on the system, then I want to run this query (example):

$get_all_users = "SELECT * FROM users WHERE gender = 'female'";

But for some reason, the query which displays all users by default, is always being executed. Here is my approach:

// get gender from radio buttons
$refined_gender = htmlentities (strip_tags(@$_POST['gender']));

$get_all_users = "SELECT * FROM users";

if (isset($_POST['submit'])){
if (isset($_POST['gender'])) {
        if ($refined_gender){
                $get_all_users = "SELECT * FROM users WHERE gender = '$refined_gender'";
        }
    }
}
Freddy
  • 683
  • 4
  • 35
  • 114
  • 1
    1. Have you verified (`var_dump($_POST)`) that those two items are actually set? 2. This probably isn't why it's not working at this point, but `htmlentities` and `strip_tags` are not suitable for escaping input to an SQL query. – Don't Panic Mar 31 '16 at 21:52
  • The reason you do `@$_POST['gender']` is, because of the annoying warning that warned you that this key inside the POST array does not exist. So why not check if beforehand and build your WHERE clause somewhere else where you check (in a switch case) what the user picked in the frontend. 1/2 – paskl Mar 31 '16 at 21:55
  • Meaning with that, in the future, with more filters, you have a big if else construct which is hard to throw an eye at. So have a function build the WHERE is a much better approch. Also it would kill duplicate code in the process. 2/2 – paskl Mar 31 '16 at 21:56
  • 1
    Why not load all content on the page and use a client side filter to display / hide content based on the users selection. For example, when you echo the results into the page, set the gender as a class of each entry and then on the users selection toggle the class to display / hide? - Two benefits to this - reduced server load and speed of displaying the filtered results to the user. And then you could add other filterws in (for example all those people named "Bob" would be a simple thing to filter client side without the need for another query. – gavgrif Mar 31 '16 at 21:59
  • Is the '@' necessary next to '@$_POST['gender']'? I've never seen that used, so that might be the problem. – Lachie Mar 31 '16 at 22:02
  • @Lachie http://stackoverflow.com/questions/1032161/what-is-the-use-of-the-symbol-in-php – Don't Panic Mar 31 '16 at 22:05
  • Freddy Dont use `@` as an error silencer. Instead use `isset()` to make sure a value has been passed. – RiggsFolly Mar 31 '16 at 22:06
  • @gavgrif Because this would blow up the DOM. Do you really need to fetch 900 users when you only want to filter the 2 girls out of it? Thats stress for the database and stress for the browser that does not need to be. – paskl Mar 31 '16 at 22:09
  • @Freddy If you discover that `$_POST['submit']` and `$_POST['gender']` are not set, verify that the form which submits these is actually using POST and not GET. This seems to be a common cause for this type of problem. – Don't Panic Mar 31 '16 at 22:09
  • Show us the HTML and ALL the database access code! Are you using `mysqli_` or `PDO`? – RiggsFolly Mar 31 '16 at 22:13
  • COBBLERS @paski..... two things - first a DB can handle way more complex issues than a simple query and secondly - the OP already states that the default view is to display all users and then the expected action is that the user filters the existing display to show only the filtered content (meaning it was already there prior to the filter) - this is from the OP "By default, to display all users on users.php, I am running this query by default:" - demonstrating that all users are shown BY DEFAULT! – gavgrif Mar 31 '16 at 22:17
  • @gavgrif Oh, if you're already showing N users on your frontend by default that makes it ok to do it still the ugly way. – paskl Mar 31 '16 at 22:19
  • @paski - given that the user states "I have a users page, where all the users in the system are displayed by default." and then the question at hand is how to filter that content after it has initially displayed - then yup - its totally ok. The answer provided is to filter the content that is already dsiaplyed ratherthan fetching a whole new bunch of data. Incurring another round to the server - when all the content is already present in the page and simply needs to be filtered/ If you wantt otalk about suggesting a better way of doing then then have at it, but that wasnt the question asked – gavgrif Mar 31 '16 at 22:21

1 Answers1

0

Like I would build the WHERE clause (something like that atleast) Not tested

$get_all_users = "SELECT * FROM users" . buildWhereClause($_POST);

function buildWhereClause($_POST) {
    $where = 'WHERE 1=1';
    foreach($_POST as $k => $v) {
        //check if $v is valid with a different function
        switch($k) {
            case 'gender':
                $where .= sprintf('AND `gender` = `%s`', $v);
                break;
            case 'age':
                $where .= sprintf('AND `age` = %d', $v);
                break;
        }
    }
    return $where;
}

This is a very lame function and should not be used as is. Just as an example where it could lead to. I'm sure theres clever ways of doing it and you dont always want an AND.

paskl
  • 589
  • 4
  • 25