Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

1409 importer handling of imvalid files #1437

Merged
merged 21 commits into from
Feb 9, 2022
Merged

1409 importer handling of imvalid files #1437

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
b728d54
Should consider columns with string values
abdelr Jan 11, 2022
3f308dc
based format on first valid sheet
abdelr Jan 11, 2022
e53b503
Fixing tests
abdelr Jan 11, 2022
0c922de
Collecting errors instead of throwing on first
abdelr Jan 15, 2022
a219c50
Showing error messages and marks
abdelr Jan 17, 2022
50be51b
Updating error message
abdelr Jan 17, 2022
1421907
Fixing test
abdelr Jan 17, 2022
86eacc7
CalculateFormat does not have side effects
abdelr Jan 28, 2022
d76ef41
Fixing tests
abdelr Jan 28, 2022
2a41306
Using string interpolation
abdelr Feb 4, 2022
aa1c1f8
Changing return type from Cache to a new class
abdelr Feb 4, 2022
4ea57b1
Misc changes
abdelr Feb 4, 2022
cb2c8a3
Fixing test
abdelr Feb 4, 2022
3a58ecb
Merge branch 'develop' into 1409_importer_handling_of_imvalid_files
abdelr Feb 4, 2022
066441e
fixing tests
abdelr Feb 4, 2022
bb32d68
restoring test
abdelr Feb 4, 2022
72834f9
Using dynamic images plus misc fixes
abdelr Feb 7, 2022
f6b6adc
Cleaning resource file
abdelr Feb 7, 2022
70ba716
missing changes
abdelr Feb 7, 2022
dd693a3
Moving logic to presenter
abdelr Feb 7, 2022
566ca79
Proper initialization and resources
abdelr Feb 7, 2022
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
27 changes: 23 additions & 4 deletions src/OSPSuite.Infrastructure.Import/Core/DataSource.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
using OSPSuite.Core.Domain;
using OSPSuite.Core.Import;
using OSPSuite.Infrastructure.Import.Core.Exceptions;
using OSPSuite.Infrastructure.Import.Core.Helpers;
using OSPSuite.Infrastructure.Import.Extensions;
using OSPSuite.Infrastructure.Import.Services;
using OSPSuite.Utility.Collections;
Expand Down Expand Up @@ -58,7 +57,8 @@ public interface IDataSource
Cache<string, IDataSet> DataSets { get; }
Copy link
Member

Choose a reason for hiding this comment

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

I don't like this as a cache exposed. DataSet are added outside of the DataSource as it stands

IEnumerable<string> NamesFromConvention();
NanSettings NanSettings { get; set; }
ImportedDataSet DataSetAt(int index);
ImportedDataSet ImportedDataSetAt(int index);
IDataSet DataSetAt(int index);
ParseErrors ValidateDataSourceUnits(IReadOnlyList<ColumnInfo> columnInfos);
}

Expand Down Expand Up @@ -155,7 +155,26 @@ public IEnumerable<string> NamesFromConvention()
return _importer.NamesFromConvention(_configuration.NamingConventions, _configuration.FileName, DataSets, _mappings);
}

public ImportedDataSet DataSetAt(int index)
public IDataSet DataSetAt(int index)
Copy link
Member

Choose a reason for hiding this comment

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

Let's talk about this in the meeting. I don't understand what this is doing.

