Use HTMX for check connectivity functionality#3568
Conversation
3a1a0cd to
584c694
Compare
Test results 27 files 27 suites 45m 41s ⏱️ Results for commit 372e2db. ♻️ This comment has been updated with latest results. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #3568 +/- ##
==========================================
+ Coverage 62.66% 62.81% +0.15%
==========================================
Files 611 611
Lines 45107 45125 +18
Branches 43 43
==========================================
+ Hits 28267 28346 +79
+ Misses 16830 16769 -61
Partials 10 10 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
71fd92a to
8db0ab6
Compare
johannaengland
left a comment
There was a problem hiding this comment.
Functionality is great! Just nitpicks to tests
f4b13e6 to
46ff1d9
Compare
46ff1d9 to
e527049
Compare
|
@lunkwill42 Let me know when you have taken a look at this, and I can merge it :) |
lunkwill42
left a comment
There was a problem hiding this comment.
This whole PR just gives me warm, fuzzy feelings 🫶
e527049 has a slight typo in the commit log 😉
Actually, while you're at it, can we also fix one slight thing that wasn't there before? Only profiles that fail are hyperlinked to their SeedDB page. Successful profiles are not hyperlinked. Why shouldn't they also be hyperlinked?
(A minor annoyance on my part has also always been that one can select any profile, but there is no way to go directly to see the profile details when you're unsure if you're selecting the right profile. You have to make multiple clicks to find the same profile by name in the Management Profile tab. Not sure what the UI could look like to make profile both selectable and also somehow hyperlinked so I can check their details. Feel free to make an issue out of this annoyance 😆)
python/nav/web/templates/seeddb/_seeddb_check_connectivity_response.html
Show resolved
Hide resolved
:D
NPM sNPM spnm snmp. (All jokes aside, I can fix it when rebasing the fixups before merging)
Added it :)
I can look into it after merging this PR :) I'm not sure what kind of UX would be a good fit, but maybe selecting a profile adds it to a list, instead of using the select 2 labels. |
lunkwill42
left a comment
There was a problem hiding this comment.
Great, just a nitpick on one error message!
python/nav/web/templates/seeddb/_seeddb_check_connectivity_response.html
Show resolved
Hide resolved
| if not is_valid_ip(ip_address, strict=True): | ||
| return _handle_invalid_ip(request, ip_address) |
There was a problem hiding this comment.
Doesn't seem like it made much of a difference though... since your error handling routine tries to look up the string in DNS, and the resolver will actually be as lenient as to convert integer input values into something resembling an IP address. 52 still becomes 0.0.0.52 😆
Some experimentation to find a good UX would be nice. Not high priority, but make an issue out of it :) |
372e2db to
78f8274
Compare
|

Scope and purpose
Resolves #3560.
This PR replaces a complicated flow to check if an IP Device can connect to a given management profile. The javascript flow has been completely replaced with htmx templates and views, with a two step process:
This pull request
Screenshots
Form is disabled while running connectivity test. A loading spinner is shown .
Before

After. Uses an .htmx-indicator with a spinner gif instead of

Spinner(...)class in JSConnectivity check responses
Contributor Checklist
Every pull request should have this checklist filled out, no matter how small it is.
More information about contributing to NAV can be found in the
Hacker's guide to NAV.
<major>.<minor>.x). For a new feature or other additions, it should be based onmaster.