Repository: RetroAchievements/RAIntegration
Pull request #844 (2021-12-11 22:42:43)

don't show warning when deleting invalid asset


Patch:
diff --git a/src/ui/viewmodels/AssetEditorViewModel.cpp b/src/ui/viewmodels/AssetEditorViewModel.cpp
index 0f172a15..6db49021 100644
--- a/src/ui/viewmodels/AssetEditorViewModel.cpp
+++ b/src/ui/viewmodels/AssetEditorViewModel.cpp
@@ -127,7 +127,7 @@ void AssetEditorViewModel::LoadAsset(ra::data::models::AssetModelBase* pAsset)
     if (m_pAsset == pAsset)
         return;
 
-    if (HasAssetValidationError())
+    if (HasAssetValidationError() && m_pAsset->GetChanges() != ra::data::models::AssetChanges::Deleted)
     {
         if (ra::ui::viewmodels::MessageBoxViewModel::ShowWarningMessage(L"Discard changes?",
             L"The currently loaded asset has an error that cannot be saved. If you switch to another asset, your changes will be lost.",
@@ -138,14 +138,14 @@ void AssetEditorViewModel::LoadAsset(ra::data::models::AssetModelBase* pAsset)
 
         m_pAsset->RemoveNotifyTarget(*this);
         m_pAsset->RestoreLocalCheckpoint();
-
-        SetValue(AssetValidationErrorProperty, L"");
     }
     else if (m_pAsset)
     {
         m_pAsset->RemoveNotifyTarget(*this);
     }
 
+    SetValue(AssetValidationErrorProperty, L"");
+
     // null out internal pointer while binding to prevent change triggers
     m_pAsset = nullptr;
 


Test patch:
diff --git a/tests/ui/viewmodels/AssetEditorViewModel_Tests.cpp b/tests/ui/viewmodels/AssetEditorViewModel_Tests.cpp
index a3aa7139..4691072a 100644
--- a/tests/ui/viewmodels/AssetEditorViewModel_Tests.cpp
+++ b/tests/ui/viewmodels/AssetEditorViewModel_Tests.cpp
@@ -433,7 +433,7 @@ public:
         Assert::IsFalse(editor.HasMeasured());
     }
 
-    TEST_METHOD(TestLoadAchievementValidationError)
+    TEST_METHOD(TestLoadAchievementValidationWarning)
     {
         AssetEditorViewModelHarness editor;
         AchievementModel achievement;
@@ -457,6 +457,121 @@ public:
         Assert::AreEqual(std::wstring(L"Final condition type expects another condition to follow"), editor.GetAssetValidationWarning());
 
         editor.LoadAsset(nullptr);
+        Assert::IsFalse(editor.mockDesktop.WasDialogShown());
+        Assert::IsFalse(editor.HasAssetValidationError());
+        Assert::AreEqual(std::wstring(), editor.GetAssetValidationError());
+        Assert::IsFalse(editor.HasAssetValidationWarning());
+        Assert::AreEqual(std::wstring(), editor.GetAssetValidationWarning());
+    }
+
+    TEST_METHOD(TestLoadAchievementValidationError)
+    {
+        AssetEditorViewModelHarness editor;
+        AchievementModel achievement;
+        achievement.SetName(L"Test Achievement");
+        achievement.SetID(1234U);
+        achievement.SetState(AssetState::Active);
+        achievement.SetDescription(L"Do something cool");
+        achievement.SetCategory(AssetCategory::Unofficial);
+        achievement.SetPoints(10);
+        achievement.SetBadge(L"58329");
+        achievement.SetTrigger("M:0x1234=10");
+        achievement.CreateServerCheckpoint();
+        achievement.CreateLocalCheckpoint();
+
+        editor.LoadAsset(&achievement);
+        editor.SetAssetValidationError(L"Multiple Measured"); // not really, just fake it
+
+        Assert::IsTrue(editor.HasAssetValidationError());
+        Assert::AreEqual(std::wstring(L"Multiple Measured"), editor.GetAssetValidationError());
+
+        Assert::IsFalse(editor.HasAssetValidationWarning());
+        Assert::AreEqual(std::wstring(), editor.GetAssetValidationWarning());
+
+        editor.mockDesktop.ExpectWindow<ra::ui::viewmodels::MessageBoxViewModel>([](ra::ui::viewmodels::MessageBoxViewModel& vmMessageBox)
+        {
+            Assert::AreEqual(std::wstring(L"Discard changes?"), vmMessageBox.GetHeader());
+            Assert::AreEqual(std::wstring(L"The currently loaded asset has an error that cannot be saved. If you switch to another asset, your changes will be lost."), vmMessageBox.GetMessage());
+            return ra::ui::DialogResult::Yes;
+        });
+
+        editor.LoadAsset(nullptr);
+        Assert::IsTrue(editor.mockDesktop.WasDialogShown());
+        Assert::IsFalse(editor.HasAssetValidationError());
+        Assert::AreEqual(std::wstring(), editor.GetAssetValidationError());
+        Assert::IsFalse(editor.HasAssetValidationWarning());
+        Assert::AreEqual(std::wstring(), editor.GetAssetValidationWarning());
+    }
+
+    TEST_METHOD(TestLoadAchievementValidationErrorCancel)
+    {
+        AssetEditorViewModelHarness editor;
+        AchievementModel achievement;
+        achievement.SetName(L"Test Achievement");
+        achievement.SetID(1234U);
+        achievement.SetState(AssetState::Active);
+        achievement.SetDescription(L"Do something cool");
+        achievement.SetCategory(AssetCategory::Unofficial);
+        achievement.SetPoints(10);
+        achievement.SetBadge(L"58329");
+        achievement.SetTrigger("M:0x1234=10");
+        achievement.CreateServerCheckpoint();
+        achievement.CreateLocalCheckpoint();
+
+        editor.LoadAsset(&achievement);
+        editor.SetAssetValidationError(L"Multiple Measured"); // not really, just fake it
+
+        Assert::IsTrue(editor.HasAssetValidationError());
+        Assert::AreEqual(std::wstring(L"Multiple Measured"), editor.GetAssetValidationError());
+
+        Assert::IsFalse(editor.HasAssetValidationWarning());
+        Assert::AreEqual(std::wstring(), editor.GetAssetValidationWarning());
+
+        editor.mockDesktop.ExpectWindow<ra::ui::viewmodels::MessageBoxViewModel>([](ra::ui::viewmodels::MessageBoxViewModel& vmMessageBox)
+        {
+            Assert::AreEqual(std::wstring(L"Discard changes?"), vmMessageBox.GetHeader());
+            Assert::AreEqual(std::wstring(L"The currently loaded asset has an error that cannot be saved. If you switch to another asset, your changes will be lost."), vmMessageBox.GetMessage());
+            return ra::ui::DialogResult::No;
+        });
+
+        editor.LoadAsset(nullptr);
+        Assert::IsTrue(editor.mockDesktop.WasDialogShown());
+
+        Assert::IsTrue(editor.HasAssetValidationError());
+        Assert::AreEqual(std::wstring(L"Multiple Measured"), editor.GetAssetValidationError());
+        Assert::IsFalse(editor.HasAssetValidationWarning());
+        Assert::AreEqual(std::wstring(), editor.GetAssetValidationWarning());
+        Assert::IsTrue(editor.GetAsset() == &achievement);
+    }
+
+    TEST_METHOD(TestLoadAchievementValidationErrorDeleted)
+    {
+        AssetEditorViewModelHarness editor;
+        AchievementModel achievement;
+        achievement.SetName(L"Test Achievement");
+        achievement.SetID(1234U);
+        achievement.SetState(AssetState::Active);
+        achievement.SetDescription(L"Do something cool");
+        achievement.SetCategory(AssetCategory::Unofficial);
+        achievement.SetPoints(10);
+        achievement.SetBadge(L"58329");
+        achievement.SetTrigger("M:0x1234=10");
+        achievement.CreateServerCheckpoint();
+        achievement.CreateLocalCheckpoint();
+
+        editor.LoadAsset(&achievement);
+        editor.SetAssetValidationError(L"Multiple Measured"); // not really, just fake it
+
+        Assert::IsTrue(editor.HasAssetValidationError());
+        Assert::AreEqual(std::wstring(L"Multiple Measured"), editor.GetAssetValidationError());
+
+        Assert::IsFalse(editor.HasAssetValidationWarning());
+        Assert::AreEqual(std::wstring(), editor.GetAssetValidationWarning());
+
+        achievement.SetDeleted();
+
+        editor.LoadAsset(nullptr);
+        Assert::IsFalse(editor.mockDesktop.WasDialogShown());
         Assert::IsFalse(editor.HasAssetValidationError());
         Assert::AreEqual(std::wstring(), editor.GetAssetValidationError());
         Assert::IsFalse(editor.HasAssetValidationWarning());