Refactor maintenance task views to improve display of deleted components #3230
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The description field will be mainly to store component descriptions for future use in the case where the original object has been deleted from the NAV database.
This is useful to get a reference to the model class of the referenced component, even if the foreign key itself no longer resolves to an existing record
They way the maintenance UI builds data structures to describe maintenance components (and their "trails") is quite obtuse, and seems to be a strange leftover from the olden days before Django was introduced to NAV. This can be done a lot cleaner, while improving on how missing components are described. These classes and functions represent an alternative way to build component trails to place in the template render context.
These type hints help a lot when working in an IDE
This replaces the usage of the old component trail generating functions with the new ones, and updates the templates accordingly. It also factors out `frag-component-trail.html` to a separate template fragment for clarity.
We would never refer to an IP device as an `ip device` in the UI, `IP` should always be upper case.
There is a definitive canonical URL for Locations, so we should reflect it in the model - and thereby make it easier for the maintenance task subsystem to make links to location components.
The imports were getting chaotic in `views.py`. Used isort to clean it up.
This more or less removes the redundant strings used in the function, by using a list of allowed/expected component keys.
Add each component error from POST data processing as its own error message. It got very unreadable when a bug caused all component keys to generate errors.
Why hardcode the list of component keys to put in the data structure when we already have the definitive list in a constant?
This replaces `utils.components_for_keys()` with `get_components_from_keydict()`, which works more like the newer `get_components()`, but works on POST data rather than Task object data. The end result is an edit view that builds the component trails use in the maintenance in much the same way as the refactored `view()` view, which means that the HTML template for the `edit()` view can now also re-use the `frag-component-trail.html` template.
These are no longer used by any code.
It's stupid to keep a hardcoded list of component types that only accept integer primary keys, when can be detected from the corresponding model definitions. This takes away the redundancy.
|
🦙 MegaLinter status: ✅ SUCCESS
See detailed report in MegaLinter reports MegaLinter is graciously provided by |
Test results 8 files 8 suites 8m 11s ⏱️ For more details on these failures, see this check. Results for commit 26389a5. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3230 +/- ##
==========================================
- Coverage 60.39% 60.37% -0.02%
==========================================
Files 605 605
Lines 43687 43665 -22
Branches 48 48
==========================================
- Hits 26383 26362 -21
+ Misses 17292 17291 -1
Partials 12 12 ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
|
Branch name and history are off, closing to create a new PR |
lunkwill42
deleted the
feature/maintenance-close-on-deleted-components
branch
lunkwill42
added a commit
that referenced
this pull request
Nov 29, 2024
…eted-component-display Refactor maintenance task views to improve display of deleted components #3230
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This fixes #3228, but it was not feasible without a significant refactoring of the maintenance task UI codebase.
The codebase originally stems from a time before Django was added to NAV, and was significantly convoluted compared to what it needs to be. This PR makes it slightly less so, but there is still some way to go:
There was a lot of processing and custom validation code for the component fields of the maintenance task forms, which ended up producing large hierarchies of dicts and lists (some of which were fed directly to templates), instead of re-using ORM functionality. While much of the processing would be best encapsulated in Django Forms, this refactor mostly concerns itself with using ORM objects where we can.
The ultimate goal was really only to be able to display deleted components in a better way, by utilizing a new component
descriptionfield. This is achieved in this PR by replacing the template render data witch actual lists of Django model objects, and by replacing the missing objects with a specialMissingComponentclass.Please do ask for clarifications if necessary.