Merge branch 'CURA-7473_account_pending_update_button' of github.com:Ultimaker/Cura

cura/API/Account.py

@@ -1,7 +1,7 @@
 # Copyright (c) 2018 Ultimaker B.V.
 # Cura is released under the terms of the LGPLv3 or higher.
 from datetime import datetime
-from typing import Optional, Dict, TYPE_CHECKING, Union
+from typing import Optional, Dict, TYPE_CHECKING, Callable
 
 from PyQt5.QtCore import QObject, pyqtSignal, pyqtSlot, pyqtProperty, QTimer, Q_ENUMS
 
@@ -56,6 +56,7 @@ class Account(QObject):
     lastSyncDateTimeChanged = pyqtSignal()
     syncStateChanged = pyqtSignal(int)  # because SyncState is an int Enum
     manualSyncEnabledChanged = pyqtSignal(bool)
+    updatePackagesEnabledChanged = pyqtSignal(bool)
 
     def __init__(self, application: "CuraApplication", parent = None) -> None:
         super().__init__(parent)
@@ -66,6 +67,8 @@ class Account(QObject):
         self._logged_in = False
         self._sync_state = SyncState.IDLE
         self._manual_sync_enabled = False
+        self._update_packages_enabled = False
+        self._update_packages_action = None  # type: Optional[Callable]
         self._last_sync_str = "-"
 
         self._callback_port = 32118
@@ -91,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"""
@@ -143,6 +146,18 @@ class Account(QObject):
                 if not self._update_timer.isActive():
                     self._update_timer.start()
 
+    def setUpdatePackagesAction(self, action: Callable) -> None:
+        """ Set the callback which will be invoked when the user clicks the update packages button
+
+        Should be invoked after your service sets the sync state to SYNCING and before setting the
+        sync state to SUCCESS.
+
+        Action will be reset to None when the next sync starts
+        """
+        self._update_packages_action = action
+        self._update_packages_enabled = True
+        self.updatePackagesEnabledChanged.emit(self._update_packages_enabled)
+
     def _onAccessTokenChanged(self):
         self.accessTokenChanged.emit()
 
@@ -185,6 +200,9 @@ class Account(QObject):
         sync is currently running, a sync will be requested.
         """
 
+        self._update_packages_action = None
+        self._update_packages_enabled = False
+        self.updatePackagesEnabledChanged.emit(self._update_packages_enabled)
         if self._update_timer.isActive():
             self._update_timer.stop()
         elif self._sync_state == SyncState.SYNCING:
@@ -251,6 +269,10 @@ class Account(QObject):
     def manualSyncEnabled(self) -> bool:
         return self._manual_sync_enabled
 
+    @pyqtProperty(bool, notify=updatePackagesEnabledChanged)
+    def updatePackagesEnabled(self) -> bool:
+        return self._update_packages_enabled
+
     @pyqtSlot()
     @pyqtSlot(bool)
     def sync(self, user_initiated: bool = False) -> None:
@@ -259,11 +281,14 @@ class Account(QObject):
 
         self._sync()
 
+    @pyqtSlot()
+    def onUpdatePackagesClicked(self) -> None:
+        if self._update_packages_action is not None:
+            self._update_packages_action()
+
     @pyqtSlot()
     def popupOpened(self) -> None:
         self._setManualSyncEnabled(True)
-        self._sync_state = SyncState.IDLE
-        self.syncStateChanged.emit(self._sync_state)
 
     @pyqtSlot()
     def logout(self) -> None:

plugins/Toolbox/src/CloudSync/CloudPackageChecker.py

@@ -95,10 +95,6 @@ class CloudPackageChecker(QObject):
         user_subscribed_packages = {plugin["package_id"] for plugin in subscribed_packages_payload}
         user_installed_packages = self._package_manager.getAllInstalledPackageIDs()
 
