Ensure "Notes" is always the last field#1495
Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Ensures the "Notes" field is always displayed last within each component in the component editor by updating component field reordering logic and adding regression tests to lock in the behavior.
Changes:
- Add multiple tests verifying
"Notes"is moved to the end (and stays there) across varied component shapes and duringupdate_json_structure(). - Update
_reorder_components()to move"Notes"to the end of each component after other field reordering (e.g., Product subfields).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| tests/test_data_model_vehicle_components_base.py | Adds test coverage to ensure "Notes" is always the final field across scenarios, including migration via update_json_structure(). |
| ardupilot_methodic_configurator/data_model_vehicle_components_base.py | Updates _reorder_components() to pop/reinsert "Notes" so it becomes the last key in each component dict. |
| if "Notes" in component_data: | ||
| notes_value = component_data.pop("Notes") | ||
| component_data["Notes"] = notes_value |
There was a problem hiding this comment.
component_data.pop("Notes") will raise at runtime if component_data is not a dict (e.g., a list or other non-mapping). Since this method iterates over arbitrary loaded component data, add an isinstance(component_data, dict) guard (similar to the existing "Product" dict guard) before calling pop, or otherwise ensure component_data is treated as a mapping consistently.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Amilcar Lucas <amilcar.lucas@iav.de>
☂️ Python Coverage
Overall Coverage
New FilesNo new covered files... Modified FilesNo covered modified files...
|
Coverage Report for CI Build 24364119634Warning Build has drifted: This PR's base is out of sync with its target branch, so coverage data may include unrelated changes. Coverage increased (+0.001%) to 94.169%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
Make sure "Notes" is always the last field of a component in the component editor