1

I am thinking about the right design of the method which gives user a list of clients who's last appointment was long time ago.

So I have 2 table (simplified):

  • Clients (id, first_name)
  • Appointments (id, client_id, datetime)

What I am trying to do: get the list of 3 clients who's last appointment was long time ago.

So what I do: I select users with the oldest appointments and return them (with complex SQL query). Then I create models from them.

Is it good design for this case?

use Illuminate\Database\Eloquent\Collection;

class ClientRepository {

    /**
     * Get clients with no appointments in the near history (sort by the date of last appointment ASC)
     *
     * @todo Make a better way to find falling clients (more Laravelish maybe?)
     * @param $count How many clients should method return
     * @return Illuminate\Database\Eloquent\Collection 
     */
    static public function getLost($count=3) {

        //this SQL looks too long but works as charm
        //solution based on http://stackoverflow.com/questions/1066453/mysql-group-by-and-order-by
        $sql = "
            SELECT * FROM (
                SELECT clients.*, clients.id AS cli_id , appointments.datetime AS last_appointment_datetime
                FROM clients
                INNER JOIN appointments ON clients.id=appointments.client_id
                ORDER BY appointments.datetime ASC
            ) AS tmp_table
            GROUP BY cli_id
            ORDER BY last_appointment_datetime ASC
            LIMIT ?
        ";

        $users = \DB::select($sql,[$count]);
        foreach($users as $key=>$user) {
            $user_array = (array)$user;
            $users[$key] = new Client();
            $users[$key]->forceFill($user_array);
        }
        $collection = new Collection($users);

        return $collection;
    }

}
jedrzej.kurylo
  • 39,591
  • 9
  • 98
  • 107
Dmitriy Lezhnev
  • 801
  • 2
  • 10
  • 18

1 Answers1

1

You can do it without subselect and using Eloquent's query builder:

//select clients joined with their appointments
$users = Client::join('appointments', 'appointments.client_id', '=', 'clients.id') 
  //group rows by client ID (if client has multiple appointments then client would appear multiple times in the result set
  ->groupBy('clients.id') 
  //sort them by date of last appointment in descending order
  ->orderByRaw('MAX(appointments.datetime)') 
  ->get();
jedrzej.kurylo
  • 39,591
  • 9
  • 98
  • 107
  • Yea, I tried this solution before, but there is a big disadvantage, because he groups User wrong way. I attach sample SQL source: [link](https://app.box.com/s/cii1dzn2irm1zjqmtpyxc83pc4h9jtla) And comparing this two queries shows a problem: `select clients.id,clients.first_name,appointments.datetime from clients inner join appointments on appointments.client_id = clients.id /*group by clients.id */ order by appointments.datetime ASC` and the same with UNCOMMENTEd line – Dmitriy Lezhnev Aug 16 '15 at 17:34
  • So your solutions **GROUPS** first and then perform **ORDER**. It gives wrong result, thats why subquery let me **ORDER** first and then **GROUP**. Can you comment if it can be achieved in Laravel-way? – Dmitriy Lezhnev Aug 16 '15 at 17:42
  • The code I provided I was sorting by MAX(appointment.datetime), I do not see that part I'm the SQL above – jedrzej.kurylo Aug 16 '15 at 19:57
  • Sorry, english is not my native. Maybe I did not understand you right. I dumped the SQL query your code produced and it seemed to get wrong results. I will check your code again. – Dmitriy Lezhnev Aug 17 '15 at 07:35
  • Please try again. For sure it was not the query my code produced, as there was no "max()" in the query. I think the code will do what you need (provided I understood correctly what you need). It takes users that had appointment in the past, groups them by ID so that only one row is returned for user, for each group/user it takes the max of the datetime, which is the date of last attempt, and then sorts them in ascending order, which will give you a list of users that had appointments starting with the one with smallest datetime, so the oldest "last appointment" date – jedrzej.kurylo Aug 17 '15 at 07:49
  • Yea, tried it again - so it gives same wrong result. The thing is it groups before ordering. Subquery solves it anyway. Thanks for helping! – Dmitriy Lezhnev Aug 18 '15 at 06:18
  • I'll say it again - this is what you need. No need to sort appointments within user as you're only interested with the date of last appointment, and that's what MAX(datetime) does. I run the code and got the correct query with MAX(): "select * from `clients` inner join `appointments` on `appointments`.`client_id` = `clients`.`id` group by `clients`.`id` order by MAX(appointments.datetime)". If you're not having the MAX() part you mist have mis-copied the code. – jedrzej.kurylo Aug 18 '15 at 06:29
  • You are right! Now I understood your code worked well. Sorry for confusing and thanks a lot! )) – Dmitriy Lezhnev Aug 18 '15 at 06:54