-        if user_subscribed_packages == self._last_notified_packages:
-            # already notified user about these
-            return
-
         # We need to re-evaluate the dismissed packages
         # (i.e. some package might got updated to the correct SDK version in the meantime,
         # hence remove them from the Dismissed Incompatible list)
@@ -109,7 +105,25 @@ class CloudPackageChecker(QObject):
 
         # We check if there are packages installed in Web Marketplace but not in Cura marketplace
         package_discrepancy = list(user_subscribed_packages.difference(user_installed_packages))
+
+        if user_subscribed_packages != self._last_notified_packages:
+            # scenario:
+            # 1. user subscribes to a package
+            # 2. dismisses the license/unsubscribes
+            # 3. subscribes to the same package again
+            # in this scenario we want to notify the user again. To capture that there was a change during
+            # step 2, we clear the last_notified after step 2. This way, the user will be notified after
+            # step 3 even though the list of packages for step 1 and 3 are equal
+            self._last_notified_packages = set()
+
         if package_discrepancy:
+            account = self._application.getCuraAPI().account
+            account.setUpdatePackagesAction(lambda: self._onSyncButtonClicked(None, None))
+
+            if user_subscribed_packages == self._last_notified_packages:
+                # already notified user about these
+                return
+
             Logger.log("d", "Discrepancy found between Cloud subscribed packages and Cura installed packages")
             self._model.addDiscrepancies(package_discrepancy)
             self._model.initialize(self._package_manager, subscribed_packages_payload)
@@ -144,7 +158,8 @@ class CloudPackageChecker(QObject):
             self._message.hide()
             self._message = None
 
-    def _onSyncButtonClicked(self, sync_message: Message, sync_message_action: str) -> None:
-        sync_message.hide()
+    def _onSyncButtonClicked(self, sync_message: Optional[Message], sync_message_action: Optional[str]) -> None:
+        if sync_message is not None:
+            sync_message.hide()
         self._hideSyncMessage()  # Should be the same message, but also sets _message to None
         self.discrepancies.emit(self._model)

resources/qml/Account/SyncState.qml

@@ -49,7 +49,7 @@ Row // Sync state icon + message
         width: 20 * screenScaleFactor
         height: width
 
-        source: Cura.API.account.manualSyncEnabled ? UM.Theme.getIcon("update") : UM.Theme.getIcon("checked")
+        // source is determined by State
         color: UM.Theme.getColor("account_sync_state_icon")
 
         RotationAnimator
@@ -80,14 +80,36 @@ Row // Sync state icon + message
         Label
         {
             id: stateLabel
-            text: catalog.i18nc("@state", catalog.i18nc("@label", "Account synced"))
+            // text is determined by State
             color: UM.Theme.getColor("text")
             font: UM.Theme.getFont("medium")
             renderType: Text.NativeRendering
             width: contentWidth + UM.Theme.getSize("default_margin").height
             height: contentHeight
             verticalAlignment: Text.AlignVCenter
-            visible: !Cura.API.account.manualSyncEnabled
+            visible: !Cura.API.account.manualSyncEnabled && !Cura.API.account.updatePackagesEnabled
+        }
+
+        Label
+        {
+            id: updatePackagesButton
+            text: catalog.i18nc("@button", "Install pending updates")
+            color: UM.Theme.getColor("secondary_button_text")
+            font: UM.Theme.getFont("medium")
+            renderType: Text.NativeRendering
+            verticalAlignment: Text.AlignVCenter
+            height: contentHeight
+            width: contentWidth + UM.Theme.getSize("default_margin").height
+            visible: Cura.API.account.updatePackagesEnabled
+
+            MouseArea
+            {
+                anchors.fill: parent
+                onClicked: Cura.API.account.onUpdatePackagesClicked()
+                hoverEnabled: true
+                onEntered: updatePackagesButton.font.underline = true
+                onExited: updatePackagesButton.font.underline = false
+            }
         }
 
         Label

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)
+
+
+
+
+
+