{
var sheetIndex = 0;
var sheet = DataSets.GetEnumerator();
var accumulatedIndexes = 0;
while (sheet.MoveNext() && index >= 0)
{
if (sheet.Current.Data.Count() > index)
Copy link
Member

Choose a reason for hiding this comment

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

(sheet.Current.Data.Count() 3 times in the same while loop. Maybe cache it?

{
return sheet.Current;
}

index -= sheet.Current.Data.Count();
sheetIndex++;
accumulatedIndexes += sheet.Current.Data.Count();
}
return null;
}

public ImportedDataSet ImportedDataSetAt(int index)
{
var sheetIndex = 0;
var sheet = DataSets.GetEnumerator();
Expand Down Expand Up @@ -296,7 +315,7 @@ private ParseErrors validateUnitsSupportedAndSameDimension(IReadOnlyList<ColumnI
return errors;
}

ParseErrors IDataSource.ValidateDataSourceUnits(IReadOnlyList<ColumnInfo> columnInfos)
public ParseErrors ValidateDataSourceUnits(IReadOnlyList<ColumnInfo> columnInfos)
{
var errors = validateUnitsSupportedAndSameDimension(columnInfos);
errors.Add(validateErrorAgainstMeasurement(columnInfos));
Expand Down
8 changes: 8 additions & 0 deletions src/OSPSuite.Infrastructure.Import/Core/Exceptions/ParseErrorDescription.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,4 +64,12 @@ public EmptyDataSetsParseErrorDescription(IEnumerable<string> dataSetNames)
Message = Error.EmptyDataSet($"{dataSetNames.ToString(", ")}");
}
}

public class NonMonotonicalTimeParseErrorDescription : ParseErrorDescription
{
public NonMonotonicalTimeParseErrorDescription(string message)
{
Message = message;
}
}
}
24 changes: 0 additions & 24 deletions src/OSPSuite.Infrastructure.Import/Core/Helpers/CachedListHelpers.cs
View file
Open in desktop

This file was deleted.

2 changes: 1 addition & 1 deletion src/OSPSuite.Infrastructure.Import/Services/Importer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ public IReadOnlyList<DataSetToDataRepositoryMappingResult> DataSourceToDataSets(

for (var i = 0; i < dataSource.DataSets.SelectMany(ds => ds.Data).Count(); i++)
{
var dataRepoMapping = _dataRepositoryMapper.ConvertImportDataSet(dataSource.DataSetAt(i));
var dataRepoMapping = _dataRepositoryMapper.ConvertImportDataSet(dataSource.ImportedDataSetAt(i));
var dataRepo = dataRepoMapping.DataRepository;
dataRepo.ConfigurationId = id;
determineMolecularWeight(metaDataCategories, dataImporterSettings, dataRepo);
Expand Down
1 change: 1 addition & 0 deletions src/OSPSuite.Presentation/Presenters/Importer/IImporterDataPresenter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,5 +65,6 @@ IReadOnlyList<ColumnInfo> columnInfos
void GetFormatBasedOnCurrentSheet();
void ResetLoadedSheets();
void SetTabMarks(ParseErrors errors, Cache<string, IDataSet> loadedDataSets);
void SetTabMarks(ParseErrors errors);
}
}
5 changes: 5 additions & 0 deletions src/OSPSuite.Presentation/Presenters/Importer/ImporterDataPresenter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -272,5 +272,10 @@ public void SetTabMarks(ParseErrors errors, Cache<string, IDataSet> loadedDataSe
{
View.SetTabMarks(errors, loadedDataSets);
}

public void SetTabMarks(ParseErrors errors)
{
View.SetTabMarks(errors);
}
}
}
8 changes: 4 additions & 4 deletions src/OSPSuite.Presentation/Presenters/Importer/ImporterPresenter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
using OSPSuite.Utility.Collections;
using OSPSuite.Utility.Extensions;
using ImporterConfiguration = OSPSuite.Core.Import.ImporterConfiguration;
using OSPSuite.Infrastructure.Import.Core.Helpers;

