Skip to content

Commit

Permalink
Bugfix/rdmp 253 filter ordering (#2007)
Browse files Browse the repository at this point in the history
* add start

* partial

* tidy up code

* filter updates

* update db migrations

* update correct db

* update changelog

* add base creation sql

* fix create

* tidy up
  • Loading branch information
JFriel authored Sep 26, 2024
1 parent 2a516bf commit 53596b0
Show file tree
Hide file tree
Showing 14 changed files with 131 additions and 13 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@


# Changelog
All notable changes to this project will be documented in this file.

Expand All @@ -7,6 +8,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [8.4.0] - Unreleased

- Add Ordering to Filters

## [8.3.1] - Unreleased

- Improve Performance of regenerating problems with child providers
Expand Down
6 changes: 5 additions & 1 deletion Rdmp.Core/Curation/Data/Aggregation/AggregateFilter.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) The University of Dundee 2018-2019
// Copyright (c) The University of Dundee 2018-2024
// This file is part of the Research Data Management Platform (RDMP).
// RDMP is free software: you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free Software Foundation, either version 3 of the License, or (at your option) any later version.
// RDMP is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for more details.
Expand Down Expand Up @@ -35,6 +35,7 @@ public class AggregateFilter : ConcreteFilter, IDisableable
private int? _clonedFromExtractionFilterID;
private int? _associatedColumnInfoID;
private bool _isDisabled;
private int _order;

/// <inheritdoc/>
public override int? ClonedFromExtractionFilter_ID
Expand Down Expand Up @@ -90,6 +91,8 @@ public IEnumerable<AggregateFilterParameter> AggregateFilterParameters
? Repository.GetObjectByID<AggregateFilterContainer>(FilterContainer_ID.Value)
: null;

public override int Order { get => _order; set => SetField(ref _order, value); }

#endregion

public AggregateFilter()
Expand Down Expand Up @@ -121,6 +124,7 @@ internal AggregateFilter(ICatalogueRepository repository, DbDataReader r) : base
Name = r["Name"] as string;
IsMandatory = (bool)r["IsMandatory"];
ClonedFromExtractionFilter_ID = ObjectToNullableInt(r["ClonedFromExtractionFilter_ID"]);
Order = int.Parse(r["Order"].ToString());

Check warning

Code scanning / CodeQL

Virtual call in constructor or destructor Warning

Avoid virtual calls in a constructor or destructor.

var associatedColumnInfo_ID = r["AssociatedColumnInfo_ID"];
if (associatedColumnInfo_ID != DBNull.Value)
Expand Down
5 changes: 3 additions & 2 deletions Rdmp.Core/Curation/Data/ConcreteFilter.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) The University of Dundee 2018-2019
// Copyright (c) The University of Dundee 2018-2024
// This file is part of the Research Data Management Platform (RDMP).
// RDMP is free software: you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free Software Foundation, either version 3 of the License, or (at your option) any later version.
// RDMP is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for more details.
Expand All @@ -25,7 +25,7 @@ namespace Rdmp.Core.Curation.Data;
/// <para>ConcreteFilter is used to provide UI editing of an IFilter without having to add persistence / DatabaseEntity logic to IFilter (which would break
/// SpontaneouslyInventedFilters)</para>
/// </summary>
public abstract class ConcreteFilter : DatabaseEntity, IFilter, ICheckable
public abstract class ConcreteFilter : DatabaseEntity, IFilter, ICheckable, IOrderable
{
/// <inheritdoc/>
protected ConcreteFilter(IRepository repository, DbDataReader r) : base(repository, r)
Expand Down Expand Up @@ -100,6 +100,7 @@ public bool IsMandatory
/// <inheritdoc cref="FilterContainer_ID"/>
[NoMappingToDatabase]
public abstract IContainer FilterContainer { get; }
public abstract int Order { get; set; }

#endregion

Expand Down
6 changes: 5 additions & 1 deletion Rdmp.Core/Curation/Data/ExtractionFilter.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) The University of Dundee 2018-2019
// Copyright (c) The University of Dundee 2018-2024
// This file is part of the Research Data Management Platform (RDMP).
// RDMP is free software: you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free Software Foundation, either version 3 of the License, or (at your option) any later version.
// RDMP is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for more details.
Expand Down Expand Up @@ -40,6 +40,7 @@ public class ExtractionFilter : ConcreteFilter, IHasDependencies, IInjectKnown<E

private int _extractionInformationID;
private Lazy<ExtractionFilterParameterSet[]> _knownExtractionFilterParameterSets;
private int _order;

