trash endpoint #332
trash endpoint #332
Conversation
|
Hi @carycheng, thanks for the pull request. Before we can merge it, we need you to sign our Contributor License Agreement. You can do so electronically here: http://opensource.box.com/cla Once you have signed, just add a comment to this pull request saying, "CLA signed". Thanks! |
Pull Request Test Coverage Report for Build 1107
💛 - Coveralls |
boxsdk/object/trash.py
Outdated
| class Trash(BaseEndpoint): | ||
| """Box API endpoint for performing trash related actions in Box.""" | ||
|
|
||
| def get_from_trash(self, item, fields=None): |
There was a problem hiding this comment.
This method should probably have a better name — client.trash().get_from_trash() stutters a bit
boxsdk/object/trash.py
Outdated
| Get item from trash. | ||
|
|
||
| :param item: | ||
| The :class:`File` or :class:`Folder` or :class:`WebLink` object to restore from trash. |
There was a problem hiding this comment.
This description is incorrect; the item is not restored by this method
boxsdk/object/trash.py
Outdated
| :param item: | ||
| The :class:`File` or :class:`Folder` or :class:`WebLink` object to restore from trash. | ||
| :type item: | ||
| :class:`File` or :class:`Folder` or :class:`WebLink`. |
There was a problem hiding this comment.
I think you could just do :class:`Item` here
boxsdk/object/trash.py
Outdated
| :returns: | ||
| A trashed :class:`File`, :class:`Folder`, or :class:`WebLink` object. | ||
| :rtype: | ||
| :class:`File`, :class:`Folder`, or :class:`WebLink` |
There was a problem hiding this comment.
This can also probably be :class:`Item`
test/unit/object/test_trash.py
Outdated
|
|
||
| def test_permanently_delete(test_item_and_response, test_trash, mock_box_session): | ||
| # pylint:disable=redefined-outer-name, protected-access | ||
| test_item, mock_item_response = test_item_and_response |
There was a problem hiding this comment.
This mock response probably isn't really much like what the API would return for this case; can we align with the correct response? It should have a 204 status code with no body
test/unit/object/test_trash.py
Outdated
| trashed_items = test_trash.get_trashed_items(fields=['name']) | ||
| trashed_item = trashed_items.next() | ||
| mock_box_session.get.assert_called_once_with(expected_url, params={'fields': 'name', 'offset': None}) | ||
| assert trashed_item['type'] == mock_trash['type'] |
There was a problem hiding this comment.
We should also assert about the class of the returned item
test/unit/object/test_trash.py
Outdated
| trashed_item = trashed_items.next() | ||
| mock_box_session.get.assert_called_once_with(expected_url, params={'fields': 'name', 'offset': None}) | ||
| assert trashed_item['type'] == mock_trash['type'] | ||
| assert trashed_item['id'] == mock_trash['id'] |
There was a problem hiding this comment.
This test doesn't really verify that the item works correctly; why does this test use ['id'] notation (which would pass even if the item never got translated) instead of .id (which requires that the item got translated)?
boxsdk/object/trash.py
Outdated
| class Trash(BaseEndpoint): | ||
| """Box API endpoint for performing trash related actions in Box.""" | ||
|
|
||
| def get_item_info(self, item, fields=None): |
There was a problem hiding this comment.
_info isn't really necessary in the name; most things that the SDK gets are "information"
No description provided.