From 2e93e4b5023b837a94fc5315233868f9b390cfce Mon Sep 17 00:00:00 2001
From: Matthias <github@mpuchner.de>
Date: Tue, 23 Feb 2021 08:41:16 +0100
Subject: [PATCH] simplify realdata/instrument linking; simplify notifications;
 adopt unit tests accordingly

---
 GUI/coregui/Models/ApplicationModels.cpp      |   2 +
 GUI/coregui/Models/InstrumentModel.cpp        |   5 +
 GUI/coregui/Models/InstrumentModel.h          |   1 +
 GUI/coregui/Models/JobModel.cpp               |  18 +--
 GUI/coregui/Models/JobModel.h                 |   6 +-
 GUI/coregui/Models/JobModelFunctions.cpp      |   5 +-
 GUI/coregui/Models/RealDataItem.cpp           |  72 ++++++++----
 GUI/coregui/Models/RealDataItem.h             |  15 ++-
 GUI/coregui/Models/RealDataModel.cpp          |  22 +++-
 GUI/coregui/Models/RealDataModel.h            |  10 +-
 .../LinkInstrumentManager.cpp                 | 106 +++++-------------
 .../ImportDataWidgets/LinkInstrumentManager.h |  12 +-
 .../RealDataPropertiesWidget.cpp              |   5 +-
 .../SpecularDataImportWidget.cpp              |  10 +-
 GUI/coregui/mainwindow/projectdocument.cpp    |   1 -
 Tests/UnitTests/GUI/TestLinkInstrument.cpp    |   2 +-
 16 files changed, 167 insertions(+), 125 deletions(-)

diff --git a/GUI/coregui/Models/ApplicationModels.cpp b/GUI/coregui/Models/ApplicationModels.cpp
index f8e682c7e53..2b879512f90 100644
--- a/GUI/coregui/Models/ApplicationModels.cpp
+++ b/GUI/coregui/Models/ApplicationModels.cpp
@@ -53,6 +53,8 @@ ApplicationModels::ApplicationModels(QObject* parent)
     m_realDataModel = new RealDataModel(this);
     m_jobModel = new JobModel(this);
 
+    m_realDataModel->setInstrumentModel(m_instrumentModel);
+
     connectModel(m_documentModel);
     connectModel(m_materialModel);
     connectModel(m_sampleModel);
diff --git a/GUI/coregui/Models/InstrumentModel.cpp b/GUI/coregui/Models/InstrumentModel.cpp
index 13ae0eff9b0..34368ff1f2d 100644
--- a/GUI/coregui/Models/InstrumentModel.cpp
+++ b/GUI/coregui/Models/InstrumentModel.cpp
@@ -86,6 +86,11 @@ InstrumentItem* InstrumentModel::findInstrumentById(const QString& instrumentId)
     return nullptr;
 }
 
