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

[PY-386][external] Core method to move items between folders & tests #709

Merged
merged 3 commits into from
Nov 9, 2023

Conversation

JBWilkie
Copy link
Collaborator

@JBWilkie JBWilkie commented Nov 7, 2023

Problem

Currently no way to move items between folders in darwin.future

Solution

Define the core method to move a list of items by their UUIDs by using the path endpoint

Changelog

Added core method to move items between folders

Copy link

linear bot commented Nov 7, 2023

PY-386 Core: Edit items: move to folder

https://www.notion.so/v7labs/Item-Management-Functionality-Scoping-df5f846a135e46e1bf31c473afda32ee#c93dd9600c9b47b394df6660276d490d

move_item(client, id, folder)
# client: Client
# id: str, validatable as UUID
# layout: pathlike string

Comment on lines +243 to +244
if response.status_code == 400:
raise BadRequest(response)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As covered in my comment in PY-386 here, we probably want to catch the 400 errors this endpoint can throw in Core

To do this, I introduced the BadRequest class. This broke some existing tests that returned 400s, as previously they were expecting HTTPError, so updated each breaking test to expect BadRequest instead



def test_move_list_of_items_to_folder_with_error_response() -> None:
api_client = Mock(spec=ClientCore)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They both achieve the same thing but for consistency can you move this to a responses.RequestMock approach

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense - There are a few other tests written this way, I'll update them as well

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Nathanjp91 introduced the use of responses on this stuff, but it has become the paradigm overall, and it's a good example of socialised testing when used well.

@JBWilkie JBWilkie mentioned this pull request Nov 8, 2023


def test_move_list_of_items_to_folder_with_error_response() -> None:
api_client = Mock(spec=ClientCore)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Nathanjp91 introduced the use of responses on this stuff, but it has become the paradigm overall, and it's a good example of socialised testing when used well.

@JBWilkie JBWilkie merged commit 05843d9 into master Nov 9, 2023
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants