From 294afdb7ca762c0cd174f9a25e5c8be4c157d0ad Mon Sep 17 00:00:00 2001
From: Nino van Hooff <ninovanhooff@gmail.com>
Date: Fri, 26 Jun 2020 16:09:33 +0200
Subject: [PATCH] Connect sync timer to sync() for consistency and add unit
 tests.

Also rename a function to camelCase

CURA-7473
---
 cura/API/Account.py                 |  4 +--
 resources/qml/Account/SyncState.qml |  2 +-
 tests/API/TestAccount.py            | 53 +++++++++++++++++++++++++++++
 3 files changed, 56 insertions(+), 3 deletions(-)

diff --git a/cura/API/Account.py b/cura/API/Account.py
index 991a2ef967..3ef95834d2 100644
--- a/cura/API/Account.py
+++ b/cura/API/Account.py
@@ -94,7 +94,7 @@ class Account(QObject):
         self._update_timer.setInterval(int(self.SYNC_INTERVAL * 1000))
         # The timer is restarted explicitly after an update was processed. This prevents 2 concurrent updates
         self._update_timer.setSingleShot(True)
-        self._update_timer.timeout.connect(self.syncRequested)
+        self._update_timer.timeout.connect(self.sync)
 
         self._sync_services = {}  # type: Dict[str, int]
         """contains entries "service_name" : SyncState"""
@@ -282,7 +282,7 @@ class Account(QObject):
         self._sync()
 
     @pyqtSlot()
-    def update_packages(self):
+    def onUpdatePackagesClicked(self):
         if self._update_packages_action is not None:
             self._update_packages_action()
 
diff --git a/resources/qml/Account/SyncState.qml b/resources/qml/Account/SyncState.qml
index 77547e48f8..66e7f7877b 100644
--- a/resources/qml/Account/SyncState.qml
+++ b/resources/qml/Account/SyncState.qml
@@ -105,7 +105,7 @@ Row // Sync state icon + message
             MouseArea
             {
                 anchors.fill: parent
-                onClicked: Cura.API.account.update_packages()
+                onClicked: Cura.API.account.onUpdatePackagesClicked()
                 hoverEnabled: true
                 onEntered: updatePackagesButton.font.underline = true
                 onExited: updatePackagesButton.font.underline = false
diff --git a/tests/API/TestAccount.py b/tests/API/TestAccount.py
index 09091ba7e0..6780e50b81 100644
--- a/tests/API/TestAccount.py
+++ b/tests/API/TestAccount.py
@@ -3,6 +3,7 @@ from unittest.mock import MagicMock, patch
 import pytest
 
 from cura.API import Account
+from cura.API.Account import SyncState
 from cura.OAuth2.Models import UserProfile
 
 
@@ -117,3 +118,55 @@ def test_userProfile(user_profile):
 
     mocked_auth_service.getUserProfile = MagicMock(return_value=None)
     assert account.userProfile is None
+
+
+def test_sync_success():
+    account = Account(MagicMock())
+
+    service1 = "test_service1"
+    service2 = "test_service2"
+
+    account.setSyncState(service1, SyncState.SYNCING)
+    assert account.syncState == SyncState.SYNCING
+
+    account.setSyncState(service2, SyncState.SYNCING)
+    assert account.syncState == SyncState.SYNCING
+
+    account.setSyncState(service1, SyncState.SUCCESS)
+    # service2 still syncing
+    assert account.syncState == SyncState.SYNCING
+
+    account.setSyncState(service2, SyncState.SUCCESS)
+    assert account.syncState == SyncState.SUCCESS
+
+
+def test_sync_update_action():
+    account = Account(MagicMock())
+
+    service1 = "test_service1"
+
+    mockUpdateCallback = MagicMock()
+
+    account.setSyncState(service1, SyncState.SYNCING)
+    assert account.syncState == SyncState.SYNCING
+
+    account.setUpdatePackagesAction(mockUpdateCallback)
+    account.onUpdatePackagesClicked()
+    mockUpdateCallback.assert_called_once_with()
+    account.setSyncState(service1, SyncState.SUCCESS)
+
+    account.sync()  # starting a new sync resets the update action to None
+
+    account.setSyncState(service1, SyncState.SYNCING)
+    assert account.syncState == SyncState.SYNCING
+
+    account.onUpdatePackagesClicked()  # Should not be connected to an action anymore
+    mockUpdateCallback.assert_called_once_with()  # No additional calls
+    assert account.updatePackagesEnabled is False
+    account.setSyncState(service1, SyncState.SUCCESS)
+
+
+
+
+
+