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 display of minigraph when using sysName as hostname #8842

Merged
merged 7 commits into from Jul 2, 2018

Conversation

Projects
None yet
3 participants
@InsaneSplash
Copy link
Contributor

InsaneSplash commented Jun 25, 2018

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

Corrected widget to display the minigraph when a sysName is used instead of a hostname.

@@ -330,7 +330,7 @@
$common_output[] = shorten_text($result['storage_descr'], 50).'</td><td class="text-left">';
$common_output[] = overlib_link($link, print_percentage_bar(150, 20, $percent, null, 'ffffff', $background['left'], $percent.'%', 'ffffff', $background['right']), $overlib_content);
} else {
$common_output[] = generate_device_link($result, generate_minigraph_image($result, $config['time']['day'], $config['time']['now'], $graph_type, 'no', 150, 21), $graph_params, 0, 0, 0);
$common_output[] = generate_device_link($result['hostname'], generate_minigraph_image($result, $config['time']['day'], $config['time']['now'], $graph_type, 'no', 150, 21), $graph_params, 0, 0, 0);

This comment has been minimized.

@laf

laf Jun 26, 2018

Member

generate_device_link() expects a full $device result to be passed. I think you'll need to use format_hostname() within generate_device_link() which will return the sysName if config.php is set up correctly.

@laf laf added the WebUI label Jun 26, 2018

@murrant

This comment has been minimized.

Copy link
Member

murrant commented Jun 28, 2018

I've added tests and made some tweaks.

I think when both these settings are enabled, we want basically everything to be sysName. (except when "hostname" is not an IP or hostname)

@laf laf added this to the 1.42 milestone Jun 29, 2018

@murrant

murrant approved these changes Jul 2, 2018

Stale

@murrant murrant merged commit 528b91a into librenms:master Jul 2, 2018

3 checks passed

WIP ready for review
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
license/cla Contributor License Agreement is signed.
Details

@InsaneSplash InsaneSplash deleted the InsaneSplash:fix_widget_with_sysname branch Jul 24, 2018

gs-kamnas pushed a commit to gs-kamnas/librenms that referenced this pull request Aug 1, 2018

corrected display of minigraph when using sysName as hostname (libren…
…ms#8842)

* corrected display of minigraph when using sysName as hostname

* Check to see if its an IP or hostname. Make sure all 3 scenarios work.

* removed test rrd symlink

* removed test rrd symlink

* reverted old change

* Improve and add tests.

@lock lock bot locked as resolved and limited conversation to collaborators Sep 22, 2018

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.