Skip to content

Rename all_pb_names to all_product_block_names and refactor related logic #968

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Rename all_pb_names to all_product_block_names and refactor related logic #968

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
17 changes: 8 additions & 9 deletions orchestrator/graphql/schemas/product.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,21 +52,20 @@
return await resolve_subscriptions(info, filter_by_with_related_subscriptions, sort_by, first, after)

@strawberry.field(description="Returns list of all nested productblock names") # type: ignore
async def all_pb_names(self) -> list[str]:

async def all_product_block_names(self) -> list[str]:
model = get_original_model(self, ProductTable)

def get_all_pb_names(product_blocks: list[ProductBlockTable]) -> Iterable[str]:
def get_names(product_blocks: list[ProductBlockTable], visited: set) -> Iterable[str]:
for product_block in product_blocks:
if product_block.product_block_id in visited:
continue

Check warning on line 61 in orchestrator/graphql/schemas/product.py

View check run for this annotation

Codecov / codecov/patch

orchestrator/graphql/schemas/product.py#L61 

Added line #L61 was not covered by tests
visited.add(product_block.product_block_id)
yield product_block.name

if product_block.depends_on:
yield from get_all_pb_names(product_block.depends_on)

names: list[str] = list(get_all_pb_names(model.product_blocks))
names.sort()
yield from get_names(product_block.depends_on, visited)

return names
names = set(get_names(model.product_blocks, set()))
return sorted(names)
Copy link
Contributor

Choose a reason for hiding this comment

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

Looking good!

There is a fixture test/unit_tests/fixtures/products/product_blocks/product_block_one_nested.py which has recursive product block relations.

Could you add a testcase using that fixture to test the recursion case?


@strawberry.field(description="Return product blocks") # type: ignore
async def product_blocks(self) -> list[Annotated["ProductBlock", strawberry.lazy(".product_block")]]:
Expand Down
4 changes: 2 additions & 2 deletions test/unit_tests/graphql/test_product.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ def get_all_product_names_query(
query ProductQuery($filterBy: [GraphqlFilter!]) {
products(filterBy: $filterBy) {
page {
allPbNames
allProductBlockNames
}
pageInfo {
endCursor
Expand Down Expand Up @@ -235,7 +235,7 @@ def test_all_product_block_names(test_client, generic_product_4):
result = response.json()
products_data = result["data"]["products"]
products = products_data["page"]
names = products[0]["allPbNames"]
names = products[0]["allProductBlockNames"]

assert len(names) == 2

Expand Down