Clean up the logic of creating new project tree items, and improve test coverage

nw/gui/projtree.py

@@ -164,6 +164,11 @@ class GuiProjectTree(QTreeWidget):
             logger.error("No project open")
             return False
 
+        if not isinstance(itemType, nwItemType):
+            # This would indicate an internal bug
+            logger.error("No itemType provided")
+            return False
+
         # The item needs to be assigned an item class, so one must be
         # provided, or it must be possible to extract it from the parent
         # item of the new item.
@@ -172,21 +177,18 @@ class GuiProjectTree(QTreeWidget):
             if pItem is not None:
                 itemClass = pItem.itemClass
 
+        # If class is still not set, alert the user and exit
         if itemClass is None:
-            if itemType is not None:
+            if itemType == nwItemType.FILE:
-                if itemType == nwItemType.FILE:
+                self.makeAlert(
-                    self.makeAlert(
+                    "Please select a valid location in the tree to add the document.",
-                        "Please select a valid location in the tree to add a document.",
+                    nwAlert.ERROR
-                        nwAlert.ERROR
+                )
-                    )
+            else:
-                    return False
+                self.makeAlert(
-                elif itemType == nwItemType.FOLDER:
+                    "Please select a valid location in the tree to add the folder.",
-                    self.makeAlert(
+                    nwAlert.ERROR
-                        "Please select a valid location in the tree to add a folder.",
+                )
-                        nwAlert.ERROR
-                    )
-                    return False
-            self.makeAlert("Failed to add new item.", nwAlert.BUG)
             return False
 
         # Everything is fine, we have what we need, so we proceed
@@ -259,8 +261,7 @@ class GuiProjectTree(QTreeWidget):
         # Add the new item to the tree
         if tHandle is not None:
             self.revealNewTreeItem(tHandle, nHandle)
-            if self.mainConf.showGUI:
+            self.theParent.editItem(tHandle)
-                self.theParent.editItem(tHandle)
 
         return True
 
@@ -287,7 +288,7 @@ class GuiProjectTree(QTreeWidget):
             logger.error("No project open")
             return False
 
-        if qApp.focusWidget() != self and self.mainConf.showGUI:
+        if qApp.focusWidget() != self:
             return False
 
         tHandle = self.getSelectedHandle()

tests/test_gui_projtree.py

@@ -5,95 +5,133 @@
 import pytest
 import os
 
+from tools import writeFile
+
 from PyQt5.QtCore import QItemSelectionModel
 from PyQt5.QtWidgets import QAction, QMessageBox
 
 from nw.constants import nwItemType, nwItemClass
 
-keyDelay = 2
-typeDelay = 1
-stepDelay = 20
-
 @pytest.mark.gui
