Skip to content

Comments

Fixes #2601 Error when selecting "Link Data To Simulations"#2602

Merged
msevestre merged 6 commits intodevelopfrom
2601-error-when-selecting-link-data-to-simulations
Aug 27, 2025
Merged

Fixes #2601 Error when selecting "Link Data To Simulations"#2602
msevestre merged 6 commits intodevelopfrom
2601-error-when-selecting-link-data-to-simulations

Conversation

@rwmcintosh
Copy link
Member

Fixes #2601

Description

Some order and selection issues when we are trying to update Used state for a linked observation column as we select/deselect the matching calculation column.

Type of change

Please mark relevant options with an x in the brackets.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires documentation changes (link at least one user or developer documentation issue):
  • Algorithm update - updates algorithm documentation/questions/answers etc.
  • Other (please describe):

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

  • Integration tests
  • Unit tests
  • Manual tests
  • No tests required

Reviewer checklist

Mark everything that needs to be checked before merging the PR.

  • Check if the code is well documented
  • Check if the behavior is what is expected
  • Check if the code is well tested
  • Check if the code is readable and well formatted
  • Additional checks (document below if any)
  • Check if documentation update issue(s) are created if the option This change requires a documentation update above is selected

Screenshots (if appropriate):

Questions (if appropriate):

Comment on lines -18 to -20
/// Returns all selected <see cref="DataColumnDTO"/> and their descendants.
/// </summary>
IReadOnlyList<DataColumnDTO> SelectedDescendantColumns { get; }
Copy link
Member Author

Choose a reason for hiding this comment

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

I removed this option because I think we want the group rows to function as a multi select. In other words, if I select the 'Simulation' group and then I set the Used checkbox, all simulation outputs should be added. Selecting the group is the same as selecting all the rows of the group.

Copy link
Member

Choose a reason for hiding this comment

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

what if you have multiple grouping? Does that work?

Copy link
Member Author

Choose a reason for hiding this comment

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

yep. Group rows have negative row indexes. The extension method takes care of it all.

@rwmcintosh rwmcintosh added this to v13 and v12.1 Aug 25, 2025
@rwmcintosh rwmcintosh removed this from v13 Aug 25, 2025
@rwmcintosh rwmcintosh moved this to In Review in v12.1 Aug 25, 2025
@rwmcintosh
Copy link
Member Author

@benjaperez1983 can you revert the changes you pushed to this branch and create a new PR for the color changes

Copy link
Member

@msevestre msevestre left a comment

Choose a reason for hiding this comment

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

Unclear to me where the code fixes the issusue 2601. There is lots of noise around setting colors etc but the actual "Sequence" not found is not seen at first glance
Naming is hard..isLinkedDataToSimulation remains a mystery.
Does it mean that oberved data is llink to simuatlion results?

@rwmcintosh
Copy link
Member Author

Unclear to me where the code fixes the issusue 2601. There is lots of noise around setting colors etc but the actual "Sequence" not found is not seen at first glance
Naming is hard..isLinkedDataToSimulation remains a mystery.
Does it mean that oberved data is llink to simuatlion results?

I asked Ben to reverse his color changes from this pr

@rwmcintosh
Copy link
Member Author

I think the color matching should go into v13 rather than 12.1. Difference is that's a new feature where this original report is a crash fix.

@rwmcintosh rwmcintosh requested a review from msevestre August 27, 2025 12:45
@msevestre msevestre merged commit aaef1ba into develop Aug 27, 2025
1 check passed
@msevestre msevestre deleted the 2601-error-when-selecting-link-data-to-simulations branch August 27, 2025 15:04
@github-project-automation github-project-automation bot moved this from In Review to Done in v12.1 Aug 27, 2025
@Yuri05 Yuri05 moved this from Done to Verified in v12.1 Aug 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

No open projects
Status: Verified

Development

Successfully merging this pull request may close these issues.

Error when selecting "Link Data To Simulations"

3 participants