Skip to content

Commit

Permalink
Clean up database query
Browse files Browse the repository at this point in the history
- Use existing `selectRaw()` method to avoid using the global `app()`
  helper as a service locator, which hides dependencies.
- Do the same for the join.
- The `Expression` is necessary to prevent the aliased column from being
  prefixed with the database table prefix, if configured.
  • Loading branch information
franzliedke committed Jul 11, 2019
1 parent d270096 commit 7f10483
Showing 1 changed file with 8 additions and 7 deletions.
15 changes: 8 additions & 7 deletions src/Notification/NotificationRepository.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

use Carbon\Carbon;
use Flarum\User\User;
use Illuminate\Database\Query\Expression;

class NotificationRepository
{
Expand All @@ -26,10 +27,8 @@ class NotificationRepository
*/
public function findByUser(User $user, $limit = null, $offset = 0)
{
$primaries = Notification::select(
app('flarum.db')->raw('MAX(id) AS id'),
app('flarum.db')->raw('SUM(read_at IS NULL) AS unread_count')
)
$primaries = Notification::selectRaw('MAX(id) AS id')
->selectRaw('SUM(read_at IS NULL) AS unread_count')
->where('user_id', $user->id)
->whereIn('type', $user->getAlertableNotificationTypes())
->where('is_deleted', false)
Expand All @@ -39,9 +38,11 @@ public function findByUser(User $user, $limit = null, $offset = 0)
->skip($offset)
->take($limit);

return Notification::select('notifications.*', app('flarum.db')->raw('p.unread_count'))
->mergeBindings($primaries->getQuery())
->join(app('flarum.db')->raw('('.$primaries->toSql().') p'), 'notifications.id', '=', app('flarum.db')->raw('p.id'))
return Notification::select('notifications.*')
->selectRaw('p.unread_count')
// Expression is necessary until Laravel 5.8.
// See https://github.com/laravel/framework/pull/28400
->joinSub($primaries, 'p', 'notifications.id', '=', new Expression('p.id'))
->latest()
->get();
}
Expand Down

0 comments on commit 7f10483

Please sign in to comment.