namespace OSPSuite.Presentation.Presenters.Importer
{
Expand Down Expand Up @@ -100,14 +99,15 @@ private void plotDataSet(object sender, DataSetSelectedEventArgs e)
{
try
{
_view.HideExtraErrors();
var dataRepository = _dataRepositoryMapper.ConvertImportDataSet(_dataSource.DataSetAt(e.Index));
var dataRepository = _dataRepositoryMapper.ConvertImportDataSet(_dataSource.ImportedDataSetAt(e.Index));
_confirmationPresenter.PlotDataRepository(dataRepository.DataRepository);
}
catch (InvalidArgumentException invalidException)
{
_view.ShowExtraErrors(Error.ErrorWhenPlottingDataRepository(e.Index, invalidException.Message));
_view.DisableConfirmationView();
var errors = new ParseErrors();
errors.Add(_dataSource.DataSetAt(e.Index), new NonMonotonicalTimeParseErrorDescription(Error.ErrorWhenPlottingDataRepository(e.Index, invalidException.Message)));
_importerDataPresenter.SetTabMarks(errors);
}
}

Expand Down
1 change: 1 addition & 0 deletions src/OSPSuite.Presentation/Views/Importer/IImporterDataView.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,5 +22,6 @@ public interface IImporterDataView : IView<IImporterDataPresenter>
string SelectedTab { get; set; }
string GetFilter();
void SetTabMarks(ParseErrors errors, Cache<string, IDataSet> loadedDataSets);
void SetTabMarks(ParseErrors errors);
}
}
2 changes: 0 additions & 2 deletions src/OSPSuite.Presentation/Views/Importer/IImporterView.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,5 @@ public interface IImporterView : IView<IImporterPresenter>
void EnableConfirmationView();
void DisableConfirmationView();
void AddNanView(INanView nanView);
void ShowExtraErrors(string errorMessage);
void HideExtraErrors();
}
}
3 changes: 0 additions & 3 deletions src/OSPSuite.UI/Properties/licenses.licx
View file
Open in desktop

This file was deleted.

34 changes: 12 additions & 22 deletions src/OSPSuite.UI/Views/Importer/ImporterDataView.Designer.cs
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

19 changes: 13 additions & 6 deletions src/OSPSuite.UI/Views/Importer/ImporterDataView.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,11 @@
using DevExpress.XtraTab.ViewInfo;
using OSPSuite.Assets;
using OSPSuite.Infrastructure.Import.Core;
using OSPSuite.Infrastructure.Import.Core.Exceptions;
using OSPSuite.Presentation.Presenters.Importer;
using OSPSuite.Presentation.Views.Importer;
using OSPSuite.UI.Controls;
using OSPSuite.UI.Extensions;
using OSPSuite.UI.Services;
using OSPSuite.Utility.Collections;
using OSPSuite.Utility.Extensions;

Expand All @@ -23,6 +23,7 @@ namespace OSPSuite.UI.Views.Importer
public partial class ImporterDataView : BaseUserControl, IImporterDataView
{
private IImporterDataPresenter _dataPresenter;
private readonly IImageListRetriever _imageListRetriever;

private string _contextMenuSelectedTab;
private bool sheetImportedFlag;
Expand All @@ -31,7 +32,7 @@ public partial class ImporterDataView : BaseUserControl, IImporterDataView
private ParseErrors _lastErrors = new ParseErrors();
private Cache<string, IDataSet> _lastLoadedDataSets = new Cache<string, IDataSet>();

public ImporterDataView()
public ImporterDataView(IImageListRetriever imageListRetriever)
{
InitializeComponent();
btnImport.Click += (s, a) => OnEvent(onButtonImportClicked, s, a);
Expand All @@ -50,6 +51,7 @@ public ImporterDataView()
btnImport.Text = Captions.Importer.LoadCurrentSheet;
allImportButtonsDisabledFlag = false;
dataViewingGridView.OptionsBehavior.Editable = false;
_imageListRetriever = imageListRetriever;
}

public override void InitializeResources()
Expand Down Expand Up @@ -228,7 +230,7 @@ public string GetActiveFilterCriteria()

public void AddTabs(List<string> sheetNames)
{
importerTabControl.Images = imageCollection1;
importerTabControl.Images = _imageListRetriever.AllImages16x16;
foreach (var sheetName in sheetNames)
importerTabControl.TabPages.Add(sheetName);

Expand Down Expand Up @@ -311,12 +313,12 @@ private void refreshErrorMessage()
if (_lastLoadedDataSets.Contains(SelectedTab) && _lastErrors.Contains(_lastLoadedDataSets[SelectedTab]))
Copy link
Member

Choose a reason for hiding this comment

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

somehow this feels like this code belongs in the presenter.

{
labelControlError.Text = Error.ParseErrorMessage(_lastErrors.ErrorsFor(_lastLoadedDataSets[SelectedTab]).Select(x => x.Message));
layoutControlItem2.Visibility = DevExpress.XtraLayout.Utils.LayoutVisibility.Always;
layoutControlItemError.Visibility = DevExpress.XtraLayout.Utils.LayoutVisibility.Always;
}
else
{
labelControlError.Text = "";
layoutControlItem2.Visibility = DevExpress.XtraLayout.Utils.LayoutVisibility.Never;
layoutControlItemError.Visibility = DevExpress.XtraLayout.Utils.LayoutVisibility.Never;
}
}

Expand All @@ -326,14 +328,19 @@ private void refreshErrorMarks()
{
if (_lastLoadedDataSets.Contains(x.Text))
Copy link
Member

Choose a reason for hiding this comment

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

this codes for which image to show also belongs in the presenter
And hopefully, we won't have those magin number anymore 1 and 0 once we use ApplicationIcons

{
x.ImageIndex = _lastErrors.Contains(_lastLoadedDataSets[x.Text]) ? 1 : 0;
x.ImageIndex = _lastErrors.Contains(_lastLoadedDataSets[x.Text]) ? _imageListRetriever.ImageIndex(ApplicationIcons.Cancel) : _imageListRetriever.ImageIndex(ApplicationIcons.OK);
return;
}

x.ImageIndex = -1;
});
}

