#880 Create CatalogBlock model #925
Conversation
shopelectro/models.py
Outdated
|
|
||
|
|
||
| class CatalogBlock(models.Model): | ||
| """Catalog block is an element of catalog matrix UI element.""" |
There was a problem hiding this comment.
| """Catalog block is an element of catalog matrix UI element.""" | |
| """Catalog block is an UI element of catalog matrix.""" |
shopelectro/models.py
Outdated
| ) | ||
|
|
||
|
|
||
| class CatalogBlock(models.Model): |
There was a problem hiding this comment.
MatrixBlock or CatalogMatrixBlock.
It will be used only for Matrix page which is always for Catalog.
If you'll rename it, keep in mind CatalogBlockManager and the name in tests
shopelectro/models.py
Outdated
| def url(self): | ||
| self.category.url | ||
|
|
||
| def rows(self): |
There was a problem hiding this comment.
| def rows(self): | |
| def rows(self) -> CategoryQuerySet: |
shopelectro/models.py
Outdated
| class CatalogBlock(models.Model): | ||
| """Catalog block is an element of catalog matrix UI element.""" | ||
|
|
||
| # @todo #880:60m Add it to the admin panel, use it in the matrix view. |
There was a problem hiding this comment.
-
Add it->Add CatalogBlock. It'll be header in the long tasks list. -
Here we have two pretty different tasks:
- Use CatalogBlock at the matrix view
- Add CatalogBlock to the admin panel
- And the first one is for 60m including tests, not for 30m. But it's up to you.
- CategoryBlock should be inline on Category Edit page on the admin panel
| def test_get_catalog_blocks(self): | ||
| """Perform four queries to fetch in batch blocks, categories, pages and category's children.""" | ||
| roots = Category.objects.filter(level=0) | ||
| roots_count = roots.count() |
There was a problem hiding this comment.
we can use roots.count() directly, without var
There was a problem hiding this comment.
It'll bring an additional query to assertNumQueries(4) scope, that doesn't related to the test
shopelectro/tests/tests_models.py
Outdated
| @@ -86,3 +86,32 @@ def test_get_brands_from_products(self): | |||
| p: p.tags.filter(group__name=settings.BRAND_TAG_GROUP_NAME).first() | |||
| for p in products | |||
| } | |||
|
|
|||
| def test_get_catalog_blocks(self): | |||
There was a problem hiding this comment.
| def test_get_catalog_blocks(self): | |
| def test_get_catalog_blocks_queries(self): |
The name will able us to get rid from pydoc. It just doubles code lines now
There was a problem hiding this comment.
It is already in a relevant class QueryQuantities, so I'll remove the comment
shopelectro/tests/tests_models.py
Outdated
| for category in roots: | ||
| CatalogBlock.objects.create(category=category) | ||
|
|
||
| with self.assertNumQueries(4): |
There was a problem hiding this comment.
This structure would be more clear. Up to you
with self.assertNumQueries(1):
# fetch blocks
with self.assertNumQueries(3):
# fetch category, page, rows
There was a problem hiding this comment.
Looks better, but blocks can't be fetched in 1 query, because of select_related('category', 'cateogyr__page') and rows takes 2 queries, so this structure be more relevant:
with self.assertNumQueries(2):
# fetch blocks, category, page
with self.assertNumQueries(2):
# fetch rows
P.S.: To make clear the point about select_related
select_related gets the related objects in the same database query
shopelectro/tests/tests_models.py
Outdated
| block = CatalogBlock.objects.create(category=first) | ||
| sized_block = CatalogBlock.objects.create(category=second, block_size=2) | ||
|
|
||
| self.assertNotEquals(first.children.active(), len(sized_block.rows())) |
There was a problem hiding this comment.
you are comparing QS with number here. Let me suggest what you meaned
| self.assertNotEquals(first.children.active(), len(sized_block.rows())) | |
| self.assertEquals(first.children.active().count(), sized_block.rows().count()) |
There was a problem hiding this comment.
Sorry for the mess. I have reworked the test
|
@artemiy312 , i guess your code contains too many work for 1h. I recommend you to increase task price to 2h |
Closes #880