Displaying filtered user data using Ajax and PHP

Question

I want to get my code cleaner and more efficient. This code gets variables form a PHP file and filters it to show the selected user name, all available usergroups on a list box and the groups he is currently into another.

Currently it's working great but it has a lot of delay between the modal shows up and the groups being shown.

JS:

  function saveUserGroup(id){  
         $('#useridgr').attr('placeholder','').val('');  
         $("#usernamegr").attr('placeholder', 'Nome').val('');  
         $('#list_box').html('');   $('#list_box_ini').html('');
            
       

   $.ajax({
            type: 'GET',
            url: 'users/' + id + '/users' + '/id',
            data: {'_token': $("input[name='_token']").val()},
            success: function(data){
              $('#useridgr').attr('placeholder',data.id).val(data.id);
              $("#usernamegr").attr('placeholder',data.name).val(data.name);
        
              loadgroupsdata(data.id, 0);
            }   }); }
        
        var loadgroupsdata = (uid, type) => {
        
          $.ajax({
            type: 'GET',
            url: 'usergrupo/' + uid + '/' + type,
            data: {'_token': $("input[name='_token']").val()},
            success: function(data){
        
              if (type == 0) listbox = '#list_box';
              else listbox = '#list_box_ini';
        
              loadgroups(uid, listbox, type, data);
        
            }
        
          }) }
        
        function loadgroups(uid, location, type, data){
        
          $.each(data,function(i,v){
            $(location).append("<option value='"+v.idgrupo+"'>"+v.nome+"</option>");   })
        
          if(type == 0) loadgroupsdata(uid, 1);
        
        }

function sendData(type){
  uid = document.getElementById("useridgr").value;
  
  if(type == 1) listname = 'list_box_ini'; else listname = 'list_box';
  var e = document.getElementById(listname);
  
  var value = e.options[e.selectedIndex].value;
  
  $.ajax({
    type: 'GET',
    url: 'savegroup/' + uid + '/' + type + '/' + value,
    data: {'_token': $("input[name='_token']").val()},
    success: function(data){
      saveUserGroup(uid);
      
    }
    
  })
}

PHP:

public function getData($uid, $tablename, $index)
  {

    $data = \DB::table($tablename)->where($index, $uid)->first();
    return response()->json($data, 200);

  }

  public function getGroups($id, $type)
  {

    if ($type == 0){
      $data = \DB::table('grupoutilizador')
      -> join ('users', function ($join) use ($id){
        $join->on('users.id', '=', 'grupoutilizador.iduser')
        ->where('users.id', "=", $id );
      }) -> join ('grupo', 'grupo.idgrupo', "=", 'grupoutilizador.idgrupo') ->
      select('users.id', 'users.name', 'grupo.idgrupo', 'grupo.nome')
      -> get();

    } else {
      include 'config.php';

      $usergroups = mysqli_query($conn2, 'SELECT grupo.idgrupo FROM grupo, grupoutilizador WHERE grupoutilizador.idgrupo = grupo.idgrupo AND grupoutilizador.iduser =' .$id);

      $i = 0;

      if ($usergroups->num_rows != 0){
        foreach ($usergroups as $usergroup) {
          $ids[$i] = $usergroup;
          $i++;
        }
      } else {
        $ids[$i] = 0-1;

      }

      $data = \DB::table('grupo')
      ->whereNotIn('grupo.idgrupo', $ids)
      -> get();

    }

    return response()->json($data, 200);
  }

   public function saveGroups($uid, $type, $value){
        if ($type == 1){
          DB::table('grupoutilizador')->insert([
            'iduser' => $uid,
            'idgrupo' => $value,
          ]);

        } else {

          $result = DB::table('grupoutilizador') -> where([
          ['iduser', '=', $uid],
          ['idgrupo', '=', $value],
          ]) -> delete();

              echo $result;


        }
   }

Answer 1

The fact that you are making three separate trips to the server stands out as an obvious refinement opportunity.

We don't know exactly where your performance bottleneck is, but you can be sure that best practice is to reduce your total trips to ...well, anywhere.

Think about it like this:

Your mother tells you to go to the shop and get a dozen eggs.

You get on your bicycle and make 12 trips to the shop and deliver one egg to your mother per trip.

Your sister shakes her head disapprovingly.

Your mother tells the neighbors that she loves you because you are "special".

If you know that the full flow of this task is to write data to the db ...then get some data from one table ...then get some data from another table, then the sensible thing is to dictate this flow entirely on the server side then only return to the client-side when everything is finished.

Effectively, I recommend using a single ajax request (with POST instead of GET because you are writing data to your database).

The dedicated server-side receiving script should execute the database writing query, then immediately execute the database fetching queries, then generate a single json string which contains all of the necessary user and usergroup data to satisfy your client-side processes.

If the bottleneck is isolated to the database interactions, then we don't have enough information to assist with optimizing the schema.

---

General advice:

• Don't use mysqli_query() when you have a project that provides querying helper methods. You whole application should use consistent techniques.

• Only use concatenation when necessary. '/users' + '/id' should be '/users/id'

• When sending data to the server to fetch/read data from the db, use $_GET. When sending to write/insert/update/delete data, use $_POST. There are exceptions, but this is the rule.

• Use consistent spacing and tabbing in your code so that it is easier to maintain.

• Don't use old school comma-join syntax -- use JOINs.

• This is weird:

  $i = 0;
  if ($usergroups->num_rows != 0){
    foreach ($usergroups as $usergroup) {
      $ids[$i] = $usergroup;
      $i++;
    }
  } else {
    $ids[$i] = 0-1;
  
  }
  

  Why declare a counter? Why subtract 1 from 0? You should probably trash most this section of code, and maybe entertain something closer to...

  foreach ($usergroups as $usergroup) {
      $ids[] = $usergroup;
  }
  
  $data = \DB::table('grupo')
      ->whereNotIn('grupo.idgrupo', $ids ?? -1)
      ->get();
  

• Never omit curly braces in an attempt to reduce the size of your code. If you have a simple condition and like ternary expressions, use those instead.

• Try to avoid declaring single-use variables.

• If type dictates location, then don't send both parameters to the next function scope. Generate location conditionally when you get there.

Answer 2

Have you tested the server response time independently of your frontend code? The page being slow might not be because of your code being inefficient, if the server or database takes a long time then there's nothing you can do to fix that on the frontend side.

That being said, first thing you should do with this code is to look up some standards for PHP and JS and follow those.

Get a code editor, for example PHPStorm where you can autoformat the code (indent lines among other things) and remove extra empty lines, so that your code becomes readable.

Also, avoid things like this

if(type == 1) listname = 'list_box_ini'; else listname = 'list_box';

Always use curly braces and proper line breaks and indentation

if (type == 1) {
    listname = 'list_box_ini';
} else {

}

I suggest you do such basic fixes to your code and then post it again.