Merge pull request #425 from sebhub/fix-item-validation

Fix item reformat during validation

doorstop/core/item.py

@@ -552,9 +552,6 @@ def unlink(self, value):
     def is_reviewed(self):
         return self._data['reviewed']
 
-    def set_links(self, links):
-        self._data['links'] = links
-
     @requires_tree
     def find_ref(self):
         """Get the external file reference and line number.

doorstop/core/tests/test_item_validator.py

@@ -104,6 +104,45 @@ def test_validate_cleared_new(self):
         self.assertTrue(self.item_validator.validate(self.item))
         self.assertEqual('abc123', self.item.links[0].stamp)
 
+    @patch('doorstop.settings.REFORMAT', True)
+    def test_validate_reformat_when_setting_is_set(self):
+        """Verify that new links are stamped automatically."""
+        mock_item = Mock()
+        mock_item.stamp = Mock(return_value=Stamp('abc123'))
+
+        mock_tree = MagicMock()
+        mock_tree.find_item = Mock(return_value=mock_item)
+        self.item.tree = mock_tree
+        self.item.links = [{'mock_uid': None}, {'mock_uid': None}]
+        self.item_validator.disable_get_issues_document()
+        self.assertTrue(self.item_validator.validate(self.item))
+        self.assertEqual('abc123', self.item.links[0].stamp)
+
+        # two calls:
+        # 1) setting up mock links with self.item.links above
+        # 2) calling item.links because of REFORMAT == True
+        self.assertEqual(self.item._write.call_count, 2)
+
+    @patch('doorstop.settings.REFORMAT', False)
+    def test_validate_no_reformat_when_setting_is_not_set(self):
+        """Verify that new links are stamped automatically."""
+
+        mock_item = Mock()
+        mock_item.stamp = Mock(return_value=Stamp('abc123'))
+
+        mock_tree = MagicMock()
+        mock_tree.find_item = Mock(return_value=mock_item)
+        self.item.tree = mock_tree
+        self.item.links = [{'mock_uid': None}, {'mock_uid': None}]
+        self.item_validator.disable_get_issues_document()
+        self.assertTrue(self.item_validator.validate(self.item))
+        self.assertEqual('abc123', self.item.links[0].stamp)
+
+        # two calls:
+        # 1) setting up mock links with self.item.links above
+        # 2) calling item.review()
+        self.assertEqual(self.item._write.call_count, 2)
+
     def test_validate_nonnormative_with_links(self):
         """Verify a non-normative item with links can be checked."""
         self.item.normative = False

doorstop/core/validators/item_validator.py

@@ -158,35 +158,34 @@ def _get_issues_tree(self, item, tree):
         identifiers = set()
         for uid in item.links:
             try:
-                item = tree.find_item(uid)
+                parent = tree.find_item(uid)
             except DoorstopError:
                 identifiers.add(uid)  # keep the invalid UID
                 msg = "linked to unknown item: {}".format(uid)
                 yield DoorstopError(msg)
             else:
-                # check the linked item
-                if not item.active:
-                    msg = "linked to inactive item: {}".format(item)
+                # check the parent item
+                if not parent.active:
+                    msg = "linked to inactive item: {}".format(parent)
                     yield DoorstopInfo(msg)
-                if not item.normative:
-                    msg = "linked to non-normative item: {}".format(item)
+                if not parent.normative:
+                    msg = "linked to non-normative item: {}".format(parent)
                     yield DoorstopWarning(msg)
                 # check the link status
                 if uid.stamp == Stamp(True):
-                    uid.stamp = item.stamp()
+                    uid.stamp = parent.stamp()
                 elif not str(uid.stamp) and settings.STAMP_NEW_LINKS:
-                    uid.stamp = item.stamp()
-                elif uid.stamp != item.stamp():
+                    uid.stamp = parent.stamp()
+                elif uid.stamp != parent.stamp():
                     if settings.CHECK_SUSPECT_LINKS:
-                        msg = "suspect link: {}".format(item)
+                        msg = "suspect link: {}".format(parent)
                         yield DoorstopWarning(msg)
                 # reformat the item's UID
-                identifier2 = UID(item.uid, stamp=uid.stamp)
-                identifiers.add(identifier2)
+                identifiers.add(UID(parent.uid, stamp=uid.stamp))
 
         # Apply the reformatted item UIDs
         if settings.REFORMAT:
-            item.set_links(identifiers)
+            item.links = identifiers
 
     def _get_issues_both(self, item, document, tree, skip):
         """Yield all the item's issues against its document and tree."""