+bool InstrumentModel::instrumentExists(const QString& instrumentId) const
+{
+    return findInstrumentById(instrumentId) != nullptr;
+}
+
 void InstrumentModel::onRowsChange(const QModelIndex& parent, int, int)
 {
     // valid parent means not an instrument (which is top level item) but something below
diff --git a/GUI/coregui/Models/InstrumentModel.h b/GUI/coregui/Models/InstrumentModel.h
index c319797cc84..f3a193631aa 100644
--- a/GUI/coregui/Models/InstrumentModel.h
+++ b/GUI/coregui/Models/InstrumentModel.h
@@ -33,6 +33,7 @@ public:
     QVector<InstrumentItem*> instrumentItems() const;
 
     InstrumentItem* findInstrumentById(const QString& instrumentId) const;
+    bool instrumentExists(const QString& instrumentId) const;
 
 signals:
     void instrumentAddedOrRemoved();
diff --git a/GUI/coregui/Models/JobModel.cpp b/GUI/coregui/Models/JobModel.cpp
index ccf6f5dbca1..69c6b953cbd 100644
--- a/GUI/coregui/Models/JobModel.cpp
+++ b/GUI/coregui/Models/JobModel.cpp
@@ -102,6 +102,11 @@ JobItem* JobModel::addJob(const MultiLayerItem* multiLayerItem,
     return jobItem;
 }
 
+QVector<JobItem*> JobModel::jobItems() const
+{
+    return topItems<JobItem>();
+}
+
 //! restore instrument and sample model from backup for given JobItem
 void JobModel::restore(JobItem* jobItem)
 {
@@ -153,15 +158,14 @@ QVector<SessionItem*> JobModel::nonXMLItems() const
     return result;
 }
 
-//! Link instruments to real data on project load.
-
-void JobModel::link_instruments()
+void JobModel::readFrom(QXmlStreamReader* reader, MessageService* messageService /*= 0*/)
 {
-    for (int i = 0; i < rowCount(QModelIndex()); ++i) {
-        JobItem* jobItem = getJobItemForIndex(index(i, 0, QModelIndex()));
+    SessionModel::readFrom(reader, messageService);
+
+    // Check instrument links. JobItem and realDataItem have to reference the same instrument.
+    for (auto jobItem : jobItems())
         if (RealDataItem* refItem = jobItem->realDataItem())
-            refItem->linkToInstrument(jobItem->instrumentItem(), false);
-    }
+            ASSERT(refItem->instrumentId() == jobItem->instrumentItem()->id());
 }
 
 void JobModel::onCancelAllJobs()
diff --git a/GUI/coregui/Models/JobModel.h b/GUI/coregui/Models/JobModel.h
index f7b5d9d7188..e03b282e570 100644
--- a/GUI/coregui/Models/JobModel.h
+++ b/GUI/coregui/Models/JobModel.h
@@ -37,15 +37,15 @@ public:
     JobItem* addJob(const MultiLayerItem* multiLayerItem, const InstrumentItem* instrumentItem,
                     const RealDataItem* realDataItem, const SimulationOptionsItem* optionItem);
 
+    QVector<JobItem*> jobItems() const;
+
     void restore(JobItem* jobItem);
 
     bool hasUnfinishedJobs();
 
     void clear() override;
-
     QVector<SessionItem*> nonXMLItems() const override;
-
-    void link_instruments();
+    virtual void readFrom(QXmlStreamReader* reader, MessageService* messageService = 0) override;
 
 signals:
     void aboutToDeleteJobItem(JobItem* item);
diff --git a/GUI/coregui/Models/JobModelFunctions.cpp b/GUI/coregui/Models/JobModelFunctions.cpp
index 79bfd60d1ca..26be014f4ce 100644
--- a/GUI/coregui/Models/JobModelFunctions.cpp
+++ b/GUI/coregui/Models/JobModelFunctions.cpp
@@ -202,6 +202,8 @@ void JobModelFunctions::copyRealDataItem(JobItem* jobItem, const RealDataItem* r
     // adapting the name to job name
     realDataItemCopy->dataItem()->setFileName(ItemFileNameUtils::jobReferenceFileName(*jobItem));
 
+    // #baimport ++ copy members of realDataItem
+
     if (!realDataItem->nativeData())
         return;
 
@@ -224,7 +226,8 @@ void processInstrumentLink(JobItem* jobItem)
     if (!realData)
         throw GUIHelpers::Error("JobModelFunctions::processInstrumentLink() -> Error. No data.");
 
-    realData->linkToInstrument(jobItem->instrumentItem());
+    realData->setInstrumentId(jobItem->instrumentItem()->id());
+    realData->updateToInstrument(jobItem->instrumentItem());
 }
 
 void copyMasksToInstrument(JobItem* jobItem)
diff --git a/GUI/coregui/Models/RealDataItem.cpp b/GUI/coregui/Models/RealDataItem.cpp
index 3c95a9d767d..ac6a9daddf5 100644
--- a/GUI/coregui/Models/RealDataItem.cpp
+++ b/GUI/coregui/Models/RealDataItem.cpp
@@ -14,9 +14,11 @@
 
 #include "GUI/coregui/Models/RealDataItem.h"
 #include "GUI/coregui/Models/InstrumentItems.h"
+#include "GUI/coregui/Models/InstrumentModel.h"
 #include "GUI/coregui/Models/IntensityDataItem.h"
 #include "GUI/coregui/Models/ItemFileNameUtils.h"
 #include "GUI/coregui/Models/JobItemUtils.h"
+#include "GUI/coregui/Models/RealDataModel.h"
 #include "GUI/coregui/Models/SessionModel.h"
 #include "GUI/coregui/Models/SpecularDataItem.h"
 #include "GUI/coregui/utils/GUIHelpers.h"
@@ -37,7 +39,7 @@ const QString ImportSettings("ImportSettings");
 const QString Value("Value");
 } // namespace XMLTags
 
-RealDataItem::RealDataItem() : SessionItem("RealData"), m_linkedInstrument(nullptr)
+RealDataItem::RealDataItem() : SessionItem("RealData")
 {
     setItemName("undefined");
 
@@ -48,7 +50,7 @@ RealDataItem::RealDataItem() : SessionItem("RealData"), m_linkedInstrument(nullp
     setDefaultTag(T_INTENSITY_DATA);
 
     addProperty(P_INSTRUMENT_ID, QString());
-    addProperty(P_INSTRUMENT_NAME, QString()); // #migration This is never used - return after
+    addProperty(P_INSTRUMENT_NAME, QString()); // #migration This is never used - remove after
                                                // checking whether this breaks loading old files
 
     registerTag(T_NATIVE_DATA, 1, 1,
@@ -68,11 +70,11 @@ RealDataItem::RealDataItem() : SessionItem("RealData"), m_linkedInstrument(nullp
 
     mapper()->setOnChildPropertyChange([this](SessionItem* item, const QString& name) {
         auto data_item = dynamic_cast<DataItem*>(item);
-        if (!data_item || !m_linkedInstrument || name != DataItem::P_AXES_UNITS)
+        if (!data_item || !linkedInstrument() || name != DataItem::P_AXES_UNITS)
             return;
 
         mapper()->setActive(false);
-        data_item->updateAxesUnits(m_linkedInstrument);
+        data_item->updateAxesUnits(linkedInstrument());
         mapper()->setActive(true);
     });
 }
@@ -143,6 +145,11 @@ void RealDataItem::setNativeDataUnits(const QString& units)
     getItem(P_NATIVE_DATA_UNITS)->setValue(units);
 }
 
+bool RealDataItem::hasNativeData() const
+{
+    return (nativeData() != nullptr) && (nativeData()->getOutputData() != nullptr);
+}
+
 //! Creates data item if not existing so far. Checks for rank compatibility if already existing. No
 //! further initialization like clearing the data etc.
 
@@ -197,7 +204,7 @@ void RealDataItem::setImportData(ImportDataInfo data)
     auto output_data = data.intensityData();
 
     dataItem()->reset(std::move(data));
-    getItem(P_NATIVE_DATA_UNITS)->setValue(units_name);
+    setNativeDataUnits(units_name);
     item<DataItem>(T_NATIVE_DATA)->setOutputData(output_data.release());
 }
 
@@ -218,13 +225,6 @@ bool RealDataItem::holdsDimensionalData() const
     return nativeDataUnits() != "nbins";
 }
 
-void RealDataItem::linkToInstrument(const InstrumentItem* instrument, bool make_update)
-{
-    m_linkedInstrument = instrument;
-    if (make_update)
-        updateToInstrument();
-}
-
 QString RealDataItem::instrumentId() const
 {
     return getItemValue(P_INSTRUMENT_ID).toString();
@@ -241,6 +241,12 @@ void RealDataItem::clearInstrumentId()
     // #baimport ++ should m_linkedInstrument be set to null?
 }
 
+InstrumentItem* RealDataItem::linkedInstrument() const
+{
+    return instrumentModel() != nullptr ? instrumentModel()->findInstrumentById(instrumentId())
+                                        : nullptr;
+}
+
 std::vector<int> RealDataItem::shape() const
 {
     auto data_item = dataItem();
@@ -331,21 +337,47 @@ void RealDataItem::updateNonXMLDataFileNames()
         item->setFileName(ItemFileNameUtils::nativeDataFileName(*this));
 }
 
-void RealDataItem::updateToInstrument()
+RealDataModel* RealDataItem::realDataModel() const
+{
+    return dynamic_cast<RealDataModel*>(model());
+}
+
+InstrumentModel* RealDataItem::instrumentModel() const
+{
+    return realDataModel() != nullptr ? realDataModel()->instrumentModel() : nullptr;
+}
+
+void RealDataItem::updateToInstrument(const InstrumentItem* instrument)
 {
     DataItem* data_item = dataItem();
     if (!data_item)
         return;
 
-    if (m_linkedInstrument) {
-        JobItemUtils::setIntensityItemAxesUnits(data_item, m_linkedInstrument);
+    if (instrument) {
+        JobItemUtils::setIntensityItemAxesUnits(data_item, instrument);
         return;
     }
 
-    auto native_data_item = nativeData();
-    auto data_source = native_data_item ? native_data_item : data_item;
+    // unlinking => going back to native data
+    if (isSpecularData()) {
+        if (hasNativeData()) {
+            std::unique_ptr<OutputData<double>> native_data(nativeData()->getOutputData()->clone());
+            const QString units_label = nativeDataUnits();
+            data_item->reset(ImportDataInfo(std::move(native_data), units_label));
+        } else {
+            specularDataItem()->setOutputData(nullptr);
+        }
+    } else {
+        auto native_data_item = nativeData();
+        auto data_source = native_data_item ? native_data_item : data_item;
+
+        std::unique_ptr<OutputData<double>> native_data(data_source->getOutputData()->clone());
+        const QString units_label = nativeDataUnits();
+        data_item->reset(ImportDataInfo(std::move(native_data), units_label));
+    }
+}
 
-    std::unique_ptr<OutputData<double>> native_data(data_source->getOutputData()->clone());
-    const QString units_label = nativeDataUnits();
-    data_item->reset(ImportDataInfo(std::move(native_data), units_label));
+void RealDataItem::updateToInstrument(const QString& id)
+{
+    updateToInstrument(instrumentModel()->findInstrumentById(id));
 }
diff --git a/GUI/coregui/Models/RealDataItem.h b/GUI/coregui/Models/RealDataItem.h
index 1b5dbded1bb..09e1f2086c0 100644
--- a/GUI/coregui/Models/RealDataItem.h
+++ b/GUI/coregui/Models/RealDataItem.h
@@ -16,14 +16,17 @@
 #define BORNAGAIN_GUI_COREGUI_MODELS_REALDATAITEM_H
 
 #include "GUI/coregui/Models/SessionItem.h"
+#include <QPointer>
 
 class DataItem;
 class InstrumentItem;
+class InstrumentModel;
 class IntensityDataItem;
 class SpecularDataItem;
 class MaskContainerItem;
 template <class T> class OutputData;
 class ImportDataInfo;
+class RealDataModel;
 
 //! The RealDataItem class represents intensity data imported from file and intended for fitting.
 
@@ -64,16 +67,18 @@ public:
     void initNativeData();
     QString nativeDataUnits() const;
     void setNativeDataUnits(const QString& units);
+    bool hasNativeData() const;   
 
     void setOutputData(OutputData<double>* data);
     void setImportData(ImportDataInfo data);
     bool holdsDimensionalData() const;
 
-    void linkToInstrument(const InstrumentItem* instrument, bool make_update = true);
-
+    void updateToInstrument(const InstrumentItem* instrument);
+    void updateToInstrument(const QString& id);
     QString instrumentId() const;
     void setInstrumentId(const QString& id);
     void clearInstrumentId();
+    InstrumentItem* linkedInstrument() const;
 
     //! Returns the shape of underlying data item
     std::vector<int> shape() const;
@@ -98,8 +103,10 @@ public:
 private:
     void initDataItem(size_t data_rank, const QString& tag);
     void updateNonXMLDataFileNames();
-    void updateToInstrument();
-    const InstrumentItem* m_linkedInstrument;
+
+    RealDataModel* realDataModel() const;
+    InstrumentModel* instrumentModel() const;
+
     QByteArray m_importSettings;
     QString m_nativeFileName;
 };
diff --git a/GUI/coregui/Models/RealDataModel.cpp b/GUI/coregui/Models/RealDataModel.cpp
index 77c8f8bc2dd..e5a3d88acb4 100644
--- a/GUI/coregui/Models/RealDataModel.cpp
+++ b/GUI/coregui/Models/RealDataModel.cpp
@@ -16,7 +16,8 @@
 #include "GUI/coregui/Models/DataItem.h"
 #include "GUI/coregui/Models/RealDataItem.h"
 