public void SetTabMarks(ParseErrors errors)
{
SetTabMarks(errors, _lastLoadedDataSets);
}

public void SetTabMarks(ParseErrors errors, Cache<string, IDataSet> loadedDataSets)
{
_lastErrors = errors;
Expand Down
43 changes: 0 additions & 43 deletions src/OSPSuite.UI/Views/Importer/ImporterDataView.resx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,47 +120,4 @@
<metadata name="errorProvider.TrayLocation" type="System.Drawing.Point, System.Drawing, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a">
<value>17, 17</value>
</metadata>
<data name="useForImportCheckEdit.ToolTip" xml:space="preserve">
<value>When selected, the filter will apply to the data during the import process. When deselected, the filter only affects this view. Check documentation for more information on defining filters: &lt;href=https://docs.open-systems-pharmacology.org/shared-tools-and-example-workflows/features-of-tables#filtering&gt;https://docs.open-systems-pharmacology.org/shared-tools-and-example-workflows/features-of-tables#filtering&lt;/href&gt;</value>
</data>
<metadata name="imageCollection1.TrayLocation" type="System.Drawing.Point, System.Drawing, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a">
<value>163, 17</value>
</metadata>
<assembly alias="DevExpress.Utils.v21.2" name="DevExpress.Utils.v21.2, Version=21.2.3.0, Culture=neutral, PublicKeyToken=b88d1754d700e49a" />
<data name="imageCollection1.ImageStream" type="DevExpress.Utils.ImageCollectionStreamer, DevExpress.Utils.v21.2" mimetype="application/x-microsoft.net.object.bytearray.base64">
<value>
AAEAAAD/////AQAAAAAAAAAMAgAAAFpEZXZFeHByZXNzLlV0aWxzLnYyMS4yLCBWZXJzaW9uPTIxLjIu
My4wLCBDdWx0dXJlPW5ldXRyYWwsIFB1YmxpY0tleVRva2VuPWI4OGQxNzU0ZDcwMGU0OWEMAwAAAFFT
eXN0ZW0uRHJhd2luZywgVmVyc2lvbj00LjAuMC4wLCBDdWx0dXJlPW5ldXRyYWwsIFB1YmxpY0tleVRv
a2VuPWIwM2Y1ZjdmMTFkNTBhM2EFAQAAAChEZXZFeHByZXNzLlV0aWxzLkltYWdlQ29sbGVjdGlvblN0
cmVhbWVyAgAAAAlJbWFnZVNpemUERGF0YQQHE1N5c3RlbS5EcmF3aW5nLlNpemUDAAAAAgIAAAAF/P//
/xNTeXN0ZW0uRHJhd2luZy5TaXplAgAAAAV3aWR0aAZoZWlnaHQAAAgIAwAAABAAAAAQAAAACQUAAAAP
BQAAAJgDAAAC1AEAAIlQTkcNChoKAAAADUlIRFIAAAAQAAAAEAgGAAAAH/P/YQAAAARnQU1BAACxjwv8
YQUAAAAJcEhZcwAAEnQAABJ0Ad5mH3gAAAF2SURBVDhPY8AF0s4Ys8bvcrFI3OPqmbjHQwsoxAiRIQBS
T3l6Jux0+5ew2+0/Bt7p9i1xm4coVCkmiN3l+girRjQcv8N5AlQLAiTscHmCTTE6Dt1mBcSW/2N3u9dC
tTIwhG/xDcKmGB3Pvtr1HwZABnls82AHG5CwC4efkTCyZhCI2m73P3GH202GkN4QTmwakDG65tV35sDl
GOJ2urqBGDE7nf4vvjHlf8Y+P7yatz9cDRR3hcszxOxyywQxNtxdBFXy/3/t8TSsmnc8XPM/Hkkz2IDE
3R42IEbHmVKoMghYd3cBlAUBR57txtAMNqB+vz0LiAGSPPR0B1Q5Kjj36jhWzSAMiYXdzn9AHGyGnH8N
1LwLu+bYPY57wAbE73DTgAmCDDn8dCdY8633l4GaXTA0wjDDf6T8kXrMcwuyZMQOW5zOBuGM7d62UK0I
AHTJYmyK0XHwZm8HqBZMEL/fXiB2l/MrbBpjtzser69nYIIqBQIGBgDCdSCqO2LSiQAAAABJRU5ErkJg
grwBAACJUE5HDQoaCgAAAA1JSERSAAAAEAAAABAIBgAAAB/z/2EAAAAEZ0FNQQAAsY8L/GEFAAAACXBI
WXMAABJ0AAASdAHeZh94AAABXklEQVQ4T4WSPUvDQBzG//UNJ21FQWku11gxyaWhg7i4OPsB3MUvorsf
wMEPIHRy9u2aXFpBEFxcBRE7OYiTKFJ7b0lzifqDB47nnucf7i7wG51dmGSevcF8vMOCRvsAYEJt/U0v
WN6MCP5mBA9NRR76vFpDTRUt0ms5d2VFU/ct50xVMm5I41YEfHsYe6i0mPN9+1RVAfqhE+oNDUULWVgI
Cf9jMEi9l/WVRTGAeehLmKOvj0MtPUSWNXpA7FtvMASoaIOL1udVTNJdXVIrCbWqaZYLkgA1xw0uc4jG
LHNB5NpbpsnVRTVVk1xWpwsZLui3rXpxI39mTXYnmeQl5sx8+XyuolYS83XEgNGf9y6MwivIM5t3ossJ
wY9iAAvtmjZfry9EyLwwPeT55Dj1OoTMiAGcmDhH44V/5eN9Vc3oueiwNGwoIWhPVYpQjGcjFz2VFZmL
Hug2TKnoCIAfQmzsw53qs4QAAAAASUVORK5CYIIL
</value>
</data>
<metadata name="dpiAwareImageCollection1.TrayLocation" type="System.Drawing.Point, System.Drawing, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a">
<value>338, 17</value>
</metadata>
<data name="dpiAwareImageCollection1.Stream" type="DevExpress.Utils.DPIAwareImageCollectionStreamer, DevExpress.Utils.v21.2" mimetype="application/x-microsoft.net.object.bytearray.base64">
<value>
AAEAAAD/////AQAAAAAAAAAMAgAAAFpEZXZFeHByZXNzLlV0aWxzLnYyMS4yLCBWZXJzaW9uPTIxLjIu
My4wLCBDdWx0dXJlPW5ldXRyYWwsIFB1YmxpY0tleVRva2VuPWI4OGQxNzU0ZDcwMGU0OWEFAQAAADBE
ZXZFeHByZXNzLlV0aWxzLkRQSUF3YXJlSW1hZ2VDb2xsZWN0aW9uU3RyZWFtZXIAAAAAAgAAAAs=
</value>
</data>
</root>
Loading