Skip to content

Adding device's field in get_alert_rule and list-alert-rules API function (new)#14481

Merged
murrant merged 5 commits intolibrenms:masterfrom
geg347:adding-device-fields-alert-rules-API
Nov 9, 2022
Merged

Adding device's field in get_alert_rule and list-alert-rules API function (new)#14481
murrant merged 5 commits intolibrenms:masterfrom
geg347:adding-device-fields-alert-rules-API

Conversation

@geg347
Copy link
Copy Markdown
Contributor

@geg347 geg347 commented Oct 20, 2022

Hello dear LibreNMS community,

Sorry for posting twice but here is the same PR as #14376 (I created it on master by mistake)
I am trying to solve the pipeline error by rebasing from upstream master and not creating a PR on my fork master.

Previous text :

Here I added the ability to retrieve the Devices field in get_alert_rules and list_alert_rules API request either if it is a device, group or location.
Before that nothing was returned.
If one of these 3 variables are set they will be shown as a array of integer.
If there is no such variable nothing will be returned.

I know that the SQL request is quite complex and can maybe add latency on simple query.
I also added some PHP code to reset the variable if empty and to convert the string returned in SQL to an array of integer.

I split the SQL in several lines. Tell me if you really want to use eloquent, but I will need help on this.

Have a nice day!

Please give a short description what your pull request is for

DO NOT DELETE THE UNDERLYING TEXT

Please note

Please read this information carefully. You can run ./lnms dev:check to check your code before submitting.

  • Have you followed our code guidelines?
  • If my Pull Request does some changes/fixes/enhancements in the WebUI, I have inserted a screenshot of it.
  • If my Pull Request makes discovery/polling/yaml changes, I have added/updated test data.

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.

@geg347
Copy link
Copy Markdown
Contributor Author

geg347 commented Oct 26, 2022

Thanks a lot it is working like a charm.
I tried to update alerts with the output from list_alert_rules through the edit_rule API request and everything is working. If a devices or groups or locations exist, they are kept. If everything is empty it stays with the All_devices.

I added the resource's file but for #14500 It is already merged after this PR (I hope it will be kept back)

@geg347
Copy link
Copy Markdown
Contributor Author

geg347 commented Oct 26, 2022

Ah I guess I need to add the changes from #14500 to pass the pipeline... Or rebase to master.

@geg347
Copy link
Copy Markdown
Contributor Author

geg347 commented Oct 26, 2022

Thank you again Murrant!
I managed to pass the pipeline.
If you see other possible improvements feel free to tell me.

@murrant murrant added the API label Nov 9, 2022
@murrant murrant changed the title Adding device's field in get_alert_rule and lis-alert-rules API function (new) Adding device's field in get_alert_rule and list-alert-rules API function (new) Nov 9, 2022
@murrant murrant merged commit 96c7e71 into librenms:master Nov 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants