Oncat connectivity status +configuration mechanism (#146)

* configuration mechanism for version update, oncat connectivity status
* oncat sync update and test
* update connection status on the startup
* configuration tests updated for version field, conf file with version added
* group attributed added to false, needed for latest mantid versions

Author: mpatrou

src/shiver/configuration.py

@@ -8,6 +8,7 @@
 from configparser import ConfigParser
 from pathlib import Path
 from mantid.kernel import Logger
+from shiver.version import __version__ as current_version
 
 logger = Logger("SHIVER")
 
@@ -31,6 +32,7 @@ def __init__(self):
         self.config_file_path = CONFIG_PATH_FILE
         logger.information(f"{self.config_file_path} with be used")
 
+        version_update = None
         # if template conf file path exists
         if os.path.exists(self.template_file_path):
             # file does not exist create it from template
@@ -41,20 +43,32 @@ def __init__(self):
                 shutil.copy2(self.template_file_path, self.config_file_path)
 
             self.config = ConfigParser(allow_no_value=True, comment_prefixes="/")
+
+            # the file already exists, check the version
+            self.config.read(self.config_file_path)
+            config_version = get_data("software.info", "version")
+
+            # in case of missing version or version mismatch
+            if not config_version or config_version != current_version:
+                # update the whole configuration file and the version
+                shutil.copy2(self.template_file_path, self.config_file_path)
+                version_update = current_version
+
             # parse the file
             try:
                 self.config.read(self.config_file_path)
                 # validate the file has the all the latest variables
-                self.validate()
+                self.validate(version_update)
             except ValueError as err:
                 logger.error(str(err))
                 logger.error(f"Problem with the file: {self.config_file_path}")
         else:
             logger.error(f"Template configuration file: {self.template_file_path} is missing!")
 
-    def validate(self):
+    def validate(self, version=None):
         """validates that the fields exist at the config_file_path and writes any missing fields/data
-        using the template configuration file: configuration_template.ini as a guide"""
+        using the template configuration file: configuration_template.ini as a guide
+        if version is not None, the version value is set/updated in the configuration file"""
         template_config = ConfigParser(allow_no_value=True, comment_prefixes="/")
         template_config.read(self.template_file_path)
         for section in template_config.sections():
@@ -65,6 +79,9 @@ def validate(self):
 
             for item in template_config.items(section):
                 field, _ = item
+                # if a new version is passed set that in the file
+                if version and field == "version":
+                    self.config[section][field] = version
                 if field not in self.config[section]:
                     # copy the field
                     self.config[section][field] = template_config[section][field]

src/shiver/configuration_template.ini

@@ -2,7 +2,7 @@
 #url to oncat portal
 oncat_url = https://oncat.ornl.gov
 #client id for on cat; it is unique for Shiver
-client_id = 99025bb3-ce06-4f4b-bcf2-36ebf925cd1d
+client_id = 46c478f0-a472-4551-9264-a937626d5fc2
 #the flag (bool: True/False) indicates the location of the names of the datasets (notes/comments vs. sequence name)
 use_notes = False
 
@@ -21,3 +21,8 @@ save_history = True
 
 [global.other]
 help_url = https://neutrons.github.io/Shiver/GUI/
+
+[software.info]
+#software default information
+#version is populated during the creation of the configuration file based on the current software's version
+version = 0.0.0

src/shiver/presenters/refine_ub.py

@@ -38,6 +38,8 @@ def __init__(self, ws, model, parent=None):  # pylint: disable=super-init-not-ca
         self.ads_observer = WorkspaceDisplayADSObserver(self)
         self.presenter.refresh()
         self.container = self
+        # set PeaksTableWorkspaceDisplay group equals to False for newer Mantid versions
+        self.group = False
 
     def emit_close(self):
         """Handle closing"""

src/shiver/views/oncat.py

@@ -58,7 +58,8 @@ def __init__(self, parent=None):
         self.oncat_options_layout.addWidget(self.angle_target_label, 3, 0)
         self.oncat_options_layout.addWidget(self.angle_target, 3, 1)
 
-        self.oncat_login = ONCatLogin(key="shiver", parent=self)
+        client_id = get_data("generate_tab.oncat", "client_id")
+        self.oncat_login = ONCatLogin(key="shiver", client_id=client_id, parent=self)
         self.oncat_login.connection_updated.connect(self.connect_to_oncat)
         self.oncat_options_layout.addWidget(self.oncat_login, 4, 0, 1, 2)
 
@@ -75,15 +76,6 @@ def __init__(self, parent=None):
         # Sync with remote
         self.sync_with_remote(refresh=True)
 
-        # Sync with remote every 60 seconds
-        # NOTE: make the refresh interval configurable
-        #       in application settings
-        self.update_connection_status_timer = QTimer()
-        self.update_connection_status_timer.timeout.connect(
-            self.sync_with_remote,
-        )
-        self.update_connection_status_timer.start(60_000)
-
         # change in instrument should trigger update of
         # - IPTS
         # - dataset
@@ -94,6 +86,15 @@ def __init__(self, parent=None):
         # - dataset
         self.ipts.currentTextChanged.connect(self.update_datasets)
 
+        self.show_connection_status_briefly()
+
+    def show_connection_status_briefly(self):
+        """Show connection status for 5 seconds"""
+        # connection status appears for 5 seconds only
+        self.oncat_login.status_label.show()
+        timer = QTimer()
+        timer.singleShot(5000, self.oncat_login.status_label.hide)
+
     @property
     def connected_to_oncat(self) -> bool:
         """Check if connected to OnCat"""
@@ -135,6 +136,7 @@ def connect_to_oncat(self):
         """Connect to OnCat"""
         # update connection status
         self.sync_with_remote(refresh=True)
+        self.show_connection_status_briefly()
 
     def sync_with_remote(self, refresh=False):
         """Update all items within OnCat widget."""

tests/conftest.py

@@ -7,6 +7,7 @@
 from mantidqt.gui_helper import set_matplotlib_backend
 from mantid.simpleapi import mtd
 from shiver import Shiver
+from shiver.version import __version__ as current_version
 
 # make sure matplotlib is correctly set before we run tests
 set_matplotlib_backend()
@@ -39,6 +40,21 @@ def user_conf_file(tmp_path_factory, request):
     return user_path
 
 
+@pytest.fixture(scope="session")
+def user_conf_file_with_version(tmp_path_factory, request):
+    """Fixture to create a custom configuration file in tmp_path"""
+    # custom configuration file
+    user_config = ConfigParser(allow_no_value=True)
+    # include current version
+    version_block = f"[software.info]\nversion = {current_version}\n"
+    config_data = request.param + version_block
+    user_config.read_string(config_data)
+    user_path = os.path.join(tmp_path_factory.mktemp("data"), "test_config.ini")
+    with open(user_path, "w", encoding="utf8") as config_file:
+        user_config.write(config_file)
+    return user_path
+
+
 @pytest.fixture(autouse=True)
 def _get_login(monkeypatch: pytest.fixture) -> None:
     monkeypatch.setattr(os, "getlogin", lambda: "test")

tests/models/test_configuration.py

@@ -8,6 +8,7 @@
 from qtpy.QtWidgets import QApplication
 from shiver.shiver import Shiver
 from shiver.configuration import Configuration, get_data
+from shiver.version import __version__ as current_version
 
 
 def test_config_path_default():
@@ -47,29 +48,29 @@ def test_config_path_does_not_exist(monkeypatch, tmp_path):
 
 
 @pytest.mark.parametrize(
-    "user_conf_file",
+    "user_conf_file_with_version",
     [
         """
         [generate_tab.oncat]
         #url to oncat portal
         oncat_url = https://oncat.ornl.gov
         #client id for on cat; it is unique for Shiver
-        client_id = 99025bb3-ce06-4f4b-bcf2-36ebf925cd1d
+        client_id = 46c478f0-a472-4551-9264-a937626d5fc2
 
     """
     ],
     indirect=True,
 )
-def test_field_validate_fields_exist(monkeypatch, user_conf_file):
+def test_field_validate_fields_exist(monkeypatch, user_conf_file_with_version):
     """Test configuration validate all fields exist with the same values as templates
     Note: update the parameters if the fields increase"""
     # read the custom configuration file
-    monkeypatch.setattr("shiver.configuration.CONFIG_PATH_FILE", user_conf_file)
+    monkeypatch.setattr("shiver.configuration.CONFIG_PATH_FILE", user_conf_file_with_version)
     user_config = Configuration()
 
-    assert user_config.config_file_path.endswith(user_conf_file) is True
+    assert user_config.config_file_path.endswith(user_conf_file_with_version) is True
     # check if the file exists
-    assert os.path.exists(user_conf_file)
+    assert os.path.exists(user_conf_file_with_version)
 
     # check all fields are the same as the configuration template file
     project_directory = Path(__file__).resolve().parent.parent.parent
@@ -79,11 +80,14 @@ def test_field_validate_fields_exist(monkeypatch, user_conf_file):
     # comments should be copied too
     for section in user_config.config.sections():
         for field in user_config.config[section]:
-            assert user_config.config[section][field] == template_config[section][field]
+            if field != "version":
+                assert user_config.config[section][field] == template_config[section][field]
+            else:
+                assert user_config.config[section][field] == current_version
 
 
 @pytest.mark.parametrize(
-    "user_conf_file",
+    "user_conf_file_with_version",
     [
         """
         [generate_tab.oncat]
@@ -94,36 +98,40 @@ def test_field_validate_fields_exist(monkeypatch, user_conf_file):
     ],
     indirect=True,
 )
-def test_field_validate_fields_same(monkeypatch, user_conf_file):
-    """Test configuration validate all fields exist with their values; different from the template"""
+def test_field_validate_fields_same(monkeypatch, user_conf_file_with_version):
+    """Test configuration validate all fields exist with their values; different from the template,
+    provided the version is the same"""
 
     # read the custom configuration file
-    monkeypatch.setattr("shiver.configuration.CONFIG_PATH_FILE", user_conf_file)
+    monkeypatch.setattr("shiver.configuration.CONFIG_PATH_FILE", user_conf_file_with_version)
     user_config = Configuration()
 
     # check if the file exists
-    assert os.path.exists(user_conf_file)
-    assert user_config.config_file_path == user_conf_file
+    assert os.path.exists(user_conf_file_with_version)
+    assert user_config.config_file_path == user_conf_file_with_version
 
     # check all field values have the same values as the user configuration file
     assert get_data("generate_tab.oncat", "oncat_url") == "test_url"
     assert get_data("generate_tab.oncat", "client_id") == "0000-0000"
     # cast to bool
     assert get_data("generate_tab.oncat", "use_notes") is True
+    assert get_data("software.info", "version") == current_version
 
 
 @pytest.mark.parametrize(
     "user_conf_file",
     [
         """
         [generate_tab.oncat]
+        oncat_url = test_url
         client_id = 0000-0000
+        use_notes = True
     """
     ],
     indirect=True,
 )
-def test_field_validate_fields_missing(monkeypatch, user_conf_file):
-    """Test configuration validate missing fields added from the template"""
+def test_field_validate_fields_update_different_version(monkeypatch, user_conf_file):
+    """Test configuration update all fields, provided the version is not the same"""
 
     # read the custom configuration file
     monkeypatch.setattr("shiver.configuration.CONFIG_PATH_FILE", user_conf_file)
@@ -133,6 +141,34 @@ def test_field_validate_fields_missing(monkeypatch, user_conf_file):
     assert os.path.exists(user_conf_file)
     assert user_config.config_file_path == user_conf_file
 
+    # check all field values have the same values as the user configuration file
+    assert get_data("generate_tab.oncat", "oncat_url") != "test_url"
+    assert get_data("generate_tab.oncat", "client_id") != "0000-0000"
+    # version is included with the current version
+    assert get_data("software.info", "version") == current_version
+
+
+@pytest.mark.parametrize(
+    "user_conf_file_with_version",
+    [
+        """
+        [generate_tab.oncat]
+        client_id = 0000-0000
+    """
+    ],
+    indirect=True,
+)
+def test_field_validate_fields_missing(monkeypatch, user_conf_file_with_version):
+    """Test configuration validate missing fields added from the template"""
+
+    # read the custom configuration file
+    monkeypatch.setattr("shiver.configuration.CONFIG_PATH_FILE", user_conf_file_with_version)
+    user_config = Configuration()
+
+    # check if the file exists
+    assert os.path.exists(user_conf_file_with_version)
+    assert user_config.config_file_path == user_conf_file_with_version
+
     # check all field values have the same values as the user configuration file
     assert get_data("generate_tab.oncat", "oncat_url") == "https://oncat.ornl.gov"
     assert get_data("generate_tab.oncat", "client_id") == "0000-0000"
@@ -151,7 +187,7 @@ def test_get_data_valid(monkeypatch, user_conf_file):
     assert len(get_data("generate_tab.oncat", "")) == 3
     # fields
     assert get_data("generate_tab.oncat", "oncat_url") == "https://oncat.ornl.gov"
-    assert get_data("generate_tab.oncat", "client_id") == "99025bb3-ce06-4f4b-bcf2-36ebf925cd1d"
+    assert get_data("generate_tab.oncat", "client_id") == "46c478f0-a472-4551-9264-a937626d5fc2"
     assert get_data("generate_tab.oncat", "use_notes") is False
 
     assert config.is_valid()

tests/views/test_oncat.py

@@ -1,7 +1,7 @@
 #!/usr/bin/env python
 # pylint: disable=all
 """Test the views for the ONCat application."""
-from qtpy.QtWidgets import QGroupBox
+from qtpy.QtWidgets import QGroupBox, QLabel
 from qtpy.QtCore import Signal
 import pytest
 from shiver.views.oncat import (
@@ -17,6 +17,7 @@ def test_oncat(monkeypatch, qtbot):
 
     class MockLogin(QGroupBox):
         connection_updated = Signal(bool)
+        status_label = QLabel("Test")
 
         def __init__(self, *args, parent, **kwargs):
             super().__init__(parent=parent)