Fixes #2834 Separator Selector is styled differently and pops-under pk-sim (#2835)

* Fixes  #2834 Separator Selector is styled differently and pops-under pk-sim

* pr feedback

Author: rwmcintosh

src/OSPSuite.Infrastructure.Import/Core/DataSourceFile.cs

@@ -12,7 +12,7 @@ namespace OSPSuite.Infrastructure.Import.Core
    /// </summary>
    public interface IDataSourceFile
    {
-      string Path { get; set; }
+      string Path { get; }
       IDataFormat Format { get; set; }
 
       IList<IDataFormat> AvailableFormats { get; set; }
@@ -22,6 +22,11 @@ public interface IDataSourceFile
       //as active when initialized
       string FormatCalculatedFrom { get; set; }
       DataSheetCollection DataSheets { get; }
+
+      /// <summary>
+      ///    Loads the file at <paramref name="path" /> into the data source.
+      /// </summary>
+      void LoadFromFile(string path);
    }
 
    public abstract class DataSourceFile : IDataSourceFile
@@ -52,29 +57,31 @@ protected DataSourceFile(IImportLogger logger, IHeavyWorkManager heavyWorkManage
          _heavyWorkManager = heavyWorkManager;
       }
 
-      private string _path;
+      public string Path { get; private set; }
+
+      protected abstract void DoLoadWork(string path, CancellationToken cancellationToken = default);
 
-      public string Path
+      public virtual void LoadFromFile(string path)
       {
-         get => _path;
-         set
+         
+         var cts = new CancellationTokenSource();
+         if(heavyWorkSucceeds(path, cts))
+            Path = path;
+      }
+
+      private bool heavyWorkSucceeds(string path, CancellationTokenSource cts)
+      {
+         return _heavyWorkManager.Start(() =>
          {
-            _path = value;
-            var cts = new CancellationTokenSource();
-            _heavyWorkManager.Start(() =>
+            try
             {
-               try
-               {
-                  LoadFromFile(_path, cts.Token);
-               }
-               catch (OperationCanceledException)
-               {
-                  //Nothing to do, just not throw exception.
-               }
-            }, "Importing data...", cts);
-         }
+               DoLoadWork(path, cts.Token);
+            }
+            catch (OperationCanceledException)
+            {
+               //Nothing to do, just not throw exception.
+            }
+         }, "Importing data...", cts);
       }
-
-      protected abstract void LoadFromFile(string path, CancellationToken cancellationToken = default);
    }
 }

src/OSPSuite.Infrastructure.Import/Core/DataSourceFileParser.cs

@@ -28,13 +28,13 @@ public IDataSourceFile For(string path)
          var lowerCasePath = path.ToLower();
          if (_csvExtensions.Any(lowerCasePath.EndsWith))
          {
-            _csvDataSourceFile.Path = path;
+            _csvDataSourceFile.LoadFromFile(path);
             return _csvDataSourceFile;
          }
 
          if (_excelExtensions.Any(lowerCasePath.EndsWith))
          {
-            _excelDataSourceFile.Path = path;
+            _excelDataSourceFile.LoadFromFile(path);
             return _excelDataSourceFile;
          }
 

src/OSPSuite.Infrastructure.Import/Core/DataSourceFileReaders/CsvDataSourceFile.cs

