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

fix: Changed sql query for state sensors on device overview page to ignore null sensor_id #5180

Merged
merged 1 commit into from Dec 16, 2016

Conversation

Projects
None yet
6 participants
@laf
Member

laf commented Dec 16, 2016

DO NOT DELETE THIS TEXT

Please note

Please read this information carefully. You can run ./scripts/pre-commit.php to check your code before submitting.

Fixes: #5168

@laf laf added the Bug 🐞 label Dec 16, 2016

@LibreNMS-CI

This comment has been minimized.

LibreNMS-CI commented Dec 16, 2016

Auto-Deploy finished, Test PR at http://5180.ci.librenms.org or https://5180.ci.librenms.org

@scrutinizer-notifier

This comment has been minimized.

scrutinizer-notifier commented Dec 16, 2016

The inspection completed: No new issues

@murrant

This comment has been minimized.

Member

murrant commented Dec 16, 2016

I couldn't reproduce the issue, but the change looks good.

@murrant murrant merged commit 355c740 into librenms:master Dec 16, 2016

2 checks passed

Auto-Deploy Build finished.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@laf laf deleted the laf:issue-5168 branch Dec 16, 2016

@E-t-z

This comment has been minimized.

Contributor

E-t-z commented Dec 17, 2016

Tested, works.

@laf

This comment has been minimized.

Member

laf commented Dec 17, 2016

The bug is from devices which had existing state sensors before the refactor from rosiak. They don't register the state translations so when the query runs with the left join we end up with two sensor_id values, first is legit, second is NULL because it's from the translation table.

Yes this is a work around

@Rosiak

This comment has been minimized.

Contributor

Rosiak commented Dec 18, 2016

Hrm, this broke something.
Will try to have a look when I'm back home.
screen shot 2016-12-18 at 13 55 04

@laf

This comment has been minimized.

Member

laf commented Dec 18, 2016

Try #5191

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment