-
Notifications
You must be signed in to change notification settings - Fork 433
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow to resend vouchers by email #1812
base: master
Are you sure you want to change the base?
Conversation
# FIXME We can't get the original subject | ||
subject = gettext('Resent Voucher') | ||
# FIXME We can't get the original message | ||
message = LazyI18nString('We are resending you your voucher for {event}: {voucher_list}') |
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 doesn't get translated this way, does it?
Should I do this?:
message = LazyI18nString('We are resending you your voucher for {event}: {voucher_list}') | |
message = LazyI18nString(_('We are resending you your voucher for {event}: {voucher_list}')) |
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.
No, the correct way is using LazyI18nString.from_gettext, see for example pretix/base/settings.py for usage
locale=event.settings.locale, | ||
) | ||
v.log_action( | ||
'pretix.voucher.sent', |
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.
Should resending be a separate action from sending?
@@ -285,6 +285,29 @@ def get(self, request, *args, **kwargs): | |||
return redirect('control:event.vouchers', event=request.event.slug, organizer=request.event.organizer.slug) | |||
|
|||
|
|||
class VoucherResend(EventPermissionRequiredMixin, View): | |||
permission = 'can_change_vouchers' |
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.
Does resending count as changing?
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.
Yes
@@ -421,6 +444,20 @@ def post(self, request, *args, **kwargs): | |||
obj.max_usages = min(obj.redeemed, obj.max_usages) | |||
obj.save(update_fields=['max_usages']) | |||
messages.success(request, _('The selected vouchers have been deleted or disabled.')) | |||
elif request.POST.get('action') == 'resend': |
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.
If resending does not count as changing, this would need different permissions.
follow=True) | ||
assert doc.select(".alert-danger") | ||
|
||
# TODO test bulk resend |
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.
I didn't find any other testcases that test bulk functionality, so I didn't write any myself
133e94d
to
ecb1a4c
Compare
Codecov Report
@@ Coverage Diff @@
## master #1812 +/- ##
==========================================
- Coverage 79.29% 79.29% -0.01%
==========================================
Files 320 320
Lines 37972 38006 +34
==========================================
+ Hits 30111 30136 +25
- Misses 7861 7870 +9
|
I'm so sorry I didn't get around reviewing this yet :( Things are a little busy here and I still have some design concerns here that I need to spend some time thinking about. Hope things get better soon. Sorry again! |
No worries, take your time! |
Closes #1748
Currently it just sends a generic message. It would be nice to somehow reuse the old message or have the old message template as a starting point for composing a new message.