Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -432,6 +432,7 @@ def _reorder_components(self, existing_components: ComponentData) -> ComponentDa
reordered_components.update(remaining_components)

# Second step: for each component, ensure Product fields are in correct order (Version before URL)
# and ensure "Notes" is always the last field
for component_name, component_data in reordered_components.items():
if "Product" in component_data and isinstance(component_data["Product"], dict):
product = component_data["Product"]
Expand All @@ -448,6 +449,10 @@ def _reorder_components(self, existing_components: ComponentData) -> ComponentDa
ordered_product[field] = value
reordered_components[component_name]["Product"] = ordered_product

if "Notes" in component_data:
notes_value = component_data.pop("Notes")
component_data["Notes"] = notes_value
Comment on lines +452 to +454
Copy link

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.

return reordered_components

def init_battery_chemistry(self) -> None:
Expand Down
156 changes: 156 additions & 0 deletions tests/test_data_model_vehicle_components_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -1529,3 +1529,159 @@ def test_user_sees_complex_product_sections_with_extra_fields_preserved(self, mo
assert product["Custom Field 1"] == "Custom Value 1"
assert product["Custom Field 2"] == "Custom Value 2"
assert product["Custom Field 3"] == "Custom Value 3"

def test_user_sees_notes_field_always_last_in_component(self, model_instance) -> None:
"""
User sees the Notes field displayed at the bottom of every component editor section.

GIVEN: A component where "Notes" appears before other fields
WHEN: The system processes the component data
THEN: "Notes" is moved to be the very last field in that component
AND: All other field values and their order are preserved
"""
# Arrange: Component with Notes appearing before connection data
existing_components = {
"Telemetry": {
"Notes": "SiK radio on Telem1",
"Product": {"Manufacturer": "RFDesign", "Model": "RFD900x"},
"FC Connection": {"Type": "SERIAL1", "Protocol": "MAVLink2"},
}
}

# Act
result = model_instance._reorder_components(existing_components)

# Assert: Notes is last
fields = list(result["Telemetry"].keys())
assert fields[-1] == "Notes", "Notes should be the last field in the component"
assert result["Telemetry"]["Notes"] == "SiK radio on Telem1"
# Other fields are preserved in their original relative order
original_non_notes_fields = [key for key in existing_components["Telemetry"] if key != "Notes"]
reordered_non_notes_fields = [key for key in fields if key != "Notes"]
assert reordered_non_notes_fields == original_non_notes_fields

def test_user_sees_notes_field_last_when_already_at_end(self, model_instance) -> None:
"""
User sees Notes remain last when it is already the last field.

GIVEN: A component where "Notes" is already the last field
WHEN: The system processes the component data
THEN: "Notes" stays last and all other fields are untouched
"""
# Arrange: Notes already last
existing_components = {
"GNSS Receiver": {
"Product": {"Manufacturer": "u-blox", "Model": "NEO-M8N"},
"FC Connection": {"Type": "SERIAL3", "Protocol": "AUTO"},
"Notes": "Primary GPS",
}
}

# Act
result = model_instance._reorder_components(existing_components)

# Assert
fields = list(result["GNSS Receiver"].keys())
assert fields[-1] == "Notes"
assert result["GNSS Receiver"]["Notes"] == "Primary GPS"

def test_user_sees_components_without_notes_field_unaffected(self, model_instance) -> None:
"""
User sees components that have no Notes field remain completely unaffected by Notes ordering.

GIVEN: Components with no Notes field
WHEN: The system processes the component data
THEN: All component fields are preserved without modification
"""
# Arrange
existing_components = {
"Motors": {
"Product": {"Manufacturer": "T-Motor", "Model": "F60 Pro IV"},
"Specifications": {"Poles": 14},
},
"Propellers": {
"Product": {"Manufacturer": "HQProp", "Model": "5.1x4.6"},
"Specifications": {"Diameter_inches": 5.1},
},
}

# Act
result = model_instance._reorder_components(existing_components)

# Assert: No Notes key injected, all fields unchanged
assert "Notes" not in result["Motors"]
assert "Notes" not in result["Propellers"]
assert list(result["Motors"].keys()) == ["Product", "Specifications"]
assert list(result["Propellers"].keys()) == ["Product", "Specifications"]

def test_user_sees_notes_last_across_all_components_simultaneously(self, model_instance) -> None:
"""
User sees Notes consistently last in every component when multiple components are processed together.

GIVEN: Multiple components each with Notes in different positions
WHEN: The system processes all component data at once
THEN: Notes is the last field in every component that has it
AND: Components without Notes are unaffected
"""
# Arrange: Multiple components with Notes in varying positions
existing_components = {
"Flight Controller": {
"Notes": "flight controller notes",
"Product": {"Manufacturer": "Holybro", "Model": "Pixhawk 6C"},
"Firmware": {"Type": "ArduCopter", "Version": "4.6.x"},
},
"Battery": {
"Product": {"Manufacturer": "Tattu"},
"Notes": "battery notes",
"Specifications": {"Chemistry": "Lipo"},
},
"ESC": {
"Product": {"Manufacturer": "Mamba"},
"FC->ESC Connection": {"Type": "Main Out", "Protocol": "DShot600"},
"ESC->FC Telemetry": {"Type": "Main Out", "Protocol": "BDShot"},
"Notes": "esc notes",
},
"Motors": {
"Specifications": {"Poles": 14},
# No Notes field
},
}

# Act
result = model_instance._reorder_components(existing_components)

# Assert: Notes is last in every component that has it
for component_name in ["Flight Controller", "Battery", "ESC"]:
fields = list(result[component_name].keys())
assert fields[-1] == "Notes", f"Notes should be last in {component_name}, got: {fields}"

# Motors has no Notes and should be unaffected
assert "Notes" not in result["Motors"]

def test_system_moves_notes_last_during_update_json_structure(self, model_instance) -> None:
"""
System moves Notes to be the last field during a full update_json_structure call.

GIVEN: A stored vehicle_components.json where Notes appears before connection data
WHEN: update_json_structure is called on load
THEN: Notes is the last field in the affected component after migration
"""
# Arrange: Inject raw data with Notes in the middle
model_instance._data = {
"Format version": 1,
"Components": {
"RC Receiver": {
"Notes": "FrSky receiver",
"Product": {"Manufacturer": "FrSky", "Model": "R-XSR"},
"FC Connection": {"Type": "RCin/SBUS", "Protocol": "SBUS"},
}
},
}

# Act
model_instance.update_json_structure()

# Assert
rc_fields = list(model_instance._data["Components"]["RC Receiver"].keys())
assert rc_fields[-1] == "Notes", f"Notes should be last after update_json_structure, got: {rc_fields}"
assert model_instance._data["Components"]["RC Receiver"]["Notes"] == "FrSky receiver"
Loading