Disallow refund if payment hasn't been exported yet ....

[chr-chr]

Hello,

actually I can refund a sepadebit which wasn't exported,
which leads in some mess ...

Can anyone triplecheck this change:

8871493

Please note the not in the if clause.

[bockstaller]

I've added a test case to reproduce the behavior you are describing and think the change works as intended by @pc-coholic, but has a misleading commit message.

The change you highlighted is just short-circuiting the payment info_data validation in the payment_refund_supported method.
execute_refund will fill out the refund details if an export exists, or marks the refund as completed if no export exists (we haven't exported anything, so we can simply mark it as refunded and go on with our day, not collecting it).

I think the mess you are getting orginates from somewhere else, can you describe it in more detail?

[chr-chr]

payment_refund_supported shall return false in case there was no export:

    def payment_refund_supported(self, payment: OrderPayment) -> bool:
        if not payment.sepaexportorder_set.exists():
            return False

Otherwise on refund a sepa debit which wasn't exported:

• refund.done() ...
• the payment still gets exported (and processed)
• and there is no refund generated.

I just started my development setup and diving into the code.
There are more bugs, displaying the payments within an order ...

[raphaelm]

why should a non exported refund not work? i think it should just so the payment from ever being exported

[chr-chr]

why should a non exported refund not work?

Yes, that's the question that raised me too.

Actually payment_refund_supported will always return True (!)

Still a refund must be generated and exported bc the debit will be in the next export (fix execute_refund).

Also regarding the case: a part of the order gets a refund (e.g. storno one ticket) .... cancel or modifying a pending debit??!

Note: While testing I found a display bug of the sepa debits within the order view. I'll file another issue about.

[bockstaller]

payment_refund_supported is only validating that all information is available and correct to issue a refund. As long as your exported payments aren't corrupted it should return True.

refund.done() marks the payment as refunded. Only confirmed payments are exported by the OrganizerExportListView and the ExportListView.

-> An unexported payment can be refunded by setting its state to refunded and never exporting it
-> An exported payment creates a regular refund which you have to fulfill manually

[chr-chr]

After some more testing and hacking, I come to the conclusion
the "triplecheck" is done: the code line I was pointing to is correct.

I was not expecting this results:

-> An unexported payment can be refunded by setting its state to refunded and never exporting it
-> An exported payment creates a regular refund which you have to fulfill manually

That makes me wonder. It's the other way around, isn't it?

• case a) refund (full or part of!) on an exported payment works well: I'll get a refund I can export or mark as done.

• case b) refund on a not yet exported payment this creates and mark a refund as done so no export. But the debit will be in the next export anyway!

case b) did not make sense to me ... (use case?) so I was expecting this should be better disallowed

case b) I refund full or a part of the amount I must manually refund? I'll expect same outcome as case a)