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

WebUI: Change VRFs page to group together by RD and vrf_name #8799

Merged
merged 1 commit into from Jun 13, 2018

Conversation

Projects
None yet
2 participants
@vivia11
Contributor

vivia11 commented Jun 1, 2018

UI used group vrfs together by the route distinguisher only. This causes problems when route distinguishers are not configured for every vrf.

The UI now groups by vrf name AND the route distinguisher.

Also unset the port variable before using it in the code.

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

@@ -97,13 +97,14 @@
$port_fields = 'port_id, ifvrf, device_id, ifDescr, ifAlias, ifName';
foreach (dbFetchRows("SELECT $vrf_fields, $dev_fields FROM `vrfs` AS V, `devices` AS D WHERE D.device_id = V.device_id") as $vrf_device) {
if (empty($vrf_devices[$vrf_device['mplsVpnVrfRouteDistinguisher']])) {
$vrf_devices[$vrf_device['mplsVpnVrfRouteDistinguisher']][0] = $vrf_device;
if (empty($vrf_devices[$vrf_device['vrf_name']][$vrf_device['mplsVpnVrfRouteDistinguisher']])) {

This comment has been minimized.

@laf

laf Jun 4, 2018

Member

What happens if vrf_name is empty?

This comment has been minimized.

@vivia11

vivia11 Jun 4, 2018

Contributor

The page will still display a table with the associated devices/interfaces. It will display the route distinguisher and any description, but no name. This is assuming: there is no vrf_name, but there IS an RD.
The original code was assuming that the RD would never be empty, and in cases that were empty, it only displayed the first vrf entry, or none.

This code assumes that vrf_name and the RD will never both be empty.

@laf

laf approved these changes Jun 13, 2018

LGTM

@laf laf merged commit d88d230 into librenms:master Jun 13, 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

@vivia11 vivia11 deleted the vivia11:vrf-ui branch Jun 15, 2018

mattie47 added a commit to mattie47/librenms that referenced this pull request Jul 2, 2018

Change VRFs page to group together by RD and vrf_name (librenms#8799)
UI used group vrfs together by the route distinguisher only. This causes problems when route distinguishers are not configured for every vrf. 

The UI now groups by vrf name AND the route distinguisher.

Also unset the port variable before using it in the code. 

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.

- [x] Have you followed our [code guidelines?](http://docs.librenms.org/Developing/Code-Guidelines/)

#### Testers

If you would like to test this pull request then please run: `./scripts/github-apply <pr_id>`, i.e `./scripts/github-apply 5926`

@lock lock bot locked as resolved and limited conversation to collaborators Aug 14, 2018

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