Replace Quickselect in Maintenance form with HTMX#3425
Conversation
|
|
Test results 12 files 12 suites 12m 2s ⏱️ Results for commit 165a596. ♻️ This comment has been updated with latest results. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #3425 +/- ##
===========================================
+ Coverage 0 60.87% +60.87%
===========================================
Files 0 607 +607
Lines 0 44280 +44280
Branches 0 48 +48
===========================================
+ Hits 0 26954 +26954
- Misses 0 17314 +17314
- Partials 0 12 +12 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
lunkwill42
left a comment
There was a problem hiding this comment.
This is a beautiful first PR, if you ask me 🤩
The test coverage is great, but you'll find that most of my comments are about the tests themselves. You'll find that I tend to be quite picky about these things ;-)
I did not comment on all tests, as my comments and renaming suggestions can work generically - feel free to compare my comments to your other tests.
I quite like this pattern of naming tests (but you will of course find lots of legacy test code in NAV which does not measure up to this - clean as you go is a good keyword here): https://en.wikipedia.org/wiki/Given-When-Then
johannaengland
left a comment
There was a problem hiding this comment.
Works great! Just small nitpicks
Make component_search POST-only As mentioned in code review. feat: Start implementing component select with HTMX feat: Implement component select and listing with HTMX chore: Add changelog entry chore(maintenance): Remove quickselect code from maintenance tool fix(maintenance/utils): Use 3.9 comopatible union type
chore(maintenance/tests): Improve test names and reduce asserts per test feat(maintenance/views): Strip whitespace from search query
2c1e705 to
870d276
Compare
lunkwill42
left a comment
There was a problem hiding this comment.
This is nice. SonarCloud has two tiny nit-picks, about a missing docstring and alt-attribute for the spinner. Fix those, and we should be good to merge!
I removed the POC: from the title. This is no longer just a Proof-of-concept, this is more or less production ready :)
|
|
@lunkwill42 Ready for merge 🚀 |
Scope and purpose
This PR continues the initial POC done in #2883 , and adds a completed replacement for the "Quickselect" component in the Maintenance view using HTMX. The same change could be implemented in the
devicehistorytool, to get rid of thequickselectcomponent entirely. However, this would have to be done in a separate issue/PR.This pull request
/maintenance/search/(component_search()) and/maintenance/selectcomponents(component_select())._selected-components-list, which is used on initial load innew_task.htmland as response template incomponent_search(...)component".Screenshots
Previous implementation
With HTMX
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.