-
Notifications
You must be signed in to change notification settings - Fork 42
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-399][externa] Meta methods & tests for setting layout #728
Conversation
PY-399 Set item layout
ex.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really good stuff John, you're taking previous input into account in new code nicely.
if not isinstance(layout, ItemLayout): | ||
try: | ||
layout = ItemLayout(**layout) | ||
except ValueError: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do these not throw ValidationError
, potentially as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice spot, they could indeed
Will except ValidationError as well in these
@@ -143,3 +144,25 @@ def archive(self) -> None: | |||
ids = [item.id for item in self] | |||
filters = {"item_ids": [str(item) for item in ids]} | |||
archive_list_of_items(self.client, team_slug, dataset_ids, filters) | |||
|
|||
def set_layout(self, layout: ItemLayout) -> None: | |||
if "team_slug" not in self.meta_params: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd maybe space out these individual if
s a bit, but that's a minor point!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
...actually a bit surprised that Black didn't tbh
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, good for readability
@@ -226,3 +226,44 @@ def test_archive_with_bad_team_slug(item: Item) -> None: | |||
with pytest.raises(AssertionError): | |||
item.meta_params["team_slug"] = 123 | |||
item.archive() | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These tests are bang on, nice one
Problem
Currently no Meta methods for setting item layout
Solution
Introducted functions to do this for Meta Items & ItemQuery objects
Changelog
Meta methods to set the layout of Items & ItemQuery objects