cachet
cachet copied to clipboard
cachethq
Component groups sorted by status and order
Related to https://github.com/CachetHQ/Cachet/issues/2100
Now groups are ordered by severity status and specified order.
We should instead define the original relationship to be sorted perhapse rather than doing a client side sort.
@GrahamCampbell there is no relationship that we originally fetch the groups from. We're ordering the groups, not the components in this PR.
Yeah, I'd say so. Consistency. Although that said, the API allows for sorting.
@jbrooksuk @GrahamCampbell
What about this query scope for ComponentGroup model that extract the group severity?
public function scopeWithSeverity(Builder $query)
{
$subquery = 'SELECT group_id, MAX(status) as severity FROM components GROUP BY group_id';
return $query->join(DB::raw("($subquery) as sq"), 'group_id', '=', 'id');
}
After that in Api\ComponentGroupController we can do this:
public function getGroups()
{
$groups = ComponentGroup::withSeverity();
if (!$this->guard->check()) {
$groups->visible();
}
$groups->search(Binput::except(['sort', 'order', 'per_page']));
if ($sortBy = Binput::get('sort')) {
$direction = Binput::get('order', 'asc');
$groups->sort($sortBy, $direction);
} else {
$groups->orderBy('severity', 'desc')->orderBy('order');
}
$groups = $groups->paginate(Binput::get('per_page', 20));
return $this->paginator($groups, Request::instance());
}
And this in ComponentsComposer:
public function compose(View $view)
{
$componentGroups = $this->getVisibleGroupedComponents();
$ungroupedComponents = Component::ungrouped()->get();
$view->withComponentGroups($componentGroups)
->withUngroupedComponents($ungroupedComponents);
}
/**
* Get visible grouped components.
*
* @return \Illuminate\Support\Collection
*/
protected function getVisibleGroupedComponents()
{
$componentGroupsBuilder = ComponentGroup::withSeverity()
->orderBy('severity', 'desc')
->orderBy('order');
if (!$this->guard->check()) {
$componentGroupsBuilder->visible();
}
$usedComponentGroups = Component::grouped()->pluck('group_id');
return $componentGroupsBuilder->used($usedComponentGroups)
->get();
}
Not sure about anyone else but seeing DB::raw makes me want to rip my eyeballs out.
@ConnorVG Is only an example to show you what I think to do.
We can try to get the same thing using the query builder, although I think it would become only more complicated to read.
Looks okay to me. @GrahamCampbell?
Like @ConnorVG said, that DB::raw is making me sad.
@jbrooksuk @ConnorVG Agree, I'm trying to figure out how to execute this subquery without DB::raw ... any suggestions?
Use Eloquent?
SELECT group_id, MAX(status) as severity FROM components GROUP BY group_id ->
$query->selectRaw('MAX(status) as severity')
->addSelect('group_id')
->groupBy('group_id');
idk about merging it into the scope though, I'm at work atm so I can't test anything.
@ConnorVG what about this? I think there aren't SQL injection points.
/**
* Add status severity.
*
* @param \Illuminate\Database\Eloquent\Builder $query
* @return \Illuminate\Database\Eloquent\Builder
*/
public function scopeWithSeverity(Builder $query)
{
$subquery = Component::selectRaw('MAX(status) as severity')
->addSelect('group_id')
->groupBy('group_id');
return $query->join(DB::raw("({$subquery->toSql()}) as sq"), 'group_id', '=', 'id');
}
@ConnorVG I don't know an other way to do this.
Does not seem that the query can cause security vulnerability. I don't see anything wrong to use DB::raw in this case.
Thank you for your input on Cachet 2.x. We are shifting our attention and resources to Cachet 3.x and will no longer be supporting the 2.x version. If your feedback or issue is relevant to the 3.x series, we encourage you to engage with the new branch.
For more information on the Cachet rebuild and our plans for 3.x, you can read the announcement here.
We appreciate your understanding and look forward to your contributions to the new version.