pretix / pretix

Ticket shop application for conferences, festivals, concerts, tech events, shows, exhibitions, workshops, barcamps, etc.
https://pretix.eu
Other
1.88k stars 471 forks source link

Payment reminder uses delayed expiration date instead of communicated expiration date #4344

Closed KarlKeu00 closed 3 months ago

KarlKeu00 commented 3 months ago

Problem and impact

If both the payment term and the payment term delay are set in the payment settings for an event, the payment reminder will only be sent three days before the delayed expiry date. However, the description of the payment term delay states (at least in the German translation): "The order will not actually expire until so many days after the communicated expiry date." And by the way: no payment reminders seem to be sent out at all if the orders do not expire automatically.

Ref: https://github.com/pretix/pretix/blob/fa3ac69b8ef4988b07db6dfea9b503419ce15a3a/src/pretix/base/services/orders.py#L1347-L1348 https://github.com/pretix/pretix/blob/fa3ac69b8ef4988b07db6dfea9b503419ce15a3a/src/pretix/base/models/orders.py#L980-L1005

Expected behaviour

If the payment term is set to 14 days, the reminder email is set to 3 days before expiry and the delay is set to 30 days, I would expect a payment reminder to be sent 11 days after the order and the tickets to be returned to the pool 44 days after the order without payment being received.

Steps to reproduce

  1. set payment term to 14 days
  2. set payment reminder to 3 days before expiry
  3. set delay of expiry date to 30 days
  4. place order
  5. do not pay
  6. the payment reminder only comes after 31 days

Operating system, dependency versions

Docker pretix/standalone:stable

Version

2024.6.0

raphaelm commented 3 months ago

If both the payment term and the payment term delay are set in the payment settings for an event, the payment reminder will only be sent three days before the delayed expiry date. However, the description of the payment term delay states (at least in the German translation): "The order will not actually expire until so many days after the communicated expiry date."

This is a bug.

And by the way: no payment reminders seem to be sent out at all if the orders do not expire automatically.

This seems to be a design choice, although I struggle to remember the exact reason. We're planning to work on payment reminders in general, soon, so I'd hold off on that for then.

raphaelm commented 3 months ago

I don't see where the bug is coming from, though. Your cited line

https://github.com/pretix/pretix/blob/fa3ac69b8ef4988b07db6dfea9b503419ce15a3a/src/pretix/base/services/orders.py#L1347-L1348

refers to the automatic expiration, where delay should have an impact. In the code of the actual reminders, I don't see any use of the delay:

https://github.com/pretix/pretix/blob/fa3ac69b8ef4988b07db6dfea9b503419ce15a3a/src/pretix/base/services/orders.py#L1360-L1410

KarlKeu00 commented 3 months ago

As I understand it, here the system regularly checks whether the order has expired. https://github.com/pretix/pretix/blob/fa3ac69b8ef4988b07db6dfea9b503419ce15a3a/src/pretix/base/services/orders.py#L1327-L1348

The decision is based on the property payment_term_expire_date, which in this case returns the wrong result, namely the expiry date + the delay. According to the GitHub search, the property payment_term_expire_date is not used anywhere else, so it should only return the expiry date without delay. Perhaps the property should also be renamed to payment_term_communicated_expire_date? https://github.com/pretix/pretix/blob/fa3ac69b8ef4988b07db6dfea9b503419ce15a3a/src/pretix/base/models/orders.py#L980-L1005

The method mark_order_expired is called, in which the mail is finally sent or the event is triggered. https://github.com/pretix/pretix/blob/fa3ac69b8ef4988b07db6dfea9b503419ce15a3a/src/pretix/base/services/orders.py#L333-L354

raphaelm commented 3 months ago

Sorry, I still believe you are reading the code wrong. expire_orders is responsible for calling mark_order_expired after expire_date + delay have passed and is behaving absolutely correct.

expire_orders is not in any capacity involved in sending emails.

Emails are being sent by send_expiry_warnings which incorporates the delay neither directly nor through payment_term_expire_date.

Steps to reproduce

set payment term to 14 days set payment reminder to 3 days before expiry set delay of expiry date to 30 days place order do not pay the payment reminder only comes after 31 days

Are these actually steps that followed and happened like this, or is this your expectation of what probably would happen?

KarlKeu00 commented 3 months ago

Hmm, that's strange. In any case, no payment reminders are sent in my case as soon as a delay is activated. I can reproduce this error with the steps. I also just got another idea: I may not have set up the cron job correctly. I am currently checking whether this could have been the cause 😅. In this case, I would be very sorry.

KarlKeu00 commented 3 months ago

I actually believe that the missing cronjob was the problem. I would simply reopen the issue if I notice anything else in this regard. Perhaps it would be helpful to display the last execution of the job on a status page in the admin area and to display a banner in the event of a misconfiguration?