Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ability to remove inactive items #619

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion doorstop/cli/commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ def run_remove(args, cwd, _, catch=True):
with utilities.capture(catch=catch) as success:
# get the item
tree = _get_tree(args, cwd)
item = tree.find_item(args.uid)
item = tree.find_item(args.uid, only_active=False)

# delete it
item.delete()
Expand Down
12 changes: 10 additions & 2 deletions doorstop/cli/tests/test_all.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@
from doorstop.core.builder import _clear_tree
from doorstop.core.document import Document

REQ_COUNT = 23
ALL_COUNT = 55
REQ_COUNT = 24
ALL_COUNT = 56


class TempTestCase(unittest.TestCase):
Expand Down Expand Up @@ -218,18 +218,26 @@ class TestRemove(unittest.TestCase):
"""Integration tests for the 'doorstop remove' command."""

ITEM = os.path.join(TUTORIAL, "TUT003.yml")
INACTIVE_ITEM = os.path.join(TUTORIAL, "TUT026.yml")

def setUp(self):
self.backup = common.read_text(self.ITEM)
self.inactive_backup = common.read_text(self.INACTIVE_ITEM)

def tearDown(self):
common.write_text(self.backup, self.ITEM)
common.write_text(self.inactive_backup, self.INACTIVE_ITEM)

def test_remove(self):
"""Verify 'doorstop remove' can be called."""
self.assertIs(None, main(["remove", "tut3"]))
self.assertFalse(os.path.exists(self.ITEM))

def test_remove_inactive(self):
"""Verify 'doorstop remove' can remove inactive"""
self.assertIs(None, main(["remove", "tut26"]))
self.assertFalse(os.path.exists(self.INACTIVE_ITEM))

def test_remove_error(self):
"""Verify 'doorstop remove' returns an error on unknown item UIDs."""
self.assertRaises(SystemExit, main, ["remove", "tut9999"])
Expand Down
11 changes: 8 additions & 3 deletions doorstop/core/builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -94,10 +94,15 @@ def find_document(prefix):
return document


def find_item(uid):
"""Find an item without an explicitly building a tree."""
def find_item(uid, only_active=True):
"""Find an item without an explicitly building a tree.

:param uid: UID
:param only_active: Returns only active items

"""
tree = _get_tree()
item = tree.find_item(uid)
item = tree.find_item(uid, only_active=only_active)
return item


Expand Down
7 changes: 5 additions & 2 deletions doorstop/core/document.py
Original file line number Diff line number Diff line change
Expand Up @@ -562,7 +562,7 @@ def remove_item(self, value, reorder=True):

"""
uid = UID(value)
item = self.find_item(uid)
item = self.find_item(uid, only_active=False)
item.delete()
if reorder:
self.reorder()
Expand Down Expand Up @@ -780,10 +780,11 @@ def _items_by_level(items, keep=None):
for item in items_at_level:
yield level, item

def find_item(self, value, _kind=""):
def find_item(self, value, only_active=True, _kind=""):
"""Return an item by its UID.

:param value: item or UID
:param only_active: Returns only active items

:raises: :class:`~doorstop.common.DoorstopError` if the item
cannot be found
Expand All @@ -798,6 +799,8 @@ def find_item(self, value, _kind=""):
return item
else:
log.trace("item is inactive: {}".format(item)) # type: ignore
if not only_active:
return item

raise DoorstopError("no matching{} UID: {}".format(_kind, uid))

Expand Down
2 changes: 1 addition & 1 deletion doorstop/core/tests/test_builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ def test_find_item(self, mock_find_item):
_clear_tree()
uid = "req1"
find_item(uid)
mock_find_item.assert_called_once_with(uid)
mock_find_item.assert_called_once_with(uid, only_active=True)

def test_tree_finds_documents(self):
"""Verify items can be found using a convenience function."""
Expand Down
9 changes: 9 additions & 0 deletions doorstop/core/tests/test_document.py
Original file line number Diff line number Diff line change
Expand Up @@ -580,6 +580,15 @@ def test_remove_item(self, mock_remove, mock_reorder):
mock_reorder.assert_called_once_with(self.document.items, keep=None, start=None)
mock_remove.assert_called_once_with(item.path)

@patch("doorstop.core.document.Document._reorder_automatic")
@patch("os.remove")
def test_remove_inactive_item(self, mock_remove, mock_reorder):
"""Verify an item can be removed."""
with patch("doorstop.settings.REORDER", True):
item = self.document.remove_item("REQ005")
mock_reorder.assert_called_once_with(self.document.items, keep=None, start=None)
mock_remove.assert_called_once_with(item.path)

@patch("os.remove")
def test_remove_item_contains(self, mock_remove):
"""Verify a removed item is not contained in the document."""
Expand Down
7 changes: 7 additions & 0 deletions doorstop/core/tests/test_tree.py
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,13 @@ def test_remove_item(self, mock_delete):
self.tree.remove_item("req1", reorder=False)
mock_delete.assert_called_once_with()

@patch("doorstop.settings.REORDER", False)
@patch("doorstop.core.item.Item.delete")
def test_remove_inactive_item(self, mock_delete):
"""Verify an item can be removed from a document."""
self.tree.remove_item("req5", reorder=False)
mock_delete.assert_called_once_with()

def test_remove_item_unknown_item(self):
"""Verify an exception is raised removing an unknown item."""
self.assertRaises(DoorstopError, self.tree.remove_item, "req9999")
Expand Down
11 changes: 8 additions & 3 deletions doorstop/core/tree.py
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,7 @@ def remove_item(self, value, reorder=True):
uid = UID(value)
for document in self:
try:
document.find_item(uid)
document.find_item(uid, only_active=False)
except DoorstopError:
pass # item not found in that document
else:
Expand Down Expand Up @@ -415,10 +415,11 @@ def find_document(self, value) -> Document:

raise DoorstopError(Prefix.UNKNOWN_MESSAGE.format(prefix))

def find_item(self, value, _kind=""):
def find_item(self, value, only_active=True, _kind=""):
"""Get an item by its UID.

:param value: item or UID
:param only_active: Returns only active items

:raises: :class:`~doorstop.common.DoorstopError` if the item
cannot be found
Expand All @@ -437,12 +438,14 @@ def find_item(self, value, _kind=""):
return item
else:
log.trace("item is inactive: {}".format(item)) # type: ignore
if not only_active:
return item
else:
log.trace("found cached unknown: {}".format(uid)) # type: ignore
except KeyError:
for document in self:
try:
item = document.find_item(uid, _kind=_kind)
item = document.find_item(uid, only_active=only_active, _kind=_kind)
except DoorstopError:
pass # item not found in that document
else:
Expand All @@ -454,6 +457,8 @@ def find_item(self, value, _kind=""):
return item
else:
log.trace("item is inactive: {}".format(item)) # type: ignore
if not only_active:
return item

log.debug("could not find item: {}".format(uid))
if settings.CACHE_ITEMS:
Expand Down
10 changes: 10 additions & 0 deletions reqs/tutorial/TUT026.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
active: true
derived: false
header: ''
level: 5.2
links: []
normative: true
ref: ''
reviewed: jQyhY-PAEhuHr8zx7e2HRBUZACSJNbxvhAagi90VYw8=
text: |
An inactive requirement.
Loading