Added device group type check to rule validation logic for add_device_group API endpoint. #13212
Conversation
|
|
includes/html/api_functions.inc.php
Outdated
| $query = QueryBuilderParser::fromJson($data['rules'])->toSql(); | ||
| if (empty($query)) { | ||
| return api_error(500, "We couldn't parse your rule"); | ||
| if ($data['type'] == 'dynamic') { |
There was a problem hiding this comment.
Hmm, actually static groups can have rules, they are just unused until you change the type to dynamic. We should check if (! empty($data['rules'])) { instead of the type
There was a problem hiding this comment.
@murrant - Absolutely, if the desired behaviour is to allow the definition of rules for static groups then wrapping the QueryBuilderParser::fromJson() call and accompanying api_error validation in the conditional you've suggested would be the way to go. I'll drop this pull request, test it and submit a new one with the change included.
There was a problem hiding this comment.
You don't need to close this, just update it :)
payload when the payload is empty.
|
you messed up your branch a bit, try doing a rebase against upstream master. |
Yeah, I see that - the noob strikes again. Sorry.. monkey behind the keyboard. Fixing it now. |
|
No worries @DanielMuller-TN if you are having issues feel free to make a new branch and PR with the changes and close this one. |
|
The noob will try again with a fresh pull request, apologises. |
A simple fix for the error I was encountering with submitting statically assigned device groups via the API. I've placed the dynamic rule logic validation within a conditional check so it only executes for device groups of type 'dynamic'. It's pretty low hanging fruit, sorry.
DO NOT DELETE THE UNDERLYING TEXT
Please note
Testers
If you would like to test this pull request then please run:
./scripts/github-apply <pr_id>, i.e./scripts/github-apply 5926After 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.