OpenVPN Monitor Enhancements

[kingy444]

Thanks for all the work in initial config of site.

I have made a few enhancements to further improve it. I have listed them below and could have missed something as i was surprised at how quickly development moved. Everything i have added in follows your coding model, and I have set off by default and on by choice.

Obviously ask any questions, just enjoyed the base product and wanted to enhance it. Hoping your comfortable with enhancements. While i only just forked the project, i've been running the enhancements for the last week.

Summarised changes follow:

• Added markers for both VPN connections and the Monitor box itself. (This addresses https://github.com/furlongm/openvpn-monitor/issues/39)
• Added Route lines between VPN Server and VPN Client
• Added Layer control to turn items off based on type and per vpn
• Added Fullscreen map capability
• Added Legend to the map for new icons (dynamic based on enabled icons)
• Added ability to manage icons that appear too close together (spiderfy)
• Changed the version label to a panel footer with custom css to allow it to work better in a responsive environment. (Label was offscreen)
• Corrected usage of 'table-responsive'. Should be used on a div and not the table itself. Responsive now working on mobile.
• Added openvpn-monitor.conf.example (Addressing https://github.com/furlongm/openvpn-monitor/issues/33). If you agree with that request you will need to remove openvpn-monitor.conf, and add a step to 'cp /var/www/html/openvpn-monitor/openvpn-monitor.conf.example /var/www/html/openvpn-monitor/openvpn-monitor.conf' on initial download
• Updated the ReadME to include relevant info on the changes and recognition of plugins
• Added FavIcon
• Updated PopUps to work with spiderfy and contain more info on multiple markers

[kingy444]

I have reviewed all codacy bot detections and corrected where possible. For info. Most of the detections are related to the fact that the Control.FullScreen.js file is a plugin, and therefore it has a lot of undefined detections. These are all false as the Leaflet variables are obviously called elsewhere

If you are happy with the changes and additions i have made, im also keen to introduce a detection system for update availability on GitHub, and a click to update function. Basically, headless once the server is deployed and control is from the web interface

[furlongm]

Hi, thanks for PR! :)

For an quick initial review, it should be possible to use JS links from https://cdnjs.com/ instead of copying JS into the project. See how the other JS and CSS has been done so far. That should also reduce the codacy issues.

There were also some minor quantifiedcode issues raised: https://www.quantifiedcode.com/app/project/gh:furlongm:openvpn-monitor/diff/4127f2f103550bb75d481bee8074043fbe8103b4/bb486ed8509e90b7c36f1c2ad3fdaab5f9a993f9/issues

For using openvpn-monitor.conf.example , we would have to ensure that this does not break the pip install as it currently does.

Would it be possible to break each piece into a smaller single commit? That might make reviewing easier.

[kingy444]

Will work through it and update this once done. Looks like the Spidery is not on cdnjs atm so ill see if we can get it up there

[furlongm]

Could add #noqa for the URL line too long issues?

[kingy444]

Thanks for the tip - added # noqa: E501 to the cdnjs affected lines - tests now passing

[furlongm]

Cool 👍

Could you squash all commits into one, and reset to 7c2b6370be8c2f97e0a8d08607e993792876d51c ?

Then submit smaller pull requests based on added functionality, bugfix per patch etc... (e.g. with git add -p)

[kingy444]

Unfortunately new to this github thing - might need a bit more to go on than that.

How can You submit smaller pull requests when all changes are in the one file?

[furlongm]

git squash COMMIT pick the first commit and squash all others - this will create one big commit. git reset HEAD^1 git add -p git commit -m "fix thing" git add -p git commit -m "add functionality" .... etc .... git push --force # this will overwrite your current github commits, but will allow you to create new PRs

[kingy444]

I think i did that right - there are definitely a lot less entries in the PR.

Let me know if i need to do anything else.

[furlongm]

Hmm, the first commit seems to contain an already merged change?

And the second commit - I was hoping you could break this into smaller commits/PRs with e.g. git add -p; git commit -m "fix readme"; git add -p; git commit -m "fix images"; git add -p; git commit -m "add spiderfy", etc, etc.

[kingy444]

unfortunately most of the fixes are part of the openvpn-monitor.py file so breaking them out is quite difficult. i did most of the changes on my local before forking.

what would you like me to do from here. apologies for making this difficult, had never used github as a contributor before so ive obviously made this much harder for you.

where would you like me to go from here?

[furlongm]

git add -p should help break them out?

[kingy444]

Evening Mate - I have obviously struggled to get this broken into smaller commits, mainly due to all the changes being in the one file. If need be I will re-code them all from scratch but that is obviously going to take hours. Have to ask if there is anything i can do to merge this with the current branch rather than re-coding the whole merge request?

[furlongm]

It looks like there are still conflicts with master. Did you try with git add -p? It asks you which patch lines to add to a certain commit.

So if you checkout my master, then copy your files on top, you should be able to use git add -p to break the changes down into smaller commits.

Alternatively, you could use git reset HEAD^^^ to reset your three commits, and then run git add -p to create the individual commits. However there is a merge conflict, so this won't work without resolving that.

[ndobbs]

These changes look great - any updates as to when and if they'll be merged?

[kingy444]

Apologies mate I’ve honestly been flat out with family stuff for that long I hadn’t got around to this.

I’m perfectly capable of writing and developing some cool enhancements, unfortunately this is my first crack at GirHubbing and I seem to have made it difficult for furlongm to combine these, not for conflicts etc, more so I have it as one huge commit and he cannot sanity check smaller items as it currently stands.

I’ll try and have another crack shortly to get it in format that suits furlongm so he can try and incorporate in the project

[kingy444]

@furlongm i have resolved the commits, but given how closely integrated all the features are its quite difficult to just give you a feature at a time to approve the integration of.

If you need that for sanity sake i can go through and add each feature from scratch again, but definitely alot of work involved in that. I've been running this since the original merge request without fault if its any help.

Let me know so i can get to work if required for @ndobbs

[ndobbs]

@furlongm would be awesome to use these changes.

[furlongm]

@ndobbs: agreed.

It should be broken down into smaller commits with git add -p though.

I'll stick it on my to-do list and see if I can do it.

[ndobbs]

@furlongm and @kingy444 we really appreciate all of the work you've done. Glad its opensource, we use this interface for hundreds of clients.

[evadim]

Great contribution! Like very much! 👍

I try it and found what images path changed :

-flag = '{0!s}flags/{1!s}.png'
+flag = '{0!s}/images/flags/{1!s}.png'

and alias added to http server configuration. Why it done in new way? Actually, I rewrote all paths in src to make it work and only after found alias thing 8-)

[kingy444]

@evadim I found this solved an issue where images were not displayed correctly on a raspberry pi server, and given the addition of the marker icons it made sense to move to an images folder. Tested on a couple other OS and there seemed to be no issues

[tony-caffe]

Any update to this yet? I would love to start using this feature.

[furlongm]

I added some of these features to the master branch, but others still need to be added. This branch has conflicts with master now so it doesn't apply cleanly, and requires some work to sync up again.

[9-beep]

Hi fur,

I wanna show my icon in the title of openvpn monitor tab , how can i do that ?