Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Corrected active_count for Alert icon color #9933

Merged
merged 6 commits into from Mar 14, 2019
@@ -222,6 +222,9 @@ public function compose(View $view)
$alert_status = AlertRule::select('severity')
->isActive()
->hasAccess($user)
->leftJoin('devices', 'alerts.device_id', '=', 'devices.device_id')
->where('devices.disabled', '=', '0')
->where('devices.ignore', '=', '0')
This conversation was marked as resolved by PipoCanaja

This comment has been minimized.

Copy link
@PipoCanaja

PipoCanaja Mar 12, 2019

Author Contributor

Should we include this in "isActive" ? I mean, an alert can be active if and only if those 2 where conditions on the device are met.

This comment has been minimized.

Copy link
@murrant

murrant Mar 13, 2019

Member

Not in a simple way. Joining tables can have implications on the query, so I don't like hiding it in a scope.

This comment has been minimized.

Copy link
@murrant

murrant Mar 13, 2019

Member

But maybe :D

This comment has been minimized.

Copy link
@PipoCanaja

PipoCanaja Mar 13, 2019

Author Contributor

I'll follow your 1st advice and keep it as it is for now.

->groupBy('severity')
This conversation was marked as resolved by PipoCanaja

This comment has been minimized.

Copy link
@murrant

murrant Mar 13, 2019

Member

groupBy('severity') is already set on this query ;)

->pluck('severity');
@@ -39,7 +39,7 @@ class AlertRule extends BaseModel
*/
public function scopeEnabled($query)
{
return $query->where('disabled', 0);
return $query->where('alert_rules.disabled', 0);
This conversation was marked as resolved by PipoCanaja

This comment has been minimized.

Copy link
@PipoCanaja

PipoCanaja Mar 12, 2019

Author Contributor

disabled is a column name that exist in other tables.

}
/**
@@ -52,7 +52,7 @@ public function scopeIsActive($query)
{
return $query->enabled()

Check warning on line 53 in app/Models/AlertRule.php

Scrutinizer / Inspection

app/Models/AlertRule.php#L53

The expression ``return $query->enabled()->join('alerts', 'alerts.rule_id', 'alert_rules.id')->whereNotIn('alerts.state', array(0, 2))`` also could return the type ``Illuminate\Database\Query\Builder`` which is incompatible with the documented return type ``Illuminate\Database\Eloquent\Builder``.
->join('alerts', 'alerts.rule_id', 'alert_rules.id')
->where('alerts.state', 1);
->whereNotIn('alerts.state', [0, 2]);
}
/**
@@ -3,10 +3,10 @@
use LibreNMS\Authentication\LegacyAuth;
if (LegacyAuth::user()->hasGlobalRead()) {
$data['active_count'] = array('query' => 'SELECT COUNT(`alerts`.`id`) FROM `alerts` LEFT JOIN `devices` ON `alerts`.`device_id`=`devices`.`device_id` RIGHT JOIN `alert_rules` ON `alerts`.`rule_id`=`alert_rules`.`id` WHERE 1 AND `alerts`.`state` NOT IN (0,1)');
$data['active_count'] = array('query' => 'SELECT COUNT(`alerts`.`id`) FROM `alerts` LEFT JOIN `devices` ON `alerts`.`device_id`=`devices`.`device_id` RIGHT JOIN `alert_rules` ON `alerts`.`rule_id`=`alert_rules`.`id` WHERE 1 AND `alerts`.`state` NOT IN (0,2) AND `devices`.`disabled` = 0 AND `devices`.`ignore` = 0');
} else {
$data['active_count'] = array(
'query' => 'SELECT COUNT(`alerts`.`id`) FROM `alerts` LEFT JOIN `devices` ON `alerts`.`device_id`=`devices`.`device_id` LEFT JOIN `devices_perms` AS `DP` ON `devices`.`device_id` = `DP`.`device_id` RIGHT JOIN `alert_rules` ON `alerts`.`rule_id`=`alert_rules`.`id` WHERE 1 AND `alerts`.`state` NOT IN (0,1) AND `DP`.`user_id`=?',
'query' => 'SELECT COUNT(`alerts`.`id`) FROM `alerts` LEFT JOIN `devices` ON `alerts`.`device_id`=`devices`.`device_id` LEFT JOIN `devices_perms` AS `DP` ON `devices`.`device_id` = `DP`.`device_id` RIGHT JOIN `alert_rules` ON `alerts`.`rule_id`=`alert_rules`.`id` WHERE 1 AND `alerts`.`state` NOT IN (0,2) AND `devices`.`disabled` = 0 AND `devices`.`ignore` = 0 AND `DP`.`user_id`=?',
'params' => array(LegacyAuth::id()),
);
}
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.