-
Notifications
You must be signed in to change notification settings - Fork 16
84 track orders #137
base: master
Are you sure you want to change the base?
84 track orders #137
Changes from 10 commits
a23c18b
7fb4863
ef79250
ce0cdbf
e402461
6b07d2e
6446078
6ccd627
ef54ea7
6c7f143
ba5daba
b8113f7
9499ff5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,61 @@ | ||
# -*- coding: utf-8 -*- | ||
# Generated by Django 1.10.2 on 2016-10-27 17:06 | ||
from __future__ import unicode_literals | ||
|
||
from decimal import Decimal | ||
from django.conf import settings | ||
from django.db import migrations, models | ||
import django.db.models.deletion | ||
import djmoney.models.fields | ||
|
||
|
||
class Migration(migrations.Migration): | ||
|
||
dependencies = [ | ||
migrations.swappable_dependency(settings.AUTH_USER_MODEL), | ||
('market', '0007_auto_20161012_1313'), | ||
] | ||
|
||
operations = [ | ||
migrations.CreateModel( | ||
name='PaymentItem', | ||
fields=[ | ||
('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), | ||
('name', models.CharField(max_length=512, verbose_name='product_name')), | ||
('price_currency', djmoney.models.fields.CurrencyField(choices=[('AFN', 'Afghani'), ('DZD', 'Algerian Dinar'), ('ARS', 'Argentine Peso'), ('AMD', 'Armenian Dram'), ('AWG', 'Aruban Guilder'), ('AUD', 'Australian Dollar'), ('AZN', 'Azerbaijanian Manat'), ('BSD', 'Bahamian Dollar'), ('BHD', 'Bahraini Dinar'), ('THB', 'Baht'), ('BBD', 'Barbados Dollar'), ('BYR', 'Belarussian Ruble'), ('BZD', 'Belize Dollar'), ('BMD', 'Bermudian Dollar (customarily known as Bermuda Dollar)'), ('BTN', 'Bhutanese ngultrum'), ('VEF', 'Bolivar Fuerte'), ('XBA', 'Bond Markets Units European Composite Unit (EURCO)'), ('BRL', 'Brazilian Real'), ('BND', 'Brunei Dollar'), ('BGN', 'Bulgarian Lev'), ('BIF', 'Burundi Franc'), ('XOF', 'CFA Franc BCEAO'), ('XAF', 'CFA franc BEAC'), ('XPF', 'CFP Franc'), ('CAD', 'Canadian Dollar'), ('CVE', 'Cape Verde Escudo'), ('KYD', 'Cayman Islands Dollar'), ('CLP', 'Chilean peso'), ('XTS', 'Codes specifically reserved for testing purposes'), ('COP', 'Colombian peso'), ('KMF', 'Comoro Franc'), ('CDF', 'Congolese franc'), ('BAM', 'Convertible Marks'), ('NIO', 'Cordoba Oro'), ('CRC', 'Costa Rican Colon'), ('HRK', 'Croatian Kuna'), ('CUP', 'Cuban Peso'), ('CUC', 'Cuban convertible peso'), ('CZK', 'Czech Koruna'), ('GMD', 'Dalasi'), ('DKK', 'Danish Krone'), ('MKD', 'Denar'), ('DJF', 'Djibouti Franc'), ('STD', 'Dobra'), ('DOP', 'Dominican Peso'), ('VND', 'Dong'), ('XCD', 'East Caribbean Dollar'), ('EGP', 'Egyptian Pound'), ('ETB', 'Ethiopian Birr'), ('EUR', 'Euro'), ('XBB', 'European Monetary Unit (E.M.U.-6)'), ('XBD', 'European Unit of Account 17(E.U.A.-17)'), ('XBC', 'European Unit of Account 9(E.U.A.-9)'), ('FKP', 'Falkland Islands Pound'), ('FJD', 'Fiji Dollar'), ('HUF', 'Forint'), ('GHS', 'Ghana Cedi'), ('GIP', 'Gibraltar Pound'), ('XAU', 'Gold'), ('XFO', 'Gold-Franc'), ('PYG', 'Guarani'), ('GNF', 'Guinea Franc'), ('GYD', 'Guyana Dollar'), ('HTG', 'Haitian gourde'), ('HKD', 'Hong Kong Dollar'), ('UAH', 'Hryvnia'), ('ISK', 'Iceland Krona'), ('INR', 'Indian Rupee'), ('IRR', 'Iranian Rial'), ('IQD', 'Iraqi Dinar'), ('IMP', 'Isle of Man pount'), ('JMD', 'Jamaican Dollar'), ('JOD', 'Jordanian Dinar'), ('KES', 'Kenyan Shilling'), ('PGK', 'Kina'), ('LAK', 'Kip'), ('KWD', 'Kuwaiti Dinar'), ('AOA', 'Kwanza'), ('MMK', 'Kyat'), ('GEL', 'Lari'), ('LVL', 'Latvian Lats'), ('LBP', 'Lebanese Pound'), ('ALL', 'Lek'), ('HNL', 'Lempira'), ('SLL', 'Leone'), ('LSL', 'Lesotho loti'), ('LRD', 'Liberian Dollar'), ('LYD', 'Libyan Dinar'), ('SZL', 'Lilangeni'), ('LTL', 'Lithuanian Litas'), ('MGA', 'Malagasy Ariary'), ('MWK', 'Malawian Kwacha'), ('MYR', 'Malaysian Ringgit'), ('TMM', 'Manat'), ('MUR', 'Mauritius Rupee'), ('MZN', 'Metical'), ('MXN', 'Mexican peso'), ('MDL', 'Moldovan Leu'), ('MAD', 'Moroccan Dirham'), ('NGN', 'Naira'), ('ERN', 'Nakfa'), ('NAD', 'Namibian Dollar'), ('NPR', 'Nepalese Rupee'), ('ANG', 'Netherlands Antillian Guilder'), ('ILS', 'New Israeli Sheqel'), ('RON', 'New Leu'), ('TWD', 'New Taiwan Dollar'), ('NZD', 'New Zealand Dollar'), ('KPW', 'North Korean Won'), ('NOK', 'Norwegian Krone'), ('PEN', 'Nuevo Sol'), ('MRO', 'Ouguiya'), ('TOP', 'Paanga'), ('PKR', 'Pakistan Rupee'), ('XPD', 'Palladium'), ('MOP', 'Pataca'), ('PHP', 'Philippine Peso'), ('XPT', 'Platinum'), ('GBP', 'Pound Sterling'), ('BWP', 'Pula'), ('QAR', 'Qatari Rial'), ('GTQ', 'Quetzal'), ('ZAR', 'Rand'), ('OMR', 'Rial Omani'), ('KHR', 'Riel'), ('MVR', 'Rufiyaa'), ('IDR', 'Rupiah'), ('RUB', 'Russian Ruble'), ('RWF', 'Rwanda Franc'), ('XDR', 'SDR'), ('SHP', 'Saint Helena Pound'), ('SAR', 'Saudi Riyal'), ('RSD', 'Serbian Dinar'), ('SCR', 'Seychelles Rupee'), ('XAG', 'Silver'), ('SGD', 'Singapore Dollar'), ('SBD', 'Solomon Islands Dollar'), ('KGS', 'Som'), ('SOS', 'Somali Shilling'), ('TJS', 'Somoni'), ('LKR', 'Sri Lanka Rupee'), ('SDG', 'Sudanese Pound'), ('SRD', 'Surinam Dollar'), ('SEK', 'Swedish Krona'), ('CHF', 'Swiss Franc'), ('SYP', 'Syrian Pound'), ('BDT', 'Taka'), ('WST', 'Tala'), ('TZS', 'Tanzanian Shilling'), ('KZT', 'Tenge'), ('TTD', 'Trinidad and Tobago Dollar'), ('MNT', 'Tugrik'), ('TND', 'Tunisian Dinar'), ('TRY', 'Turkish Lira'), ('TVD', 'Tuvalu dollar'), ('AED', 'UAE Dirham'), ('XFU', 'UIC-Franc'), ('USD', 'US Dollar'), ('UGX', 'Uganda Shilling'), ('UYU', 'Uruguayan peso'), ('UZS', 'Uzbekistan Sum'), ('VUV', 'Vatu'), ('KRW', 'Won'), ('YER', 'Yemeni Rial'), ('JPY', 'Yen'), ('CNY', 'Yuan Renminbi'), ('ZMK', 'Zambian Kwacha'), ('ZMW', 'Zambian Kwacha'), ('ZWD', 'Zimbabwe Dollar A/06'), ('ZWN', 'Zimbabwe dollar A/08'), ('ZWL', 'Zimbabwe dollar A/09'), ('PLN', 'Zloty')], default='DKK', editable=False, max_length=3)), | ||
('quantity', models.PositiveSmallIntegerField(default=1)), | ||
('price', djmoney.models.fields.MoneyField(decimal_places=2, default=Decimal('0.0'), max_digits=12, verbose_name='price')), | ||
('delivery_date', models.DateField(null=True)), | ||
], | ||
), | ||
migrations.AddField( | ||
model_name='payment', | ||
name='basket', | ||
field=models.ForeignKey(default=None, editable=False, on_delete=django.db.models.deletion.CASCADE, to='market.Basket'), | ||
), | ||
migrations.AddField( | ||
model_name='payment', | ||
name='extra', | ||
field=models.CharField(blank=True, default=None, max_length=512, null=True, verbose_name='extra information about the payment'), | ||
), | ||
migrations.AddField( | ||
model_name='payment', | ||
name='status', | ||
field=models.CharField(choices=[('unpaid', 'unpaid'), ('paid-cash', 'paid-cash'), ('paid-mobile', 'paid-mobile'), ('cancelled', 'cancelled')], default='unpaid', max_length=15, null=True), | ||
), | ||
migrations.AddField( | ||
model_name='payment', | ||
name='user', | ||
field=models.ForeignKey(default=None, editable=False, null=True, on_delete=django.db.models.deletion.CASCADE, to=settings.AUTH_USER_MODEL), | ||
), | ||
migrations.AlterField( | ||
model_name='payment', | ||
name='account', | ||
field=models.ForeignKey(null=True, on_delete=django.db.models.deletion.CASCADE, to='accounts.Account'), | ||
), | ||
migrations.AddField( | ||
model_name='paymentitem', | ||
name='payment', | ||
field=models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='items', to='market.Payment'), | ||
), | ||
] |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,10 @@ | ||
from decimal import Decimal | ||
|
||
from django.db import models, transaction | ||
from .payment import Payment, PaymentItem | ||
from django.db.models import Sum, F, Count, Max | ||
from django.core.exceptions import ValidationError | ||
from moneyed import Money | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The sorting is off, try doing this:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My bad, I expected that it works as pre commit hook. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When installed, it will fire, but you have to run |
||
from django.utils import timezone | ||
|
||
|
||
|
@@ -39,10 +43,13 @@ def __str__(self): | |
return 'Basket {} {} {}'.format(self.user, self.status, self.created) | ||
|
||
def add_to_items(self, product=None, quantity=1, delivery_date=None): | ||
current = self.items.filter(product=product, | ||
delivery_date=delivery_date) | ||
if current.exists(): | ||
current.update(quantity=models.F('quantity') + quantity) | ||
# TODO: it should not be possible to add items with different currencies | ||
|
||
current = self.items.filter(product=product, delivery_date=delivery_date).first() | ||
|
||
if current: | ||
current.quantity += quantity | ||
current.save() | ||
else: | ||
BasketItem.objects.create( | ||
basket=self, | ||
|
@@ -53,35 +60,49 @@ def add_to_items(self, product=None, quantity=1, delivery_date=None): | |
|
||
def remove_from_items(self, product=None, quantity=1, delivery_date=None): | ||
current = self.items.filter(product=product, | ||
delivery_date=delivery_date) | ||
if current.exists(): | ||
if current[0].quantity - quantity > 1: | ||
current.update(quantity=models.F('quantity') - quantity) | ||
delivery_date=delivery_date).first() | ||
if current: | ||
if current.quantity - quantity > 1: | ||
current.quantity -= quantity | ||
current.save() | ||
else: | ||
self.items.filter(basket=self, product=product, delivery_date=delivery_date).delete() | ||
|
||
def get_total_amount(self): | ||
# TODO: This should return a Money type instead of decimal | ||
# TODO: When calculating this, we should fail when currencies differ | ||
total = Decimal('0') | ||
for item in self.items.all(): | ||
total += item.quantity * item.product.price.amount | ||
return total | ||
|
||
# FIXME this check should be removed, it is better to check for the currenices when an item is added | ||
# sanity check for the currencies | ||
|
||
if 1 != self.items.all().values('product__price_currency').annotate(total=Count('product__price_currency')).count(): | ||
raise ValidationError('The products have different currencies') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If the count() is 0, the You are right that it should be fired another place. |
||
|
||
total = self.items.all().aggregate(total=Sum(F('product__price') * F('quantity')), cur=Max('product__price_currency')) | ||
return Money(total['total'], total['cur']) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This doesn't work for me, I get the following:
I think it has to do with the I'm guessing it somehow works for you? Are you running Python 3.4 or 3.5? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Strange, I tested it on sqlite and it worked. It looks like a difference between psql and sqlite in types. I will rewrite it as a loop. With pre-fetching and selecting related data the performance would be almost the same, more readable and work on every database engine. Well it works for me, and it generates a correct query: And I use python 3.4.5 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, there shouldn't be any real performance concerns as the contents of a basket are quite limited :) |
||
|
||
def get_items_count(self): | ||
return self.items.all().count() | ||
|
||
@transaction.atomic | ||
def do_checkout(self): | ||
# TODO: Why on earth are we importing this here | ||
from .payment import Payment | ||
Payment.objects.create( | ||
payment = Payment.objects.create( | ||
amount=self.get_total_amount(), | ||
# FIXME: get valid account | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This means that we don't know the account that pays at the moment of checking out and creating the payment? Maybe if we start by assuming users always have exactly 1 account and always use that? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Well in ideal case: a user should be created with a personal account (better name would be organization). It should be a trigger and account model should have one more field - personal. And somewhere in navigation bar he can select which account is he using (probably a session variable). Basket should be based on account_id. This would allow to share the basket for the whole organization. Anyway it has nothing to do with what I have done. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sounds interesting to add a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It would add more consistency to the shopping logic. Everything is based on the When we talk about shopping logic the payment model it is not optimal as well. It should be called an There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are you suggesting a separate issue for this? I can create the issue and try to describe it... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah the issue is quite complex and has noting to do with the payment the logic. The issue could have following sub-task:
Link to the payments:
Also #130 should be removed - it doesn’t solve the problem. It is up to consideration if this should be part of the MVP. What do you think? Also the issues should be revisited. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think the account needs to be a personal account, as long as all users have at least 1 I'll create a new issue, thanks for fleshing it out!! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Moved to #139 |
||
user=self.user, | ||
basket=self | ||
) | ||
# self.order = payment | ||
self.status = self.CHECKEDOUT | ||
|
||
for item in self.items.all(): | ||
PaymentItem.objects.create( | ||
payment=payment, | ||
name=item.product.title, | ||
price=item.product.price, | ||
quantity=item.quantity, | ||
delivery_date=item.delivery_date | ||
) | ||
|
||
self.save() | ||
return payment.id | ||
|
||
|
||
class BasketItem(models.Model): | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,63 +2,69 @@ | |
{% load bootstrap3 %} | ||
{% load i18n %} | ||
{% block content %} | ||
|
||
<div class="row"> | ||
<div class="col-md-12 col-span-3"> | ||
<h3 class="page-header">{% trans 'Your cart' %}</h3> | ||
<div class="row"> | ||
<div class="col-md-12 col-span-3"> | ||
<h3 class="page-header">{% trans 'Your cart' %}</h3> | ||
</div> | ||
</div> | ||
{% if items %} | ||
<div class="row"> | ||
<div class="col-md-3"><strong>Product</strong></div> | ||
<div class="col-md-3"><strong>Delivery date</strong></div> | ||
<div class="col-md-3"><strong>Delivered to:</strong></div> | ||
<div class="col-md-1"><strong>Amount</strong></div> | ||
<div class="col-md-2"><strong>Price</strong></div> | ||
</div> | ||
</div> | ||
|
||
<div class="row"> | ||
<div class="col-md-3"><strong>Product</strong></div> | ||
<div class="col-md-3"><strong>Delivery date</strong></div> | ||
<div class="col-md-3"><strong>Delivered to:</strong></div> | ||
<div class="col-md-3"><strong>Price</strong></div> | ||
</div> | ||
|
||
{% for item in items %} | ||
<div class="row"> | ||
<div class="col-md-3 text-left">{{item.product.title}}</div> | ||
<div class="col-md-3">{{item.delivery_date|date:"d/m - Y"|default:"-"}}</div> | ||
<div class="col-md-3">{{item.delivered_to}}</div> | ||
<div class="col-md-3">{{item.product.price}} DKK</div> | ||
</div> | ||
{% endfor %} | ||
|
||
<div class="row"> | ||
<hr> | ||
<div class="col-md-4 text-left"> | ||
<strong>Total</strong> | ||
{% for item in items %} | ||
<div class="row"> | ||
<div class="col-md-3 text-left">{{item.product.title}}</div> | ||
<div class="col-md-3">{{item.delivery_date|date:"d/m - Y"|default:"-"}}</div> | ||
<div class="col-md-3">{{item.delivered_to}}</div> | ||
<div class="col-md-1">{{item.quantity}}</div> | ||
<div class="col-md-2">{{item.product.price}}</div> | ||
</div> | ||
<div class="col-md-4 text-right"> | ||
<strong>{{basket.get_total_amount}}</strong> | ||
{% endfor %} | ||
|
||
<div class="row"> | ||
<hr> | ||
<div class="col-md-10 text-left"> | ||
<strong>Total</strong> | ||
</div> | ||
<div class="col-md-2"> | ||
<strong>{{basket.get_total_amount}}</strong> | ||
</div> | ||
</div> | ||
</div> | ||
|
||
<hr /> | ||
<hr /> | ||
<div class="row"> | ||
<div class="col-md-12 text-left"> | ||
<p>The above is your order. Please look it through it includes the items | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This text (and the previous text) needs to be marked for translation There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Well I didn’t create the most of the text and I hoped that someone is in charge of it. I just proposed in some cases how it could look like. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can also open up a separate issue about being consequential with translation throughout the codebase... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Opened #138 for this so we can get on with the PR :) |
||
you want to buy. When you click "Go to payment" below, you will be sent | ||
to a secure page at ePay.dk, where you will enter your payment information | ||
and accept the payment.</p> | ||
{% comment %} | ||
<p>We accept the following payment methods: Dankort, Visa/Dankort.</p> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fine to just delete this :) |
||
{% endcomment %} | ||
|
||
<div class="row"> | ||
<div class="col-md-8 text-left"> | ||
<p>The above is your order. Please look it through it includes the items | ||
you want to buy. When you click "Go to payment" below, you will be sent | ||
to a secure page at ePay.dk, where you will enter your payment information | ||
and accept the payment.</p> | ||
<p>We accept the following payment methods: Dankort, Visa/Dankort.</p> | ||
</div> | ||
</div> | ||
</div> | ||
<br /> | ||
<br /> | ||
<br /> | ||
<div class="row"> | ||
<div class="col-md-4 text-left"> | ||
<a class="btn btn-success" href="{% url 'eggplant:market:market_home' %}" >Go to payments</a> | ||
</div> | ||
<div class="col-md-4 text-right"> | ||
<form action="{% url 'eggplant:market:checkout' %}" method="POST"> | ||
{% csrf_token %} | ||
<button name="submit" class="btn btn-success">Go to Payment</button> | ||
</form> | ||
</div> | ||
</div> | ||
|
||
<br /> | ||
<br /> | ||
<br /> | ||
<div class="row"> | ||
<div class="col-md-6 text-left"> | ||
<a class="btn btn-success" href="{% url 'eggplant:market:market_home' %}" >Go to Market</a> | ||
</div> | ||
<div class="col-md-5 text-right"> | ||
<form action="{% url 'eggplant:market:checkout' %}" method="POST"> | ||
{% csrf_token %} | ||
<button name="submit" class="btn btn-info" onclick="return confirm('Please confirm the Order?')">Pay Cash</button> | ||
</form> | ||
</div> | ||
</div> | ||
{% else %} | ||
<p>No items in the Basket</p> | ||
{% endif %} | ||
{% endblock%} |
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.
oh nice, did you add this manually?
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.
This was done autocratically