@@ -14,27 +14,33 @@ public interface ICsvDataSourceFile : IDataSourceFile
    public class CsvDataSourceFile : DataSourceFile, ICsvDataSourceFile
    {
       private readonly ICsvSeparatorSelector _csvSeparatorSelector;
+      private CSVSeparators _csvSeparators;
 
       public CsvDataSourceFile(IImportLogger logger, ICsvSeparatorSelector csvSeparatorSelector, IHeavyWorkManager heavyWorkManager) : base(logger, heavyWorkManager)
       {
          _csvSeparatorSelector = csvSeparatorSelector;
       }
 
-      protected override void LoadFromFile(string path, CancellationToken c)
+      public override void LoadFromFile(string path)
       {
-         var csvSeparators = _csvSeparatorSelector.GetCsvSeparator(path);
+         _csvSeparators = _csvSeparatorSelector.GetCsvSeparator(path);
 
          //if separator selection dialog was cancelled, abort
-         if (csvSeparators == null)
+         if (_csvSeparators == null)
             return;
 
+         base.LoadFromFile(path);
+      }
+      
+      protected override void DoLoadWork(string path, CancellationToken cancellationToken = default)
+      {
          //we keep a copy of the already loaded sheets, in case the reading fails
          var alreadyLoadedDataSheets = DataSheets.Clone();
          DataSheets.Clear();
 
          try
          {
-            using (var reader = new CsvReaderFromFile(path, csvSeparators.ColumnSeparator))
+            using (var reader = new CsvReaderFromFile(path, _csvSeparators.ColumnSeparator))
             {
                var csv = reader.Csv;
                var headers = csv.GetFieldHeaders();
@@ -52,7 +58,7 @@ protected override void LoadFromFile(string path, CancellationToken c)
                {
                   csv.CopyCurrentRecordTo(currentRow);
                   var currentCultureDecimalSeparator = Convert.ToChar(Thread.CurrentThread.CurrentCulture.NumberFormat.NumberDecimalSeparator);
-                  var rowList = currentRow.Select(x => x.Replace(csvSeparators.DecimalSeparator, currentCultureDecimalSeparator)).ToList();
+                  var rowList = currentRow.Select(x => x.Replace(_csvSeparators.DecimalSeparator, currentCultureDecimalSeparator)).ToList();
                   var levels = getMeasurementLevels(rowList);
                   dataSheet.CalculateColumnDescription(levels);
                   dataSheet.AddRow(rowList);

src/OSPSuite.Infrastructure.Import/Core/DataSourceFileReaders/ExcelDataSourceFile.cs

@@ -18,7 +18,7 @@ public ExcelDataSourceFile(IImportLogger logger, IHeavyWorkManager heavyWorkMana
       {
       }
 
-      protected override void LoadFromFile(string path, CancellationToken cancellationToken = default)
+      protected override void DoLoadWork(string path, CancellationToken cancellationToken = default)
       {
          //we keep a copy of the already loaded sheets, in case the reading fails
          var alreadyLoadedDataSheets = DataSheets.Clone();

src/OSPSuite.Presentation/Presenters/Importer/ImporterDataPresenter.cs

@@ -216,7 +216,7 @@ public void RemoveTab(string tabName)
 
       public void ReopenAllSheets()
       {
-         _dataSourceFile.Path = _dataSourceFile.Path;
+         _dataSourceFile.LoadFromFile(_dataSourceFile.Path);
          RefreshTabs();
       }
 

src/OSPSuite.UI/Views/Importer/CsvSeparatorSelectorView.cs

@@ -3,6 +3,7 @@
 using DevExpress.XtraLayout.Utils;
 using OSPSuite.Presentation.Extensions;
 using OSPSuite.Presentation.Presenters.Importer;
+using OSPSuite.Presentation.Views;
 using OSPSuite.Presentation.Views.Importer;
 using static OSPSuite.Assets.Captions.Importer;
 
@@ -16,7 +17,12 @@ public partial class CSVSeparatorSelectorView : BaseModalView, ICSVSeparatorSele
       private readonly List<char> _columnSeparatorList = new List<char> { Comma, ' ', ';', Period };
       private readonly List<char> _decimalSeparatorList = new List<char> { Period, Comma };
 
-      public CSVSeparatorSelectorView()
+      //only for design time
+      public CSVSeparatorSelectorView() : this(null)
+      {
+      }
+
+      public CSVSeparatorSelectorView(IShell shell) : base(shell)
       {
          InitializeComponent();
          fillSeparatorComboBox();

tests/OSPSuite.Presentation.Tests/Importer/Core/DataSourceFileReaders/CsvDataSourceFileSpecs.cs

@@ -42,7 +42,7 @@ protected override void Context()
 
       protected override void Because()
       {
-         sut.Path = _csvFilePath;
+         sut.LoadFromFile(_csvFilePath);
       }
 
       [TestCase]
@@ -123,7 +123,7 @@ protected override void Context()
       [Observation]
       public void duplicate_header_file_throws_exception()
       {
-         Assert.Throws<InvalidObservedDataFileException>(() => sut.Path = _csvFilePath);
+         Assert.Throws<InvalidObservedDataFileException>(() => sut.LoadFromFile(_csvFilePath));
       }
    }
 }

tests/OSPSuite.Presentation.Tests/Importer/Core/DataSourceFileReaders/ExcelDataSourceFileSpecs.cs

@@ -25,10 +25,8 @@ protected override void Context()
       {
          workManager = new HeavyWorkManagerForSpecs();
 
-         sut = new ExcelDataSourceFile(A.Fake<IImportLogger>(), workManager)
-         {
-            Path = _excelFilePath
-         };
+         sut = new ExcelDataSourceFile(A.Fake<IImportLogger>(), workManager);
+         sut.LoadFromFile(_excelFilePath);
       }
 
       public override void GlobalContext()
@@ -43,7 +41,7 @@ public class When_reading_excel : ConcernForExcelDataSourceFile
       [TestCase]
       public void headers_are_adjusted_on_empty_columns()
       {
-         sut.Path = Path.Combine(AppDomain.CurrentDomain.BaseDirectory, "Data", "sample2.xlsx");
+         sut.LoadFromFile(Path.Combine(AppDomain.CurrentDomain.BaseDirectory, "Data", "sample2.xlsx"));
          var columns = sut.DataSheets.ElementAt(0).GetHeaders();
          columns.Count().ShouldBeEqualTo(4);
          for (var i = 0; i < 4; i++)

tests/OSPSuite.Presentation.Tests/Importer/Presenters/ImporterPresenterSpecs.cs

@@ -238,7 +238,7 @@ protected override void Context()
 
          _dataSourceFile = new ExcelDataSourceFile(A.Fake<IImportLogger>(), new HeavyWorkManagerForSpecs());
          _dataSourceFile.Format = A.Fake<IDataFormat>();
-         _dataSourceFile.Path = Path.Combine(AppDomain.CurrentDomain.BaseDirectory, "Data", "IntegrationSampleUnitFromColumn.xlsx");
+         _dataSourceFile.LoadFromFile(Path.Combine(AppDomain.CurrentDomain.BaseDirectory, "Data", "IntegrationSampleUnitFromColumn.xlsx"));
          A.CallTo(() => _importerDataPresenter.SetDataSource(A<string>.Ignored)).Returns(_dataSourceFile);
          _importerDataPresenter.OnImportSheets += Raise.With(new ImportSheetsEventArgs()
             { Filter = "", DataSourceFile = _dataSourceFile, SheetNames = _sheets.Keys.ToList() });