-def testGuiProjTree_Main(qtbot, monkeypatch, nwGUI, nwMinimal):
+def testGuiProjTree_TreeItems(qtbot, caplog, monkeypatch, nwGUI, nwMinimal):
-    """Test the project tree.
+    """Test adding and removing items from the project tree.
     """
     # Block message box
     monkeypatch.setattr(QMessageBox, "question", lambda *args: QMessageBox.Yes)
+    monkeypatch.setattr("nw.guimain.GuiMain.editItem", lambda *args: None)
 
     nwGUI.theProject.projTree.setSeed(42)
-    assert nwGUI.openProject(nwMinimal)
     nwTree = nwGUI.treeView
 
+    ##
+    #  Add New Items
+    ##
+
+    # Try to add and move item with no project
+    assert not nwTree.newTreeItem(nwItemType.FILE, None)
+    assert not nwTree.moveTreeItem(1)
+
+    # Open a project
+    assert nwGUI.openProject(nwMinimal)
+
     # No location selected for new item
+    nwTree.clearSelection()
     assert not nwTree.newTreeItem(nwItemType.FILE, None)
     assert not nwTree.newTreeItem(nwItemType.FOLDER, None)
+    assert nwTree.newTreeItem(nwItemType.FILE, nwItemClass.NOVEL)
+
+    # No itemType set or ROOT, but no class
+    assert not nwTree.newTreeItem(None, None)
+    assert not nwTree.newTreeItem(nwItemType.ROOT, None)
 
     # Select a location
     chItem = nwTree._getTreeItem("a6d311a93600a")
     nwTree.setCurrentItem(chItem, QItemSelectionModel.Current)
     chItem.setExpanded(True)
 
-    # Create new item with no class set
+    # Create new item with no class set (defaults to NOVEL)
     assert nwTree.newTreeItem(nwItemType.FILE, None)
     assert nwTree.newTreeItem(nwItemType.FOLDER, None)
 
+    # Check that we have the correct tree order
+    assert nwTree.getTreeFromHandle("a6d311a93600a") == [
+        "a6d311a93600a", "f5ab3e30151e1", "8c659a11cd429", "44cb730c42048", "71ee45a3c0db9"
+    ]
+
     # Add roots
-    assert not nwTree.newTreeItem(nwItemType.ROOT, None)              # Defaults to NOVEL
     assert not nwTree.newTreeItem(nwItemType.ROOT, nwItemClass.WORLD) # Duplicate
     assert nwTree.newTreeItem(nwItemType.ROOT, nwItemClass.CUSTOM)    # Valid
 
-    # Check that we have the correct tree order
+    # Change max depth and try to add a subfolder that is too deep
+    monkeypatch.setattr("nw.constants.nwConst.MAX_DEPTH", 2)
+    chItem = nwTree._getTreeItem("71ee45a3c0db9")
+    nwTree.setCurrentItem(chItem, QItemSelectionModel.Current)
+    assert not nwTree.newTreeItem(nwItemType.FOLDER, None)
+
+    ##
+    #  Move Items
+    ##
+
+    nwTree.setSelectedHandle("8c659a11cd429")
+
+    # Shift focus and try to move item
+    monkeypatch.setattr("PyQt5.QtWidgets.qApp.focusWidget", lambda: None)
+    assert not nwTree.moveTreeItem(1)
     assert nwTree.getTreeFromHandle("a6d311a93600a") == [
-        "a6d311a93600a", "f5ab3e30151e1", "8c659a11cd429", "73475cb40a568", "44cb730c42048"
+        "a6d311a93600a", "f5ab3e30151e1", "8c659a11cd429", "44cb730c42048", "71ee45a3c0db9"
     ]
+    monkeypatch.setattr("PyQt5.QtWidgets.qApp.focusWidget", lambda: nwTree)
 
     # Move second item up twice (should give same result)
-    nwTree.setSelectedHandle("8c659a11cd429")
     nwGUI.mainMenu.aMoveUp.activate(QAction.Trigger)
     assert nwTree.getTreeFromHandle("a6d311a93600a") == [
-        "a6d311a93600a", "8c659a11cd429", "f5ab3e30151e1", "73475cb40a568", "44cb730c42048"
+        "a6d311a93600a", "8c659a11cd429", "f5ab3e30151e1", "44cb730c42048", "71ee45a3c0db9"
     ]
     nwGUI.mainMenu.aMoveUp.activate(QAction.Trigger)
     assert nwTree.getTreeFromHandle("a6d311a93600a") == [
-        "a6d311a93600a", "8c659a11cd429", "f5ab3e30151e1", "73475cb40a568", "44cb730c42048"
+        "a6d311a93600a", "8c659a11cd429", "f5ab3e30151e1", "44cb730c42048", "71ee45a3c0db9"
     ]
 
-    # Move it back down four times (last to should be the same)
+    # Move it back down four times (last two should be the same)
     nwGUI.mainMenu.aMoveDown.activate(QAction.Trigger)
     assert nwTree.getTreeFromHandle("a6d311a93600a") == [
-        "a6d311a93600a", "f5ab3e30151e1", "8c659a11cd429", "73475cb40a568", "44cb730c42048"
+        "a6d311a93600a", "f5ab3e30151e1", "8c659a11cd429", "44cb730c42048", "71ee45a3c0db9"
     ]
     nwGUI.mainMenu.aMoveDown.activate(QAction.Trigger)
     assert nwTree.getTreeFromHandle("a6d311a93600a") == [
-        "a6d311a93600a", "f5ab3e30151e1", "73475cb40a568", "8c659a11cd429", "44cb730c42048"
+        "a6d311a93600a", "f5ab3e30151e1", "44cb730c42048", "8c659a11cd429", "71ee45a3c0db9"
     ]
     nwGUI.mainMenu.aMoveDown.activate(QAction.Trigger)
     assert nwTree.getTreeFromHandle("a6d311a93600a") == [
-        "a6d311a93600a", "f5ab3e30151e1", "73475cb40a568", "44cb730c42048", "8c659a11cd429"
+        "a6d311a93600a", "f5ab3e30151e1", "44cb730c42048", "71ee45a3c0db9", "8c659a11cd429"
     ]
     nwGUI.mainMenu.aMoveDown.activate(QAction.Trigger)
     assert nwTree.getTreeFromHandle("a6d311a93600a") == [
-        "a6d311a93600a", "f5ab3e30151e1", "73475cb40a568", "44cb730c42048", "8c659a11cd429"
+        "a6d311a93600a", "f5ab3e30151e1", "44cb730c42048", "71ee45a3c0db9", "8c659a11cd429"
     ]
 
     # Move a root item (top level items are different) twice
     nwTree.flushTreeOrder()
-    assert nwGUI.theProject.projTree._treeOrder.index("9d5247ab588e0") == 9
+    assert nwGUI.theProject.projTree._treeOrder.index("9d5247ab588e0") == 10
     nwTree.setSelectedHandle("9d5247ab588e0")
 
     nwGUI.mainMenu.aMoveDown.activate(QAction.Trigger)
     nwTree.flushTreeOrder()
-    assert nwGUI.theProject.projTree._treeOrder.index("9d5247ab588e0") == 10
+    assert nwGUI.theProject.projTree._treeOrder.index("9d5247ab588e0") == 11
 
     nwGUI.mainMenu.aMoveDown.activate(QAction.Trigger)
     nwTree.flushTreeOrder()
-    assert nwGUI.theProject.projTree._treeOrder.index("9d5247ab588e0") == 10
+    assert nwGUI.theProject.projTree._treeOrder.index("9d5247ab588e0") == 11
+
+    ##
+    #  Delete and Trash
+    ##
 
     # Add some content to the new file
     nwGUI.openDocument("73475cb40a568")
     nwGUI.docEditor.setText("# Hello World\n")
     nwGUI.saveDocument()
+    nwGUI.saveProject()
     assert os.path.isfile(os.path.join(nwMinimal, "content", "73475cb40a568.nwd"))
 
     # Delete the items we added earlier
@@ -102,11 +140,11 @@ def testGuiProjTree_Main(qtbot, monkeypatch, nwGUI, nwMinimal):
     assert not nwTree.deleteItem(None)
     assert not nwTree.deleteItem("1111111111111")
     assert nwTree.deleteItem("73475cb40a568") # New File
-    assert nwTree.deleteItem("44cb730c42048") # New Folder
+    assert nwTree.deleteItem("71ee45a3c0db9") # New Folder
-    assert nwTree.deleteItem("71ee45a3c0db9") # Custom Root
+    assert nwTree.deleteItem("811786ad1ae74") # Custom Root
     assert "73475cb40a568" in nwGUI.theProject.projTree._treeOrder
-    assert "44cb730c42048" not in nwGUI.theProject.projTree._treeOrder
     assert "71ee45a3c0db9" not in nwGUI.theProject.projTree._treeOrder
+    assert "811786ad1ae74" not in nwGUI.theProject.projTree._treeOrder
 
     # The file is in trash, empty it
     assert os.path.isfile(os.path.join(nwMinimal, "content", "73475cb40a568.nwd"))
@@ -115,13 +153,23 @@ def testGuiProjTree_Main(qtbot, monkeypatch, nwGUI, nwMinimal):
     assert not os.path.isfile(os.path.join(nwMinimal, "content", "73475cb40a568.nwd"))
     assert "73475cb40a568" not in nwGUI.theProject.projTree._treeOrder
 
+    # Should not be allowed to add files and folders to Trash
+    trashHandle = nwGUI.theProject.projTree.trashRoot()
+    chItem = nwTree._getTreeItem(trashHandle)
+    nwTree.setCurrentItem(chItem, QItemSelectionModel.Current)
+    assert not nwTree.newTreeItem(nwItemType.FILE, None)
+    assert not nwTree.newTreeItem(nwItemType.FOLDER, None)
+
     # Close the project
     nwGUI.closeProject()
 
+    ##
+    #  Orphaned Files
+    ##
+
     # Add an orphaned file
     orphFile = os.path.join(nwMinimal, "content", "1234567890abc.nwd")
-    with open(orphFile, mode="w+", encoding="utf8") as outFile:
+    writeFile(orphFile, "# Hello World\n")
-        outFile.write("# Hello World\n")
 
     # Open the project again
     nwGUI.openProject(nwMinimal)
@@ -132,6 +180,28 @@ def testGuiProjTree_Main(qtbot, monkeypatch, nwGUI, nwMinimal):
     orItem = nwTree._getTreeItem("1234567890abc")
     assert orItem.text(nwTree.C_NAME) == "Recovered File 1"
 
-    # qtbot.stopForInteraction()
+    ##
+    #  Unexpected Error Handling
+    ##
 
-# END Test testGuiProjTree_Main
+    # Add an item with an invalid type
+    assert not nwTree.newTreeItem(nwItemType.NO_TYPE, nwItemClass.NOVEL)
+    assert "Failed to add new item" in caplog.messages[-1]
+
+    # Add new file after one that has no parent handle
+    chItem = nwTree._getTreeItem("44cb730c42048")
+    nwTree.setCurrentItem(chItem, QItemSelectionModel.Current)
+    nwTree.theProject.projTree["44cb730c42048"].itemParent = None
+    assert not nwTree.newTreeItem(nwItemType.FILE, nwItemClass.NOVEL)
+    nwTree.clearSelection()
+
+    # Add a file with no parent, and fail to find a suitable parent item
+    monkeypatch.setattr("nw.core.tree.NWTree.findRoot", lambda *args: None)
+
+    assert not nwTree.newTreeItem(nwItemType.FILE, nwItemClass.NOVEL)
+    assert not nwTree.newTreeItem(nwItemType.FOLDER, nwItemClass.NOVEL)
+
+    # qtbot.stopForInteraction()
+    nwGUI.closeProject()
+
+# END Test testGuiProjTree_TreeItems