CGM_Public/pretix_cgo
1
0
Fork 0
forked from CGM_Public/pretix_original
Code Pull Requests Actions Activity

Refactor cancelling positions and orders in the data model (#1088)

Browse Source 
- [x] Data model
- [x] display in order view in backend
- [x] review all usages of OrderPositions.objects
- [x] review all usages of order.positions
- [x] review all other model usages
- [x] review plugins
- [x] plugins backwards-compatible API?
- [x] decide on way forward for REST API
- [x] need to cancel fees
- [x] tests
- [ ] plugins
  - [ ] gdpr
  - [ ] reports
- [x] docs
This commit is contained in:
Raphael Michel
2019-01-10 16:52:34 +01:00
committed by GitHub
parent 588955901c
commit 8abfbba9d0
41 changed files with 579 additions and 351 deletions
Download Patch File Download Diff File Expand all files Collapse all files

63
src/pretix/base/migrations/0104_auto_20181114_1526.py Normal file
View File

@@ -0,0 +1,63 @@
# Generated by Django 2.1.1 on 2018-11-14 15:26
import django.db.models.deletion
import django.db.models.manager
import jsonfallback.fields
from django.db import migrations, models
def change_refunded_to_canceled(apps, schema_editor):
Order = apps.get_model('pretixbase', 'Order')
Order.objects.filter(status='r').update(status='c', total=0)
Order.objects.filter(status='c').update(total=0)
class Migration(migrations.Migration):
dependencies = [
('pretixbase', '0103_auto_20181121_1224'),
]
operations = [
migrations.AlterModelManagers(
name='orderposition',
managers=[
('all', django.db.models.manager.Manager()),
],
),
migrations.AddField(
model_name='orderposition',
name='canceled',
field=models.BooleanField(default=False),
),
migrations.AlterField(
model_name='orderfee',
name='fee_type',
field=models.CharField(choices=[('payment', 'Payment fee'), ('shipping', 'Shipping fee'), ('service', 'Service fee'), ('cancellation', 'Cancellation fee'), ('other', 'Other fees'), ('giftcard', 'Gift card')], max_length=100),
),
migrations.AlterField(
model_name='orderposition',
name='order',
field=models.ForeignKey(on_delete=django.db.models.deletion.PROTECT, related_name='all_positions',
to='pretixbase.Order', verbose_name='Order'),
),
migrations.AlterModelManagers(
name='orderfee',
managers=[
('all', django.db.models.manager.Manager()),
],
),
migrations.AddField(
model_name='orderfee',
name='canceled',
field=models.BooleanField(default=False),
),
migrations.AlterField(
model_name='orderfee',
name='order',
field=models.ForeignKey(on_delete=django.db.models.deletion.PROTECT, related_name='all_fees', to='pretixbase.Order', verbose_name='Order'),
),
migrations.RunPython(
change_refunded_to_canceled, migrations.RunPython.noop
)
]

3
src/pretix/base/models/event.py
View File

@@ -355,7 +355,8 @@ class Event(EventMixin, LoggedModel):
if not really:
raise TypeError("Pass really=True as a parameter.")
OrderPosition.objects.filter(order__event=self).delete()
OrderPosition.all.filter(order__event=self, addon_to__isnull=False).delete()
OrderPosition.all.filter(order__event=self).delete()
OrderFee.objects.filter(order__event=self).delete()
OrderPayment.objects.filter(order__event=self).delete()
OrderRefund.objects.filter(order__event=self).delete()

2
src/pretix/base/models/items.py
View File

@@ -442,7 +442,7 @@ class Item(LoggedModel):
def allow_delete(self):
from pretix.base.models.orders import OrderPosition
return not OrderPosition.objects.filter(item=self).exists()
return not OrderPosition.all.filter(item=self).exists()
@cached_property
def has_variations(self):

73
src/pretix/base/models/orders.py
View File

@@ -70,7 +70,6 @@ class Order(LockModel, LoggedModel):
* ``STATUS_PAID``
* ``STATUS_EXPIRED``
* ``STATUS_CANCELED``
* ``STATUS_REFUNDED``
:param event: The event this order belongs to
:type event: Event
@@ -102,13 +101,12 @@ class Order(LockModel, LoggedModel):
STATUS_PAID = "p"
STATUS_EXPIRED = "e"
STATUS_CANCELED = "c"
STATUS_REFUNDED = "r"
STATUS_REFUNDED = "c" # deprecated
STATUS_CHOICE = (
(STATUS_PENDING, _("pending")),
(STATUS_PAID, _("paid")),
(STATUS_EXPIRED, _("expired")),
(STATUS_CANCELED, _("canceled")),
(STATUS_REFUNDED, _("refunded"))
)
code = models.CharField(
@@ -186,6 +184,28 @@ class Order(LockModel, LoggedModel):
def __str__(self):
return self.full_code
@property
def fees(self):
"""
Related manager for all non-canceled fees. Use ``all_fees`` instead if you want
canceled positions as well.
"""
return self.all_fees(manager='objects')
@cached_property
def count_positions(self):
if hasattr(self, 'pcnt'):
return self.pcnt or 0
return self.positions.count()
@property
def positions(self):
"""
Related manager for all non-canceled positions. Use ``all_positions`` instead if you want
canceled positions as well.
"""
return self.all_positions(manager='objects')
@cached_property
def meta_info_data(self):
try:
@@ -207,7 +227,7 @@ class Order(LockModel, LoggedModel):
@property
def pending_sum(self):
total = self.total
if self.status in (Order.STATUS_REFUNDED, Order.STATUS_CANCELED):
if self.status == Order.STATUS_CANCELED:
total = Decimal('0.00')
payment_sum = self.payments.filter(
state__in=(OrderPayment.PAYMENT_STATE_CONFIRMED, OrderPayment.PAYMENT_STATE_REFUNDED)
@@ -248,9 +268,9 @@ class Order(LockModel, LoggedModel):
pending_sum_rc=-1 * Coalesce(F('payment_sum'), 0) + Coalesce(F('refund_sum'), 0),
).annotate(
is_overpaid=Case(
When(~Q(status__in=(Order.STATUS_REFUNDED, Order.STATUS_CANCELED)) & Q(pending_sum_t__lt=0),
When(~Q(status=Order.STATUS_CANCELED) & Q(pending_sum_t__lt=0),
then=Value('1')),
When(Q(status__in=(Order.STATUS_REFUNDED, Order.STATUS_CANCELED)) & Q(pending_sum_rc__lt=0),
When(Q(status=Order.STATUS_CANCELED) & Q(pending_sum_rc__lt=0),
then=Value('1')),
default=Value('0'),
output_field=models.IntegerField()
@@ -336,8 +356,7 @@ class Order(LockModel, LoggedModel):
def cancel_allowed(self):
return (
self.status == Order.STATUS_PENDING
or (self.status == Order.STATUS_PAID and self.total == Decimal('0.00'))
self.status in (Order.STATUS_PENDING, Order.STATUS_PAID) and self.positions.exists()
)
@staticmethod
@@ -399,7 +418,10 @@ class Order(LockModel, LoggedModel):
"""
positions = self.positions.all().select_related('item')
cancelable = all([op.item.allow_cancel for op in positions])
return self.cancel_allowed() and self.event.settings.cancel_allow_user and cancelable
return (
self.status == Order.STATUS_PENDING
or (self.status == Order.STATUS_PAID and self.total == Decimal('0.00'))
) and self.event.settings.cancel_allow_user and cancelable
@property
def is_expired_by_time(self):
@@ -1254,11 +1276,19 @@ class OrderRefund(models.Model):
super().save(*args, **kwargs)
class ActivePositionManager(models.Manager):
def get_queryset(self):
return super().get_queryset().filter(canceled=False)
class OrderFee(models.Model):
"""
An OrderFee object represents a fee that is added to the order total independently of
the actual positions. This might for example be a payment or a shipping fee.
The default ``OrderFee.objects`` manager only contains fees that are not ``canceled``. If
you ant all objects, you need to use ``OrderFee.all`` instead.
:param value: Gross price of this fee
:type value: Decimal
:param order: Order this fee is charged with
@@ -1275,16 +1305,20 @@ class OrderFee(models.Model):
:type tax_rule: TaxRule
:param tax_value: The tax amount included in the price
:type tax_value: Decimal
:param canceled: True, if this position is canceled and should no longer be regarded
:type canceled: bool
"""
FEE_TYPE_PAYMENT = "payment"
FEE_TYPE_SHIPPING = "shipping"
FEE_TYPE_SERVICE = "service"
FEE_TYPE_CANCELLATION = "cancellation"
FEE_TYPE_OTHER = "other"
FEE_TYPE_GIFTCARD = "giftcard"
FEE_TYPES = (
(FEE_TYPE_PAYMENT, _("Payment fee")),
(FEE_TYPE_SHIPPING, _("Shipping fee")),
(FEE_TYPE_SERVICE, _("Service fee")),
(FEE_TYPE_CANCELLATION, _("Cancellation fee")),
(FEE_TYPE_OTHER, _("Other fees")),
(FEE_TYPE_GIFTCARD, _("Gift card")),
)
@@ -1296,7 +1330,7 @@ class OrderFee(models.Model):
order = models.ForeignKey(
Order,
verbose_name=_("Order"),
related_name='fees',
related_name='all_fees',
on_delete=models.PROTECT
)
fee_type = models.CharField(
@@ -1317,6 +1351,10 @@ class OrderFee(models.Model):
max_digits=10, decimal_places=2,
verbose_name=_('Tax value')
)
canceled = models.BooleanField(default=False)
all = models.Manager()
objects = ActivePositionManager()
@property
def net_value(self):
@@ -1371,6 +1409,9 @@ class OrderPosition(AbstractPosition):
of a specified type (or variation). This has all properties of
AbstractPosition.
The default ``OrderPosition.objects`` manager only contains fees that are not ``canceled``. If
you ant all objects, you need to use ``OrderPosition.all`` instead.
:param order: The order this position is a part of
:type order: Order
:param positionid: A local ID of this position, counted for each order individually
@@ -1383,6 +1424,8 @@ class OrderPosition(AbstractPosition):
:type tax_value: Decimal
:param secret: The secret used for ticket QR codes
:type secret: str
:param canceled: True, if this position is canceled and should no longer be regarded
:type canceled: bool
:param pseudonymization_id: The QR code content for lead scanning
:type pseudonymization_id: str
"""
@@ -1390,7 +1433,7 @@ class OrderPosition(AbstractPosition):
order = models.ForeignKey(
Order,
verbose_name=_("Order"),
related_name='positions',
related_name='all_positions',
on_delete=models.PROTECT
)
tax_rate = models.DecimalField(
@@ -1412,6 +1455,10 @@ class OrderPosition(AbstractPosition):
unique=True,
db_index=True
)
canceled = models.BooleanField(default=False)
all = models.Manager()
objects = ActivePositionManager()
class Meta:
verbose_name = _("Order position")
@@ -1492,7 +1539,7 @@ class OrderPosition(AbstractPosition):
self._calculate_tax()
self.order.touch()
if self.pk is None:
while OrderPosition.objects.filter(secret=self.secret).exists():
while OrderPosition.all.filter(secret=self.secret).exists():
self.secret = generate_position_secret()
if not self.pseudonymization_id:
@@ -1508,7 +1555,7 @@ class OrderPosition(AbstractPosition):
charset = list('ABCDEFGHJKLMNPQRSTUVWXYZ3789')
while True:
code = get_random_string(length=10, allowed_chars=charset)
if not OrderPosition.objects.filter(pseudonymization_id=code).exists():
if not OrderPosition.all.filter(pseudonymization_id=code).exists():
self.pseudonymization_id = code
return

2
src/pretix/base/models/tax.py
View File

@@ -97,7 +97,7 @@ class TaxRule(LoggedModel):
return (
not OrderFee.objects.filter(tax_rule=self, order__event=self.event).exists()
and not OrderPosition.objects.filter(tax_rule=self, order__event=self.event).exists()
and not OrderPosition.all.filter(tax_rule=self, order__event=self.event).exists()
and not self.event.items.filter(tax_rule=self).exists()
and self.event.settings.tax_rate_default != self
)

8
src/pretix/base/services/checkin.py
View File

@@ -78,7 +78,7 @@ def perform_checkin(op: OrderPosition, clist: CheckinList, given_answers: dict,
dt = datetime or now()
# Fetch order position with related objects
op = OrderPosition.objects.select_related(
op = OrderPosition.all.select_related(
'item', 'variation', 'order', 'addon_to'
).prefetch_related(
'item__questions',
@@ -90,6 +90,12 @@ def perform_checkin(op: OrderPosition, clist: CheckinList, given_answers: dict,
'answers'
).get(pk=op.pk)
if op.canceled:
raise CheckInError(
_('This order position has been canceled.'),
'unpaid'
)
answers = {a.question: a for a in op.answers.all()}
require_answers = []
for q in op.item.checkin_questions:

2
src/pretix/base/services/invoices.py
View File

@@ -234,7 +234,7 @@ def generate_invoice(order: Order, trigger_pdf=True):
if trigger_pdf:
invoice_pdf(invoice.pk)
if order.status in (Order.STATUS_CANCELED, Order.STATUS_REFUNDED):
if order.status == Order.STATUS_CANCELED:
generate_cancellation(invoice, trigger_pdf)
return invoice

39
src/pretix/base/services/orders.py
View File

@@ -124,25 +124,12 @@ def extend_order(order: Order, new_date: datetime, force: bool=False, user: User
@transaction.atomic
def mark_order_refunded(order, user=None, auth=None, api_token=None):
"""
Mark this order as refunded. This sets the payment status and returns the order object.
:param order: The order to change
:param user: The user that performed the change
"""
if isinstance(order, int):
order = Order.objects.get(pk=order)
if isinstance(user, int):
user = User.objects.get(pk=user)
with order.event.lock():
order.status = Order.STATUS_REFUNDED
order.save(update_fields=['status'])
order.log_action('pretix.event.order.refunded', user=user, auth=auth or api_token)
i = order.invoices.filter(is_cancellation=False).last()
if i:
generate_cancellation(i)
return order
oautha = auth.pk if isinstance(auth, OAuthApplication) else None
device = auth.pk if isinstance(auth, Device) else None
api_token = (api_token.pk if api_token else None) or (auth if isinstance(auth, TeamAPIToken) else None)
return _cancel_order(
order.pk, user.pk if user else None, send_mail=False, api_token=api_token, device=device, oauth_application=oautha
)
@transaction.atomic
@@ -1043,7 +1030,10 @@ class OrderChangeManager:
'addon_to': opa.addon_to_id,
'old_price': opa.price,
})
opa.delete()
opa.canceled = True
if opa.voucher:
Voucher.objects.filter(pk=opa.voucher.pk).update(redeemed=F('redeemed') - 1)
opa.save(update_fields=['canceled'])
self.order.log_action('pretix.event.order.changed.cancel', user=self.user, auth=self.auth, data={
'position': op.position.pk,
'positionid': op.position.positionid,
@@ -1052,7 +1042,10 @@ class OrderChangeManager:
'old_price': op.position.price,
'addon_to': None,
})
op.position.delete()
op.position.canceled = True
if op.position.voucher:
Voucher.objects.filter(pk=op.position.voucher.pk).update(redeemed=F('redeemed') - 1)
op.position.save(update_fields=['canceled'])
elif isinstance(op, self.AddOperation):
pos = OrderPosition.objects.create(
item=op.item, variation=op.variation, addon_to=op.addon_to,
@@ -1117,7 +1110,7 @@ class OrderChangeManager:
except InvoiceAddress.DoesNotExist:
pass
split_order.total = sum([p.price for p in split_positions])
split_order.total = sum([p.price for p in split_positions if not p.canceled])
if split_order.total != Decimal('0.00') and self.order.status != Order.STATUS_PAID:
pp = self._get_payment_provider()
if pp:
@@ -1180,7 +1173,7 @@ class OrderChangeManager:
return payment_sum - refund_sum
def _recalculate_total_and_payment_fee(self):
total = sum([p.price for p in self.order.positions.all()]) + sum([f.value for f in self.order.fees.all()])
total = sum([p.price for p in self.order.positions.all() if not p.canceled]) + sum([f.value for f in self.order.fees.all()])
payment_fee = Decimal('0.00')
if self.open_payment:
current_fee = Decimal('0.00')

25
src/pretix/base/services/stats.py
View File

@@ -1,7 +1,7 @@
from decimal import Decimal
from typing import Any, Dict, Iterable, List, Tuple
from django.db.models import Count, Sum
from django.db.models import Case, Count, F, Sum, Value, When
from django.utils.translation import ugettext_lazy as _
from pretix.base.models import Event, Item, ItemCategory, Order, OrderPosition
@@ -79,18 +79,22 @@ def order_overview(event: Event, subevent: SubEvent=None) -> Tuple[List[Tuple[It
'variations'
).order_by('category__position', 'category_id', 'position', 'name')
qs = OrderPosition.objects
qs = OrderPosition.all
if subevent:
qs = qs.filter(subevent=subevent)
counters = qs.filter(
order__event=event
).annotate(
status=Case(
When(canceled=True, then=Value('c')),
default=F('order__status')
)
).values(
'item', 'variation', 'order__status'
'item', 'variation', 'status'
).annotate(cnt=Count('id'), price=Sum('price'), tax_value=Sum('tax_value')).order_by()
states = {
'canceled': Order.STATUS_CANCELED,
'refunded': Order.STATUS_REFUNDED,
'paid': Order.STATUS_PAID,
'pending': Order.STATUS_PENDING,
'expired': Order.STATUS_EXPIRED,
@@ -99,7 +103,7 @@ def order_overview(event: Event, subevent: SubEvent=None) -> Tuple[List[Tuple[It
for l, s in states.items():
num[l] = {
(p['item'], p['variation']): (p['cnt'], p['price'], p['price'] - p['tax_value'])
for p in counters if p['order__status'] == s
for p in counters if p['status'] == s
}
num['total'] = dictsum(num['pending'], num['paid'])
@@ -149,16 +153,21 @@ def order_overview(event: Event, subevent: SubEvent=None) -> Tuple[List[Tuple[It
payment_items = []
if not subevent:
counters = OrderFee.objects.filter(
counters = OrderFee.all.filter(
order__event=event
).annotate(
status=Case(
When(canceled=True, then=Value('c')),
default=F('order__status')
)
).values(
'fee_type', 'internal_type', 'order__status'
'fee_type', 'internal_type', 'status'
).annotate(cnt=Count('id'), value=Sum('value'), tax_value=Sum('tax_value')).order_by()
for l, s in states.items():
num[l] = {
(o['fee_type'], o['internal_type']): (o['cnt'], o['value'], o['value'] - o['tax_value'])
for o in counters if o['order__status'] == s
for o in counters if o['status'] == s
}
num['total'] = dictsum(num['pending'], num['paid'])

10
src/pretix/base/shredder.py
View File

@@ -133,12 +133,12 @@ class EmailAddressShredder(BaseDataShredder):
}, indent=4)
yield 'emails-by-attendee.json', 'application/json', json.dumps({
'{}-{}'.format(op.order.code, op.positionid): op.attendee_email
for op in OrderPosition.objects.filter(order__event=self.event, attendee_email__isnull=False)
for op in OrderPosition.all.filter(order__event=self.event, attendee_email__isnull=False)
}, indent=4)
@transaction.atomic
def shred_data(self):
OrderPosition.objects.filter(order__event=self.event, attendee_email__isnull=False).update(attendee_email=None)
OrderPosition.all.filter(order__event=self.event, attendee_email__isnull=False).update(attendee_email=None)
for o in self.event.orders.all():
o.email = None
@@ -202,7 +202,7 @@ class AttendeeNameShredder(BaseDataShredder):
def generate_files(self) -> List[Tuple[str, str, str]]:
yield 'attendee-names.json', 'application/json', json.dumps({
'{}-{}'.format(op.order.code, op.positionid): op.attendee_name
for op in OrderPosition.objects.filter(
for op in OrderPosition.all.filter(
order__event=self.event
).filter(
Q(Q(attendee_name_cached__isnull=False) | Q(attendee_name_parts__isnull=False))
@@ -211,7 +211,7 @@ class AttendeeNameShredder(BaseDataShredder):
@transaction.atomic
def shred_data(self):
OrderPosition.objects.filter(
OrderPosition.all.filter(
order__event=self.event
).filter(
Q(Q(attendee_name_cached__isnull=False) | Q(attendee_name_parts__isnull=False))
@@ -267,7 +267,7 @@ class QuestionAnswerShredder(BaseDataShredder):
def generate_files(self) -> List[Tuple[str, str, str]]:
yield 'question-answers.json', 'application/json', json.dumps({
'{}-{}'.format(op.order.code, op.positionid): AnswerSerializer(op.answers.all(), many=True).data
for op in OrderPosition.objects.filter(order__event=self.event).prefetch_related('answers')
for op in OrderPosition.all.filter(order__event=self.event).prefetch_related('answers')
}, indent=4)
@transaction.atomic