Task/rdmp 360 catalogue status change behaviour#2319
Conversation
…sk/RDMP-360-catalogue-status-change-behaviour
…sk/RDMP-360-catalogue-status-change-behaviour
|
|
||
| public List<Thread> Threads = new(); | ||
| private ICoreChildProvider _coreChildProvider; | ||
| private IBasicActivateItems _activator; |
Check notice
Code scanning / CodeQL
Missed 'readonly' opportunity Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 15 days ago
In general, to fix a "Missed 'readonly' opportunity" on a field, you add the readonly modifier to the field declaration, provided that all assignments to the field occur either at the declaration or within constructors of the same class, and there are no other writes (e.g., in methods, properties, or external code via ref).
Here, the best fix is to change the declaration of _activator on line 80 from private IBasicActivateItems _activator; to private readonly IBasicActivateItems _activator;. This preserves existing behavior because the field is still assigned in the constructor and read elsewhere; it only prevents any future non-constructor code from reassigning _activator. No additional imports, methods, or other definitions are required, as readonly is a built-in C# modifier and we are not altering how _activator is used.
Concretely:
- In
Rdmp.Core/CohortCreation/Execution/CohortCompiler.cs, locate the field declaration around line 80. - Replace
private IBasicActivateItems _activator;withprivate readonly IBasicActivateItems _activator;. - No other lines in the provided snippet need to be changed.
| @@ -77,7 +77,7 @@ | ||
|
|
||
| public List<Thread> Threads = new(); | ||
| private ICoreChildProvider _coreChildProvider; | ||
| private IBasicActivateItems _activator; | ||
| private readonly IBasicActivateItems _activator; | ||
|
|
||
| public CohortCompiler(IBasicActivateItems activator, CohortIdentificationConfiguration cohortIdentificationConfiguration) | ||
| { |
| private ExecuteCommandClearQueryCache _clearCacheCommand; | ||
|
|
||
| private CohortIdentificationConfigurationUICommon Common = new(); | ||
| private CohortIdentificationConfigurationUICommon Common = new(null); |
Check notice
Code scanning / CodeQL
Missed 'readonly' opportunity Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 15 days ago
To fix the issue, the field should be declared with the readonly modifier so it cannot be reassigned after object construction. This preserves current functionality because readonly still allows assignments at declaration and in any constructor of the same class, which covers the two existing assignments we see.
Concretely, in Rdmp.UI/SubComponents/CohortIdentificationConfigurationUI.cs, update the field declaration at line 76 from private CohortIdentificationConfigurationUICommon Common = new(null); to private readonly CohortIdentificationConfigurationUICommon Common = new(null);. No other code needs to change: the assignment in the constructor remains legal because readonly fields can be set in constructors. No new imports or helper methods are required.
| @@ -73,7 +73,7 @@ | ||
|
|
||
| private ExecuteCommandClearQueryCache _clearCacheCommand; | ||
|
|
||
| private CohortIdentificationConfigurationUICommon Common = new(null); | ||
| private readonly CohortIdentificationConfigurationUICommon Common = new(null); | ||
|
|
||
| public CohortIdentificationConfigurationUI() | ||
| { |
| { | ||
| private readonly IBasicActivateItems _activator; | ||
| private CohortIdentificationConfigurationUICommon Common = new (); | ||
| private CohortIdentificationConfigurationUICommon Common = new (null); |
Check notice
Code scanning / CodeQL
Missed 'readonly' opportunity Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 15 days ago
To fix this, we should add the readonly modifier to the Common field declaration so that it cannot be reassigned after the constructor finishes. This keeps current behavior—mutating properties on the CohortIdentificationConfigurationUICommon instance remains fully allowed—while preventing unintended reassignment of Common to a different instance.
Concretely, in Tools/rdmp/CommandLine/Gui/ConsoleGuiCohortIdentificationConfigurationUI.cs, change the field declaration at line 26 from:
private CohortIdentificationConfigurationUICommon Common = new (null);to:
private readonly CohortIdentificationConfigurationUICommon Common = new (null);No additional methods, imports, or other definitions are required; this is a local modifier change on the existing field and will not alter any external behavior aside from disallowing reassignment of Common within this class.
| @@ -23,7 +23,7 @@ | ||
| public partial class ConsoleGuiCohortIdentificationConfigurationUI | ||
| { | ||
| private readonly IBasicActivateItems _activator; | ||
| private CohortIdentificationConfigurationUICommon Common = new (null); | ||
| private readonly CohortIdentificationConfigurationUICommon Common = new (null); | ||
| private bool _isDisposed; | ||
| private List<object> RowObjects = new(); | ||
| private bool _contextMenuShowing = false; |
Proposed Change
When a catalogue is marked as project specific, prevent it fro mbeing used in cohort builds.
This PR is around the execution on non-project CICs that already have the catalogue in the CIC build
Type of change
What types of changes does your code introduce? Tick all that apply.
Checklist
By opening this PR, I confirm that I have: