When you view the Journal Role, the transactions show how the item was paid, as:
Cash - 1 tender type used
Cash, Debit - 2 tender types used
Debit, Credit, GC - 3 types used
etc.

However, if you choose a tender type and then void it, and pick another type, the above style still shows the same.
If I choose debit as my intitial attempt but their card is denied, I can now try their Credit (and we'll say it works). When viewing in the Journal role this should either just show:
Credit
or it could show
debit(strikethrough), Credit

Having both tener types showing the same will be confusing, so either the voided tender needs to go, or needs to include a strikethrough.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

mattferguson created an issue. See original summary.

shabana.navas’s picture

Status: Active » Needs review
FileSize
55.21 KB
740 bytes

Added code to go through the payment types and strike-through the voided tenders. Added a 'voided' class as well for styling later on. Note, this will effect the way it is displayed in the reports too (see: commerce_pos_report_preprocess_commerce_pos_report_order_details()) as the commerce_pos_report_payment_types() function is called from there as well.

travis-bradbury’s picture

Status: Needs review » Needs work

I think that change is striking out every payment type except the last one. If so, I don't think it's what we want because there could be multiple valid payment types on an order if someone paid $20 cash and the rest on credit.

I think we could get payments for the transaction where commerce_payment_transaction.status IN (list-of-statuses-where-the-status-counts-towards-the-total) by getting information about the statuses from commerce_payment_transaction_status(). That would exclude voided payments from this list rather than striking them out, which is OK in my opinion.

If we're showing but striking out types from payments that don't count toward the total, I think it's a bit more complicated since payment transactions have a status and this is a list of payment transaction types. If there are two credit payments where one failed, should it show as credit or credit?

fergy’s picture

I think that voided payments do need to be shown, but your last paragraph makes sense, Travis.

I would say credit(strikethrough), credit.

If you don't, you wouldn't be able to differentiate this from a transaction that has two credit cards used on a split bill.

shabana.navas’s picture

My initial jab at this came from a total lack of knowledge on how the entire POS transaction is done. But I understand things a lot better now thanks to your comments @mattferguson @tbradbury. So, @mattferguson, you still prefer showing the voided payments right. So, I have modified the code to show everything including the voided and take into account the split bill issue (see attached image).

I have refactored the code so we're just re-using the commerce_payment_transaction_load_multiple to grab the transactions, since we already have that, instead of the SQL query.

shabana.navas’s picture

Status: Needs work » Needs review
fergy’s picture

Status: Needs review » Reviewed & tested by the community
travis-bradbury’s picture

Status: Reviewed & tested by the community » Needs work

It looks like we're showing types multiple times if there were multiple successful payments of that type.

I think instead of "Cash, Cash, Debit, Debit, Debit", we should have "Cash, Cash, Debit, Debit".

travis-bradbury’s picture

I spoke with Matt and he thinks that it's still potentially useful for someone to see at-a-glance that an order had split payments. To reduce repetition, how about adding a number beside the type? Two credit payments could look like "Credit (2)" or in a very complicated edge case, you could have something like "Credit, Credit (2), Debit".

  • smccabe committed f140fda on 7.x-2.x authored by shabana.navas
    Issue #2871239 by shabana.navas, mattferguson, tbradbury: Payment Type...
smccabe’s picture

Status: Needs work » Fixed

well bugger, I just committed it, I thought we did want multiple types sort of like a history? Is this even worth doing?

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.