#183 object compositions #194
Conversation
ArtemijRodionov
force-pushed
the
183-object-compositions
branch
5 times, most recently
from
cd9452d to
95f952d
Compare
ArtemijRodionov
force-pushed
the
183-object-compositions
branch
from
95f952d to
9e99cf1
Compare
catalog/context/__init__.py
Outdated
| from .products import Products | ||
| from .tags import Tags | ||
|
|
||
| # @todo #183:60m Move context package to separate django app. |
There was a problem hiding this comment.
i think context should not be in separated django app. As we don't put all views or templates to separated apps
catalog/context/products.py
Outdated
|
|
||
| class Products(ModelContext): | ||
|
|
||
| def __init__(self, qs: QuerySet): |
There was a problem hiding this comment.
every context should be able to receive another context
There was a problem hiding this comment.
The interface does not require it. A context implementation may have any constructor
There was a problem hiding this comment.
@artemiy312 , but we should be able to chain any context to any.
They may have no common methods, but theirs dicts should be merged.
There is initial idea: merge dicts with simple chaining
There was a problem hiding this comment.
@artemiy312 , y, it's much more good decision, then receiving contexts
catalog/context/tags.py
Outdated
| } | ||
|
|
||
|
|
||
| class TagsByRaw(Tags): |
There was a problem hiding this comment.
it's not clear what is raw. TagsDeserialised or docstring with comment like Deserialize tags from string
catalog/context/tags.py
Outdated
| return self._qs.filter(slug__in=slugs) | ||
|
|
||
|
|
||
| class GroupTagsByProducts(Tags): |
There was a problem hiding this comment.
Group is verb, but we should start class name from noun (or create another convention about it).
So, i suggest TagsGroupedByProducts
catalog/context/tags.py
Outdated
| class Checked404Tags(Tags): | ||
|
|
||
| def qs(self): | ||
| tags = self.qs() |
There was a problem hiding this comment.
self.super.qs() or smth. Now it's infinity recursion
catalog/models.py
Outdated
| @@ -95,6 +95,7 @@ class ProductQuerySet(models.QuerySet): | |||
| def get_offset(self, start, step): | |||
| return self[start:start + step] | |||
|
|
|||
| # @todo #183:60m Try to remove `ordering` arg from ProductQuerySet.get_by_category | |||
There was a problem hiding this comment.
it's about 30 min i think. We should move it from model to context.
And use ready context arch for it
ArtemijRodionov
force-pushed
the
183-object-compositions
branch
2 times, most recently
from
d0cb865 to
e5e79aa
Compare
ArtemijRodionov
force-pushed
the
183-object-compositions
branch
2 times, most recently
from
71b896d to
9e48046
Compare
|
|
||
| def context(self): | ||
| return collections.ChainMap( | ||
| *[ctx.context() for ctx in self.contexts] |
There was a problem hiding this comment.
| *[ctx.context() for ctx in self.contexts] | |
| iter(ctx.context() for ctx in self.contexts) |
There was a problem hiding this comment.
I don't see the point. How iter can help here? ChainMap takes variadic dict as arguments, so i have to unpack contexts list
catalog/context/products.py
Outdated
|
|
||
| class Products(ModelContext): | ||
|
|
||
| def __init__(self, qs: QuerySet): |
There was a problem hiding this comment.
@artemiy312 , y, it's much more good decision, then receiving contexts
ArtemijRodionov
force-pushed
the
183-object-compositions
branch
from
9e48046 to
a183f79
Compare
Closes #183