Send revenue to jsecommerce #590
Conversation
shopelectro/models.py
Outdated
| Tag as caTag, | ||
| TagQuerySet as caTagQuerySet, | ||
| ) | ||
| from catalog import models as ca_models |
There was a problem hiding this comment.
let's call it catalog_models. We have not so many places to work with shortcut instead of full and clear word
shopelectro/models.py
Outdated
| class ProductQuerySet(ca_models.ProductQuerySet): | ||
|
|
||
| def calculate_revenue(self): | ||
| return self.aggregate(total_revenue=models.Sum('purchase_price'))['total_revenue'] |
There was a problem hiding this comment.
- it's not revenue, it's purchase price. Revenue is
cart.price - cart.purchase_price - QS getter methods have no verbs:
count,raw,firstand so on - so, i suggest name
total_purchase_price
shopelectro/views/ecommerce.py
Outdated
| order = Order.objects.all().prefetch_related('positions') | ||
|
|
||
| def get_context_data(self, **kwargs): | ||
| context = super().get_context_data(**kwargs) | ||
| positions = context['order'].positions.all() | ||
| total_revenue = Product.objects.filter( | ||
| id__in=[p.product_id for p in positions] |
There was a problem hiding this comment.
we should use position quantity in this calculation
shopelectro/models.py
Outdated
| print(position['price'], position['purchase_price']) | ||
| print(position['quantity']) | ||
| import sys | ||
| sys.stdout.flush() |
shopelectro/models.py
Outdated
| import sys | ||
| sys.stdout.flush() | ||
| price_diff = position['price'] - position['purchase_price'] | ||
| self.revenue += price_diff * position['quantity'] |
There was a problem hiding this comment.
several thoughts about revenue field:
- it's not clear to calc revenue field in
set_positionsmethod - it's not clear to keep revenue in DB. Because every order will have a state when this revenue was either set or not
- Cart is more good place to cal revenue: it focused only on positions and it's prices. Order = cart + credentials
So, i suggest to create Cart's revenue or total_revenue property: it already has total_price and total_quantity props
There was a problem hiding this comment.
it's not clear to calc revenue field in set_positions method
From set_positions docs: Save cart's state into Order instance.. May be it is not the best name for this method, but it falls into the semantic bounds
it's not clear to keep revenue in DB
We have to calculate a revenue from a cart and save it to db, because when we process a successful order, we will clear the cart, although we still need this revenue in next steps
Cart is more good place to cal revenue
We can calculate a revenue in the cart, but we still have to save it in an order.
I will move a calculation of revenue to Cart.revenue
P.S.: We would put a revenue to a session, but it seems, that this order's field may be helpful in some other cases too. What do you think?
There was a problem hiding this comment.
@artemiy312
We would put a revenue to a session
in this case Cart.clear method will clear the whole session instead of one revenue field.
It can be confusing state between real order sessions.
As of other points: you are right here. Seems set_positions is the best place to save revenue in our arch.
But semantically it's still not the best place in the world because of semantical binding between Cart.revenue and Cart.set_positions().
It's the problem in our arch:
- now we suggest that Cart.positions == CartSession (contains all it's data). It's not true
- we should create separated CartSession and models.Cart classes
- they should have methods CartSession.as_model and models.Cart.as_session
- after
cart_session.clear()call a bindedcart_modelshould preserve it's state
Create pdd task with the link on this comment plz. We can continue discussion in it's comments if it's needed
Closes #589