-RealDataModel::RealDataModel(QObject* parent) : SessionModel(SessionXML::RealDataModelTag, parent)
+RealDataModel::RealDataModel(QObject* parent)
+    : SessionModel(SessionXML::RealDataModelTag, parent), m_instrumentModel(nullptr)
 {
     setObjectName(SessionXML::RealDataModelTag);
 
@@ -61,10 +62,19 @@ bool RealDataModel::setData(const QModelIndex& index, const QVariant& value, int
 
 void RealDataModel::readFrom(QXmlStreamReader* reader, MessageService* messageService /*= 0*/)
 {
+    // do not send added-notifications until completely read - otherwise partially
+    // initialized items will be notified
+    disconnect(this, &SessionModel::rowsInserted, this, &RealDataModel::onRowsChange);
+
     SessionModel::readFrom(reader, messageService);
 
+    connect(this, &SessionModel::rowsInserted, this, &RealDataModel::onRowsChange);
+
     // #baimport If there are realDataItems with no loader => old version. Either create loader, or
     // refuse loading with "project too old
+
+    if (!realDataItems().isEmpty())
+        emit realDataAddedOrRemoved();
 }
 
 RealDataItem* RealDataModel::insertRealDataItem()
@@ -91,6 +101,16 @@ QVector<RealDataItem*> RealDataModel::realDataItems() const
     return topItems<RealDataItem>();
 }
 
+InstrumentModel* RealDataModel::instrumentModel() const
+{
+    return m_instrumentModel;
+}
+
+void RealDataModel::setInstrumentModel(InstrumentModel* instrumentModel)
+{
+    m_instrumentModel = instrumentModel;
+}
+
 void RealDataModel::onRowsChange(const QModelIndex& parent, int, int)
 {
     // valid parent means not a data (which is top level item) but something below
diff --git a/GUI/coregui/Models/RealDataModel.h b/GUI/coregui/Models/RealDataModel.h
index 7a8511ba5f3..d7e9661dc19 100644
--- a/GUI/coregui/Models/RealDataModel.h
+++ b/GUI/coregui/Models/RealDataModel.h
@@ -15,9 +15,11 @@
 #ifndef BORNAGAIN_GUI_COREGUI_MODELS_REALDATAMODEL_H
 #define BORNAGAIN_GUI_COREGUI_MODELS_REALDATAMODEL_H
 
-#include "GUI/coregui/Models/SessionModel.h"
+#include "GUI/coregui/Models/InstrumentModel.h"
+#include <QPointer>
 
 class RealDataItem;
+class InstrumentModel;
 
 //! The RealDataModel class is a model to store all imported RealDataItem's.
 
@@ -26,6 +28,7 @@ class RealDataModel : public SessionModel {
 
 public:
     explicit RealDataModel(QObject* parent = 0);
+    void setInstrumentModel(InstrumentModel* instrumentModel);
 
     virtual QVector<SessionItem*> nonXMLItems() const override;
     virtual Qt::ItemFlags flags(const QModelIndex& index) const override;
@@ -37,11 +40,16 @@ public:
     RealDataItem* insertIntensityDataItem();
     QVector<RealDataItem*> realDataItems() const;
 
+    InstrumentModel* instrumentModel() const;
+
 signals:
     void realDataAddedOrRemoved();
 
 private:
     void onRowsChange(const QModelIndex& parent, int, int);
+
+private:
+    QPointer<InstrumentModel> m_instrumentModel;
 };
 
 #endif // BORNAGAIN_GUI_COREGUI_MODELS_REALDATAMODEL_H
diff --git a/GUI/coregui/Views/ImportDataWidgets/LinkInstrumentManager.cpp b/GUI/coregui/Views/ImportDataWidgets/LinkInstrumentManager.cpp
index 8032108d5e3..433fce76deb 100644
--- a/GUI/coregui/Views/ImportDataWidgets/LinkInstrumentManager.cpp
+++ b/GUI/coregui/Views/ImportDataWidgets/LinkInstrumentManager.cpp
@@ -14,6 +14,7 @@
 
 #include "GUI/coregui/Views/ImportDataWidgets/LinkInstrumentManager.h"
 #include "GUI/coregui/Models/AxesItems.h"
+#include "GUI/coregui/Models/DataItem.h"
 #include "GUI/coregui/Models/DetectorItems.h"
 #include "GUI/coregui/Models/InstrumentItems.h"
 #include "GUI/coregui/Models/InstrumentModel.h"
@@ -21,6 +22,7 @@
 #include "GUI/coregui/Models/RealDataItem.h"
 #include "GUI/coregui/Models/RealDataModel.h"
 #include "GUI/coregui/Views/ImportDataWidgets/ImportDataUtils.h"
+#include "GUI/coregui/mainwindow/mainwindow.h"
 #include <QMessageBox>
 #include <QPushButton>
 
@@ -53,19 +55,15 @@ void LinkInstrumentManager::setModels(InstrumentModel* instrumentModel,
     connect(m_instrumentModel, &InstrumentModel::instrumentAddedOrRemoved, this,
             &LinkInstrumentManager::onInstrumentAddedOrRemoved);
 
-    connect(m_realDataModel, &RealDataModel::realDataAddedOrRemoved, this,
-            &LinkInstrumentManager::onRealDataAddedOrRemoved);
-
     updateInstrumentSubscriptions();
-    updateRealDataSubscriptions();
-    updateLinks();
 }
 
 //! Returns true if RealDataItem can be linked to the instrument (same number of bins).
 //! Also offers dialog to adjust instrument to match shape of real data.
 
 bool LinkInstrumentManager::canLinkDataToInstrument(const RealDataItem* realDataItem,
-                                                    const QString& identifier)
+                                                    const QString& identifier,
+                                                    bool quiet /*= false*/)
 {
     auto instrumentItem = m_instrumentModel->findInstrumentById(identifier);
 
@@ -73,10 +71,18 @@ bool LinkInstrumentManager::canLinkDataToInstrument(const RealDataItem* realData
     if (!instrumentItem)
         return true;
 
-    // #baTODO: Fix modality!
-    if (!ImportDataUtils::Compatible(*instrumentItem, *realDataItem)) {
-        QMessageBox::warning(nullptr, "Can't link to instrument",
-                             "Can't link, data is incompatible with the instrument.");
+    const bool isCompatible = ImportDataUtils::Compatible(*instrumentItem, *realDataItem);
+    if (!isCompatible) {
+        if (!quiet)
+            QMessageBox::warning(MainWindow::instance(), "Can't link to instrument",
+                                 "Can't link, data is incompatible with the instrument.");
+        return false;
+    }
+
+    if (realDataItem->isSpecularData() && !realDataItem->hasNativeData()) {
+        if (!quiet)
+            QMessageBox::warning(MainWindow::instance(), "Can't link to instrument",
+                                 "Can't link, data is empty.");
         return false;
     }
 
@@ -94,18 +100,6 @@ bool LinkInstrumentManager::canLinkDataToInstrument(const RealDataItem* realData
     return true;
 }
 
-//! Link or re-link RealDataItem to the instrument on identifier change.
-
-void LinkInstrumentManager::setOnRealDataPropertyChange(SessionItem* dataItem,
-                                                        const QString& property)
-{
-    if (property == RealDataItem::P_INSTRUMENT_ID) {
-        RealDataItem* realDataItem = dynamic_cast<RealDataItem*>(dataItem);
-        const QString identifier = realDataItem->instrumentId();
-        realDataItem->linkToInstrument(m_instrumentModel->findInstrumentById(identifier));
-    }
-}
-
 //! Perform actions on instrument children change.
 
 void LinkInstrumentManager::onInstrumentChildChange(InstrumentItem* instrument, SessionItem* child)
@@ -113,41 +107,28 @@ void LinkInstrumentManager::onInstrumentChildChange(InstrumentItem* instrument,
     if (child == nullptr)
         return;
 
-    onInstrumentLayoutChange(instrument);
+    ASSERT(instrument != nullptr);
+
+    // Run through all RealDataItem and refresh linking to match possible change in detector
+    // axes definition.
+    for (auto realDataItem : linkedRealDataItems(instrument))
+        if (!instrument->alignedWith(realDataItem))
+            realDataItem->clearInstrumentId();
+        else
+            realDataItem->updateToInstrument(instrument);
 }
 
 //! Updates map of instruments on insert/remove InstrumentItem event.
 
 void LinkInstrumentManager::onInstrumentAddedOrRemoved()
 {
-    updateInstrumentSubscriptions();
-    updateLinks();
-}
-
-//! Updates map of data on insert/remove RealDataItem event.
-
-void LinkInstrumentManager::onRealDataAddedOrRemoved()
-{
-    updateRealDataSubscriptions();
-    updateLinks();
-}
-
-//! Runs through all RealDataItem and check all links.
-
-void LinkInstrumentManager::updateLinks()
-{
+    // remove links in realDataItems (in case of a linked instrument was removed)
     for (auto realDataItem : m_realDataModel->realDataItems()) {
-        const QString identifier = realDataItem->instrumentId();
-        auto instrumentItem = m_instrumentModel->findInstrumentById(identifier);
-
-        if (!instrumentItem) {
-            // if no instrument with P_INSTRUMENT_ID exists, break the link
+        if (!m_instrumentModel->instrumentExists(realDataItem->instrumentId()))
             realDataItem->clearInstrumentId();
-        } else {
-            // refresh the link to update axes
-            realDataItem->linkToInstrument(instrumentItem);
-        }
     }
+
+    updateInstrumentSubscriptions();
 }
 
 //! Set up callbacks to all instrument items
@@ -165,35 +146,6 @@ void LinkInstrumentManager::updateInstrumentSubscriptions()
     }
 }
 
-//! Sets callbacks for all RealDataItem.
-
-void LinkInstrumentManager::updateRealDataSubscriptions()
-{
-    for (auto dataItem : m_realDataModel->realDataItems()) {
-        dataItem->mapper()->unsubscribe(this);
-
-        dataItem->mapper()->setOnPropertyChange(
-            [this, dataItem](const QString& name) { setOnRealDataPropertyChange(dataItem, name); },
-            this);
-    }
-}
-
-//! Runs through all RealDataItem and refresh linking to match possible change in detector
-//! axes definition.
-
-void LinkInstrumentManager::onInstrumentLayoutChange(InstrumentItem* changedInstrument)
-{
-    ASSERT(changedInstrument != nullptr);
-
-    for (auto realDataItem : linkedRealDataItems(changedInstrument))
-        if (!changedInstrument->alignedWith(realDataItem))
-            realDataItem->clearInstrumentId();
-        else
-            realDataItem->linkToInstrument(
-                changedInstrument); // #baimport This is already linked, only the UpdateToInstrument
-                                    // should be called
-}
-
 //! Returns list of RealDataItem's linked to given instrument.
 
 QList<RealDataItem*> LinkInstrumentManager::linkedRealDataItems(InstrumentItem* instrumentItem)
diff --git a/GUI/coregui/Views/ImportDataWidgets/LinkInstrumentManager.h b/GUI/coregui/Views/ImportDataWidgets/LinkInstrumentManager.h
index c10101db9aa..56f52703650 100644
--- a/GUI/coregui/Views/ImportDataWidgets/LinkInstrumentManager.h
+++ b/GUI/coregui/Views/ImportDataWidgets/LinkInstrumentManager.h
@@ -40,19 +40,19 @@ public:
 
     void setModels(InstrumentModel* instrumentModel, RealDataModel* realDataModel);
 
-    bool canLinkDataToInstrument(const RealDataItem* realDataItem, const QString& identifier);
+    //! quiet defines whether a "not possible" message box is shown if link is not possible. Use
+    //! this e.g. for unit tests. The question for adjusting the instrument is not suppressed by
+    //! this flag.
+    bool canLinkDataToInstrument(const RealDataItem* realDataItem, const QString& identifier,
+                                 bool quiet = false);
+
     QList<RealDataItem*> linkedRealDataItems(InstrumentItem* instrumentItem);
 
 private slots:
-    void setOnRealDataPropertyChange(SessionItem* dataItem, const QString& property);
     void onInstrumentChildChange(InstrumentItem* instrument, SessionItem* child);
     void onInstrumentAddedOrRemoved();
-    void onRealDataAddedOrRemoved();
 
-    void updateLinks();
     void updateInstrumentSubscriptions();
-    void updateRealDataSubscriptions();
-    void onInstrumentLayoutChange(InstrumentItem* changedInstrument);
 
 private:
     InstrumentModel* m_instrumentModel;
diff --git a/GUI/coregui/Views/ImportDataWidgets/RealDataPropertiesWidget.cpp b/GUI/coregui/Views/ImportDataWidgets/RealDataPropertiesWidget.cpp
index dae7af59182..0ab4f3e362d 100644
--- a/GUI/coregui/Views/ImportDataWidgets/RealDataPropertiesWidget.cpp
+++ b/GUI/coregui/Views/ImportDataWidgets/RealDataPropertiesWidget.cpp
@@ -96,9 +96,10 @@ void RealDataPropertiesWidget::onInstrumentComboIndexChanged(int index)
         return;
 
     if (MainWindow::instance()->linkInstrumentManager()->canLinkDataToInstrument(
-            m_currentDataItem, newSelectedInstrumentId))
+            m_currentDataItem, newSelectedInstrumentId)) {
         m_currentDataItem->setInstrumentId(newSelectedInstrumentId);
-    else
+        m_currentDataItem->updateToInstrument(newSelectedInstrumentId);
+    } else
         // Linking was impossible or denied. Set combo to previous state
         setComboToIdentifier(m_currentDataItem->instrumentId());
 }