/// <summary>
/// The column in the <see cref="Catalogue"/> which is best/most associated with this filter. A filter can query any column in any of the table(s) under
Expand Down Expand Up @@ -133,6 +134,8 @@ internal ExtractionFilter(ICatalogueRepository repository, DbDataReader r)
Description = r["Description"] as string;
Name = r["Name"] as string;
IsMandatory = (bool)r["IsMandatory"];
Order = int.Parse(r["Order"].ToString());

Check warning

Code scanning / CodeQL

Virtual call in constructor or destructor Warning

Avoid virtual calls in a constructor or destructor.


ClearAllInjections();
}
Expand All @@ -154,6 +157,7 @@ public override int? ClonedFromExtractionFilter_ID
set => throw new NotSupportedException(
"ClonedFromExtractionFilter_ID is only supported on lower level filters e.g. DeployedExtractionFilter and AggregateFilter");
}
public override int Order { get => _order; set => SetField(ref _order,value); }

/// <inheritdoc/>
public IHasDependencies[] GetObjectsThisDependsOn()
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) The University of Dundee 2018-2019
// Copyright (c) The University of Dundee 2018-2024
// This file is part of the Research Data Management Platform (RDMP).
// RDMP is free software: you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free Software Foundation, either version 3 of the License, or (at your option) any later version.
// RDMP is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for more details.
Expand All @@ -22,6 +22,7 @@ public class SpontaneouslyInventedFilter : ConcreteFilter
{
private readonly MemoryCatalogueRepository _repo;
private readonly ISqlParameter[] _filterParametersIfAny;
private int _order =0;

/// <summary>
/// Creates a new temporary (unsaveable) filter in the given memory <paramref name="repo"/>
Expand Down Expand Up @@ -68,6 +69,8 @@ public SpontaneouslyInventedFilter(MemoryCatalogueRepository repo, IFilter copyF
? _repo.GetObjectByID<IContainer>(FilterContainer_ID.Value)
: null;

public override int Order { get => _order; set => SetField(ref _order, value); }

public override ColumnInfo GetColumnInfoIfExists() => null;

public override IFilterFactory GetFilterFactory() => null;
Expand Down
6 changes: 5 additions & 1 deletion Rdmp.Core/DataExport/Data/DeployedExtractionFilter.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) The University of Dundee 2018-2019
// Copyright (c) The University of Dundee 2018-2024
// This file is part of the Research Data Management Platform (RDMP).
// RDMP is free software: you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free Software Foundation, either version 3 of the License, or (at your option) any later version.
// RDMP is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for more details.
Expand Down Expand Up @@ -36,6 +36,7 @@ public class DeployedExtractionFilter : ConcreteFilter

private int? _clonedFromExtractionFilterID;
private int? _filterContainerID;
private int _order;

/// <inheritdoc/>
public override int? ClonedFromExtractionFilter_ID
Expand Down Expand Up @@ -69,6 +70,8 @@ public override int? FilterContainer_ID
? Repository.GetObjectByID<FilterContainer>(FilterContainer_ID.Value)
: null;

public override int Order { get => _order; set => SetField(ref _order, value); }

#endregion

/// <inheritdoc/>
Expand Down Expand Up @@ -138,6 +141,7 @@ internal DeployedExtractionFilter(IDataExportRepository repository, DbDataReader
FilterContainer_ID = null;

ClonedFromExtractionFilter_ID = ObjectToNullableInt(r["ClonedFromExtractionFilter_ID"]);
Order = int.Parse(r["Order"].ToString());

Check warning

Code scanning / CodeQL

Virtual call in constructor or destructor Warning

Avoid virtual calls in a constructor or destructor.
}

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,7 @@ CREATE TABLE [dbo].[AggregateFilter](
[AssociatedColumnInfo_ID] [int] NULL,
[ID] [int] IDENTITY(1,1) NOT NULL,
[SoftwareVersion] [nvarchar](50) NOT NULL,
[Order] [int] NOT NULL DEFAULT 0
CONSTRAINT [PK_AggregateFilter] PRIMARY KEY CLUSTERED
(
[ID] ASC
Expand Down Expand Up @@ -464,6 +465,7 @@ CREATE TABLE [dbo].[ExtractionFilter](
[Name] [varchar](100) NOT NULL,
[IsMandatory] [bit] NOT NULL,
[SoftwareVersion] [nvarchar](50) NOT NULL,
[Order] [int] NOT NULL DEFAULT 0
CONSTRAINT [PK_ExtractionFilter] PRIMARY KEY CLUSTERED
(
[ID] ASC
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
----Version: 8.4.0
----Description: Add Order to Aggregate Filters

if not exists (select 1 from sys.columns where name = 'Order' and OBJECT_NAME(object_id) = 'AggregateFilter')
BEGIN
ALTER TABLE [dbo].[AggregateFilter]
ADD [Order] [int] NOT NULL DEFAULT 0 WITH VALUES
END
if not exists (select 1 from sys.columns where name = 'Order' and OBJECT_NAME(object_id) = 'ExtractionFilter')
BEGIN
ALTER TABLE [dbo].[ExtractionFilter]
ADD [Order] [int] NOT NULL DEFAULT 0 WITH VALUES
END
Binary file not shown.
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
----Version: 8.4.0
----Description: Add Order to Aggregate Filters

if not exists (select 1 from sys.columns where name = 'Order' and OBJECT_NAME(object_id) = 'DeployedExtractionFilter')
BEGIN
ALTER TABLE [dbo].[DeployedExtractionFilter]
ADD [Order] [int] NOT NULL DEFAULT 0 WITH VALUES
END
4 changes: 4 additions & 0 deletions Rdmp.Core/Rdmp.Core.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@
<None Remove="Databases\CatalogueDatabase\up\084_AddLoadDirectorySplit.sql" />
<None Remove="Databases\CatalogueDatabase\up\085_AddTicketingReleaseStatuses.sql" />
<None Remove="Databases\CatalogueDatabase\up\086_AddDataLoadPrefixOverride.sql" />
<None Remove="Databases\CatalogueDatabase\up\087_AddAggregateFilterOrdering.sql" />
<None Remove="Databases\DataExportDatabase\runAfterCreateDatabase\CreateDataExportManager.sql" />
<None Remove="Databases\DataExportDatabase\up\001_AddUsernamePasswordFieldsToExternalCohortTable.sql" />
<None Remove="Databases\DataExportDatabase\up\002_FixServerAndDatabaseNameOnExternalCohort.sql" />
Expand Down Expand Up @@ -157,6 +158,7 @@
<None Remove="Databases\DataExportDatabase\up\024_AddExtractionProgressTable.sql" />
<None Remove="Databases\DataExportDatabase\up\025_AddExtractionProgressRetry.sql" />
<None Remove="Databases\DataExportDatabase\up\025_AddFolders.sql" />
<None Remove="Databases\DataExportDatabase\up\026_AddFilterOrder.sql" />
<None Remove="Databases\DataQualityEngineDatabase\runAfterCreateDatabase\CreateTables.sql" />
<None Remove="Databases\DataQualityEngineDatabase\up\001_AnnotationsAndDiagram.sql" />
<None Remove="Databases\DataQualityEngineDatabase\up\002_AddPivotCategoryLogic.sql" />
Expand Down Expand Up @@ -255,6 +257,7 @@
<EmbeddedResource Include="Databases\CatalogueDatabase\up\084_AddLoadDirectorySplit.sql" />
<EmbeddedResource Include="Databases\CatalogueDatabase\up\085_AddTicketingReleaseStatuses.sql" />
<EmbeddedResource Include="Databases\CatalogueDatabase\up\086_AddDataLoadPrefixOverride.sql" />
<EmbeddedResource Include="Databases\CatalogueDatabase\up\087_AddAggregateFilterOrdering.sql" />
<EmbeddedResource Include="Databases\CatalogueDatabase\up\079_AddProcessTaskConfiguration.sql" />
<EmbeddedResource Include="Databases\CatalogueDatabase\up\082_AddCohortVersioning.sql" />
<EmbeddedResource Include="Databases\CatalogueDatabase\up\083_AddGroupBy.sql" />
Expand Down Expand Up @@ -297,6 +300,7 @@
<EmbeddedResource Include="Databases\DataExportDatabase\up\020_AddReleaseLogIDColumn.sql" />
<EmbeddedResource Include="Databases\DataExportDatabase\up\022_RowVer.sql" />
<EmbeddedResource Include="Databases\DataExportDatabase\up\025_AddFolders.sql" />
<EmbeddedResource Include="Databases\DataExportDatabase\up\026_AddFilterOrder.sql" />
<EmbeddedResource Include="Databases\DataQualityEngineDatabase\runAfterCreateDatabase\CreateTables.sql" />
<EmbeddedResource Include="Databases\DataQualityEngineDatabase\up\001_AnnotationsAndDiagram.sql" />
<EmbeddedResource Include="Databases\DataQualityEngineDatabase\up\002_AddPivotCategoryLogic.sql" />
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
// Copyright (c) The University of Dundee 2024-2024
// This file is part of the Research Data Management Platform (RDMP).
// RDMP is free software: you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free Software Foundation, either version 3 of the License, or (at your option) any later version.
// RDMP is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for more details.
// You should have received a copy of the GNU General Public License along with RDMP. If not, see <https://www.gnu.org/licenses/>.

using Rdmp.Core.Curation.Data;
using Rdmp.Core.MapsDirectlyToDatabaseTable;
using Rdmp.UI.ItemActivation;
using System;
using System.Linq;

namespace Rdmp.UI.CommandExecution.AtomicCommands;

public class ExecuteCommandReorderFilter : BasicUICommandExecution
{
private ConcreteFilter _source;

Check notice

Code scanning / CodeQL

Missed 'readonly' opportunity Note

Field '_source' can be 'readonly'.
private ConcreteFilter _target;

Check notice

Code scanning / CodeQL

Missed 'readonly' opportunity Note

Field '_target' can be 'readonly'.
private InsertOption _insertOption;

Check notice

Code scanning / CodeQL

Missed 'readonly' opportunity Note

Field '_insertOption' can be 'readonly'.

public ExecuteCommandReorderFilter(IActivateItems activator, ConcreteFilter source, ConcreteFilter destination, InsertOption insertOption) : base(activator)
{
_source = source;
_target = destination;
_insertOption = insertOption;
if (_source.FilterContainer_ID is null || _target.FilterContainer_ID is null)
{
SetImpossible("Both filters must exist within some container in order to be orderable");
}
if (_source.FilterContainer_ID != _target.FilterContainer_ID)
{
SetImpossible("Cannot reorder filters as they do not share a parent");
}
}

public override void Execute()
{
var order = _target.Order;

var filters = _target.FilterContainer.GetFilters().Where(f => f is ConcreteFilter).Select(f => (ConcreteFilter)f).ToArray();
Array.Sort(
filters,
delegate (ConcreteFilter a, ConcreteFilter b) { return a.Order.CompareTo(b.Order); }
);
if (!filters.All(c => c.Order != order))
{
foreach (var orderable in filters)
{
if (orderable.Order < order)
orderable.Order--;
else if (orderable.Order > order)
orderable.Order++;
else //collision on order
orderable.Order += _insertOption == InsertOption.InsertAbove ? 1 : -1;
((ISaveable)orderable).SaveToDatabase();
}
}
_source.Order = order;
_source.SaveToDatabase();
Publish(_target.FilterContainer);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@
// You should have received a copy of the GNU General Public License along with RDMP. If not, see <https://www.gnu.org/licenses/>.

using Rdmp.Core.CommandExecution;
using Rdmp.Core.CommandExecution.Combining;
using Rdmp.Core.Curation.Data;
using Rdmp.UI.CommandExecution.AtomicCommands;
using Rdmp.UI.ExtractionUIs.FilterUIs;
using Rdmp.UI.ItemActivation;

Expand All @@ -24,8 +26,16 @@ public override void Activate(ConcreteFilter target)
ItemActivator.Activate<ExtractionFilterUI, ConcreteFilter>(target);
}

public override ICommandExecution ProposeExecution(ICombineToMakeCommand cmd, ConcreteFilter target,
InsertOption insertOption = InsertOption.Default) =>
//currently nothing can be dropped onto a filter
null;
public override ICommandExecution ProposeExecution(ICombineToMakeCommand cmd, ConcreteFilter targetFilter,
InsertOption insertOption = InsertOption.Default)
{
return cmd switch
{
FilterCombineable sourceFilterCommand =>
!sourceFilterCommand.Filter.Equals(targetFilter) && sourceFilterCommand.Filter is ConcreteFilter && sourceFilterCommand.Filter.FilterContainer_ID == targetFilter.FilterContainer_ID ?
new ExecuteCommandReorderFilter(ItemActivator, (ConcreteFilter)sourceFilterCommand.Filter, targetFilter, insertOption)
: null,
_ => null
};
}
}
6 changes: 3 additions & 3 deletions SharedAssemblyInfo.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,6 @@
[assembly: AssemblyTrademark("")]
[assembly: AssemblyCulture("")]

[assembly: AssemblyVersion("8.3.0")]
[assembly: AssemblyFileVersion("8.3.0")]
[assembly: AssemblyInformationalVersion("8.3.0")]
[assembly: AssemblyVersion("8.4.0")]
[assembly: AssemblyFileVersion("8.4.0")]
[assembly: AssemblyInformationalVersion("8.4.0")]

0 comments on commit 53596b0

Please sign in to comment.