Skip to content

WebUI - Search results for ports#13787

Merged
laf merged 3 commits intolibrenms:masterfrom
PipoCanaja:portdesc
Feb 17, 2022
Merged

WebUI - Search results for ports#13787
laf merged 3 commits intolibrenms:masterfrom
PipoCanaja:portdesc

Conversation

@PipoCanaja
Copy link
Copy Markdown
Contributor

@PipoCanaja PipoCanaja commented Feb 14, 2022

My NOC is complaining that searchs in LibreNMS gives less details on the port descriptions. Which is true for ports with parsed descriptions, following #13143

This PR restores the behaviour for Parsed Descriptions in Search results as well as PortURL, both using $port->getDescription();
It also removes the code in function.php for generate_port_link(), thx @murrant .

And a setting could be added to enable shorter display in case necessary (@fbourqui )

Before
Capture d’écran 2022-02-14 à 17 16 42

After
Capture d’écran 2022-02-14 à 17 16 12

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.

@PipoCanaja PipoCanaja self-assigned this Feb 14, 2022
@murrant
Copy link
Copy Markdown
Member

murrant commented Feb 14, 2022

I believe I was simply trying to match the existing behavior (which is/was not uniform). I would love to see getDescription() deleted.

Additionally, I think that change was made in a PR before the one you linked.

@murrant
Copy link
Copy Markdown
Member

murrant commented Feb 14, 2022

Please also check generate_port_link()

@PipoCanaja
Copy link
Copy Markdown
Contributor Author

PipoCanaja commented Feb 14, 2022

Right. I only started my researches and hurried up to try to catch next release. And missed thisgenerate_port_link() which is also part of the game here.

I come with a rather simple patch : restore the full ifAlias.

Also contacted the original PR that created this short view : #13143

@fbourqui
Copy link
Copy Markdown
Contributor

@PipoCanaja
I sent a pull request to your repo that fix the original minigraph display error, PipoCanaja#2
Added some screenshot so you can see the display error occurring in iftype page (Ports tab).
Could you merge it, it should then update this pull request

@PipoCanaja
Copy link
Copy Markdown
Contributor Author

Bonsoir @fbourqui
You can check if it works as expected for you now.
Thanx for the patch !

@fbourqui
Copy link
Copy Markdown
Contributor

Salut @PipoCanaja
It looks good to me like that, no complain.

Copy link
Copy Markdown
Member

@laf laf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@laf laf merged commit 6ac1382 into librenms:master Feb 17, 2022
@librenms-bot
Copy link
Copy Markdown

This pull request has been mentioned on LibreNMS Community. There might be relevant details there:

https://community.librenms.org/t/22-3-0-changelog/18403/1

@PipoCanaja PipoCanaja deleted the portdesc branch December 10, 2023 19:39
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.

5 participants