diff --git a/GUI/coregui/Views/SpecularDataWidgets/SpecularDataImportWidget.cpp b/GUI/coregui/Views/SpecularDataWidgets/SpecularDataImportWidget.cpp
index 4377cd0d010..c3ad2341ab4 100644
--- a/GUI/coregui/Views/SpecularDataWidgets/SpecularDataImportWidget.cpp
+++ b/GUI/coregui/Views/SpecularDataWidgets/SpecularDataImportWidget.cpp
@@ -18,6 +18,7 @@
 #include "GUI/coregui/DataLoaders/DataLoaders1D.h"
 #include "GUI/coregui/DataLoaders/QREDataLoader.h"
 #include "GUI/coregui/Models/DataItemUtils.h"
+#include "GUI/coregui/Models/InstrumentItems.h"
 #include "GUI/coregui/Models/RealDataItem.h"
 #include "GUI/coregui/Models/SpecularDataItem.h"
 #include "GUI/coregui/mainwindow/AppSvc.h"
@@ -37,7 +38,7 @@ SpecularDataImportWidget::SpecularDataImportWidget(QWidget* parent)
     setSizePolicy(QSizePolicy::Expanding, QSizePolicy::Expanding);
 
     m_ui->setupUi(this);
-    m_ui->linkedInstrumentGroup->hide();    // #baimport - remove from UI if not used in the future
+    m_ui->linkedInstrumentGroup->hide(); // #baimport - remove from UI if not used in the future
 
     if (DataLoaders1D::instance().loaders().isEmpty())
         DataLoaders1D::instance().initBuiltInLoaders();
