SC-442 Fix IsEnd assignment for line segments - #918
Open
StephenCWills wants to merge 3 commits into
Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Refactors the line-segment wizard save flow to simplify and speed up topology updates, and fixes persistence of the IsEnd flag so client changes correctly reach the database.
Changes:
- Refactors
LineSegmentWizardController.PostData()into clearer, more set-based operations (joins/grouping) for deletes/updates/inserts and connection rebuilds. - Recomputes
LineSegment.IsEndbased on bus topology at the end of the save operation (to enforce consistent end-flagging). - Fixes
OpenXDAAssetControllerto copyIsEndfrom the incoming JSON payload into the DB model.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| Source/Applications/SystemCenter/Controllers/OpenXDA/Assets/OpenXDAAssetController.cs | Copies IsEnd from client JSON into LineSegment model during JToken → model mapping. |
| Source/Applications/SystemCenter/Controllers/ExternalDB/LineSegmentWizardController.cs | Refactors save logic and rebuilds segment connections/locations; recomputes and persists IsEnd from topology. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+281
to
283
| List<Segment> allSegments = [.. record.Sections | ||
| .SelectMany(s => s.Segments)]; | ||
|
|
Comment on lines
+311
to
+313
| segmentTbl.UpdateRecord(oldSegment); | ||
| oldSegment.ConnectedSegments.ForEach(cxn => segmentCxnTbl.DeleteRecord(cxn)); | ||
| oldSegment.AssetLocations.ForEach(loc => assetLocationTbl.DeleteRecord(loc)); |
This file contains hidden or 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
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.
The logic in
LineSegmentWizardController.PostData()was a bit of a mess so I tried to break it into a distinct set of tasks that could each be done independently and therefore verified for correctness. As a result, there is a rather large diff, but most of the logic is the same. Mainly, I tried to reduce nested loops, nested Linq queries, and loops handling multiple tasks intoJoin()/GroupJoin()/GroupBy()calls to improve performance and cut down on the complexity of the code. Some of the Linq queries are still a little tricky to comprehend, though.Functionally, the only thing that changed is at the very end of the method, where it walks through buses to set the
IsEndflag. The logic there ensures that we changetruetofalseor vice-versa as needed to achieve the expected state based on which segments are connected to each tap. This will completely overwrite any manual adjustments if the wizard is used to update the configuration, but it will ensure that theIsEndflags are always selected based on the most logical interpretation by just looking at the topology of the line.As for
OpenXDAAssetController, it simply wasn't copying theIsEndflag from the client into the database model. That's why the database wouldn't update when the user clicked theSave Changesbutton.