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

Added resources/sensors api call to list all sensors #9837

Merged
merged 5 commits into from Mar 19, 2019

Conversation

Projects
None yet
2 participants
@zombah
Copy link
Contributor

commented Feb 18, 2019

Simple api call to get full list of all sensors

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.

Testers

If you would like to test this pull request then please run: ./scripts/github-apply <pr_id>, i.e ./scripts/github-apply 5926
After you are done testing, you can remove the changes with ./scripts/github-remove. If there are schema changes, you can ask on discord how to revert.

Added resources/sensors api call to list all sensors
Signed-off-by: Misha Komarovskiy <zombah@gmail.com>
$sensors = array();
foreach (dbFetchRows("SELECT `sensors`.* FROM `sensors` LEFT JOIN `devices` ON `sensors`.`device_id` = `devices`.`device_id` WHERE `sensors`.`sensor_id` IS NOT NULL $sql", $sql_params) as $sensor) {
$host_id = get_vm_parent_id($device);

This comment has been minimized.

Copy link
@murrant

murrant Feb 19, 2019

Member

$device is not defined yet...

This comment has been minimized.

Copy link
@zombah

zombah Feb 19, 2019

Author Contributor

Yes, missed that. Now i see that i wrongly duplicated this construct, it looks devices table specific with it's ip and parent_id stuff. Maybe to just use check_is_read for such list functions?

Use global read permission check
Signed-off-by: Misha Komarovskiy <zombah@gmail.com>
}
$sensors[] = $sensor;
}
$sensors = dbFetchRows("SELECT `sensors`.* FROM `sensors` WHERE `sensors`.`sensor_id` IS NOT NULL);

This comment has been minimized.

Copy link
@murrant

murrant Mar 15, 2019

Member

sensor_id cannot be null, fyi

This comment has been minimized.

Copy link
@murrant

murrant Mar 15, 2019

Member

You could also use Eloquent here:

$sensors = \App\Models\Sensor::hasAccess(Auth::user())->get();
$total_sensors = $sensors->count();

This comment has been minimized.

Copy link
@zombah

zombah Mar 16, 2019

Author Contributor

@murrant MySQL schema says that NULL is default for sensor_id:

MariaDB [librenms]> describe sensors;
+---------------------------+------------------+------+-----+-------------------+-----------------------------+
| Field                     | Type             | Null | Key | Default           | Extra                       |
+---------------------------+------------------+------+-----+-------------------+-----------------------------+
| sensor_id                 | int(10) unsigned | NO   | PRI | NULL              | auto_increment              |

i think it's good to have such sanity check same way all other api calls have
or better not?

This comment has been minimized.

Copy link
@zombah

zombah Mar 16, 2019

Author Contributor

@murrant Count is not working for some reason with Eloquent

This comment has been minimized.

Copy link
@murrant

murrant Mar 18, 2019

Member

@zombah it is auto_increment and also not nullable. It cannot be null ;)

Count works for me:

php artisan tinker
Psy Shell v0.9.9 (PHP 7.3.3 — cli) by Justin Hileman
>>> $sensors = \App\Models\Sensor::hasAccess(User::first())->get();
>>> $sensors->count();
=> 13

zombah added some commits Mar 16, 2019

fix typo
Signed-off-by: Misha Komarovskiy <zombah@gmail.com>
fix another typo
Signed-off-by: Misha Komarovskiy <zombah@gmail.com>
Use laravel style for call
Signed-off-by: Misha Komarovskiy <zombah@gmail.com>
@murrant
Copy link
Member

left a comment

Looks nice and clean now :)

@murrant murrant added the API label Mar 19, 2019

@murrant murrant merged commit 55f4870 into librenms:master Mar 19, 2019

6 checks passed

Inspection Summary
Details
Node: analysis
Details
Travis CI - Pull Request Build Passed
Details
WIP Ready for review
Details
codeclimate All good!
Details
license/cla Contributor License Agreement is signed.
Details

@zombah zombah deleted the zombah:api-sensors branch Mar 20, 2019

funzoneq added a commit to funzoneq/librenms that referenced this pull request Apr 30, 2019

Added resources/sensors api call to list all sensors (librenms#9837)
* Added resources/sensors api call to list all sensors

Signed-off-by: Misha Komarovskiy <zombah@gmail.com>

* Use global read permission check

Signed-off-by: Misha Komarovskiy <zombah@gmail.com>

* fix typo

Signed-off-by: Misha Komarovskiy <zombah@gmail.com>

* fix another typo

Signed-off-by: Misha Komarovskiy <zombah@gmail.com>

* Use laravel style for call

Signed-off-by: Misha Komarovskiy <zombah@gmail.com>

@lock lock bot locked as resolved and limited conversation to collaborators May 19, 2019

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.