@@ -223,6 +224,13 @@ void SpecularDataImportWidget::onPropertiesChanged()
     QStringList errors, warnings;
     m_loader->importFile(realDataItem()->nativeFileName(), realDataItem(), &errors, &warnings);
 
+    // If there is a linked instrument, any change in import settings can break the compatibility.
+    // Therefore check the compatibility and break the link if necessary
+    if (realDataItem()->linkedInstrument() != nullptr) {
+        if (!realDataItem()->linkedInstrument()->alignedWith(realDataItem()))
+            realDataItem()->clearInstrumentId();
+    }
+
     updatePreview();
 }
 
diff --git a/GUI/coregui/mainwindow/projectdocument.cpp b/GUI/coregui/mainwindow/projectdocument.cpp
index c8dda622ef8..04e24352187 100644
--- a/GUI/coregui/mainwindow/projectdocument.cpp
+++ b/GUI/coregui/mainwindow/projectdocument.cpp
@@ -149,7 +149,6 @@ void ProjectDocument::load(const QString& project_file_name)
 
         timer2.start();
         m_dataService->load(projectDir(), m_messageService);
-        m_applicationModels->jobModel()->link_instruments();
         connectModels();
 
         if (m_messageService->warningCount())
diff --git a/Tests/UnitTests/GUI/TestLinkInstrument.cpp b/Tests/UnitTests/GUI/TestLinkInstrument.cpp
index 44e61826858..c4ce022e6c6 100644
--- a/Tests/UnitTests/GUI/TestLinkInstrument.cpp
+++ b/Tests/UnitTests/GUI/TestLinkInstrument.cpp
@@ -30,7 +30,7 @@ TEST_F(TestLinkInstrument, test_canLinkToInstrument)
     RealDataItem* realData = GuiUnittestUtils::createRealData("RealData", realDataModel);
     JobItemUtils::createDefaultDetectorMap(realData->dataItem(), instrument);
 
-    QVERIFY(manager.canLinkDataToInstrument(realData, identifier));
+    ASSERT_TRUE(manager.canLinkDataToInstrument(realData, identifier, true));
 
     // making link
     realData->setInstrumentId(identifier);
-- 
GitLab