Hi :-)

I have a problem with the module (latest alpha). Each time I try a transaction in "test" mode, I have a "Transaction Duplicated" code 94 error.
In production mode it works. Really strange... How can I help you to debug it ?

Mercanet says that in Test mode the transaction_reference should be unique. Is it the case for the module ?

Thanks for the infos :-)

CommentFileSizeAuthor
#18 2919562-17-use-uuid-as-transaction-ref.patch4.52 KBDeFr
#16 2919562-16-use-uuid-as-transaction-ref.patch3.85 KBDeFr
#14 2919562-13-use-uuid-as-transaction-ref.patch3.51 KBthony
#13 2919562-13-use-uuid-as-transaction-ref.patch3.64 KBDeFr
#12 2919562-12-use-uuid-as-transaction-ref.patch1.88 KBDeFr
#7 duplicate_transaction_reference_2919562_7.patch2.73 KBapsylone
#6 duplicate_transaction_reference_2919562_6.patch2.6 KBapsylone
#4 duplicate_transaction_reference_2919562_4.patch2.54 KBapsylone
#2 duplicate_transaction_reference_2919562_2.patch569 bytesapsylone

Comments

apsylone created an issue. See original summary.

apsylone’s picture

apsylone commented
StatusFileSize
new duplicate_transaction_reference_2919562_2.patch569 bytes

Ok, i've found why... The module for the transactionReference is based on the payment ID just created.
I think it is an error, because in development mode (cleaning databases multiple times), we recreated some payment ID that already exists for transactionReference bank side.

I think the transactionReference should use http://php.net/manual/fr/function.uniqid.php for resolving it.

borisson_’s picture

borisson_
Primary language Dutch
Location Mechelen, 🇧🇪
commented
Status: Active » Needs review

Setting to needs review to let the testbot have a look at the patch, I like the idea and I think it makes a ton of sense.

apsylone’s picture

apsylone commented
StatusFileSize
new duplicate_transaction_reference_2919562_4.patch2.54 KB

It was a bit more complex. I've added transaction_reference as fields for payment and check it again SIPS Response. WIP.

Status: Needs review » Needs work

The last submitted patch, 4: duplicate_transaction_reference_2919562_4.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

apsylone’s picture

apsylone commented
StatusFileSize
new duplicate_transaction_reference_2919562_6.patch2.6 KB

Error with the method loading payment on payment storage corrected.

apsylone’s picture

apsylone commented
StatusFileSize
new duplicate_transaction_reference_2919562_7.patch2.73 KB

Ok, removed WIP patches. This one is ok and worked.

borisson_’s picture

borisson_
Primary language Dutch
Location Mechelen, 🇧🇪
commented
Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 7: duplicate_transaction_reference_2919562_7.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

lalop’s picture

lalop commented

What is the status of this issue ?
Are you looking for a reviewer ?

DeFr’s picture

DeFr commented

Not really fond of adding another base field for that to the payment entities. Those entities already have a globally unique identifier : the UUID, using that instead of generating another unique id through uniqid() would be better.

As far as I can tell looking at least at the Sogenactif doc, the UUID can't be used as is because it's 36 characters long and transaction reference is only allowed to be up to 35 characters, but stripping the '-' from the uuid in TransformOrder, and then adding them back in the the callback should work, with no needs for needs for database schema change.

DeFr’s picture

DeFr commented
Status: Needs work » Needs review
StatusFileSize
new 2919562-12-use-uuid-as-transaction-ref.patch1.88 KB

Updated patch based on the approach above, and allowing both old transaction reference (with a plain order number) and uuid as transaction refs. That should allow a store to transition smoothly.

DeFr’s picture

DeFr commented
StatusFileSize
new 2919562-13-use-uuid-as-transaction-ref.patch3.64 KB

I think the attached patch is in fact even better ; it stores the uuid without the dash as the remote id by default, but developers can chose to alter it if needed in a pre save hook. It's then passed as is everywhere.

thony’s picture

thony commented
StatusFileSize
new 2919562-13-use-uuid-as-transaction-ref.patch3.51 KB

Hi, thx for the patch, I believe it contain a syntax error there : + $payment->setRemoteId(str_replace('-', '', $payment->uuid());
should be : + $payment->setRemoteId(str_replace('-', '', $payment->uuid()));

I also created a version that apply to the current 1.0 alpha3 (and does seem to fix the issue)

DeFr’s picture

DeFr commented

Yup ; sorry about the typo in the patch in #13, uploaded it quickly.

I think I'm going to commit this patch this week and release an alpha 4 of the module with the various fixes that went in.

DeFr’s picture

DeFr commented
StatusFileSize
new 2919562-16-use-uuid-as-transaction-ref.patch3.85 KB

Re-uploading a patch against the latest dev for the testbot.

Status: Needs review » Needs work

The last submitted patch, 16: 2919562-16-use-uuid-as-transaction-ref.patch, failed testing. View results

DeFr’s picture

DeFr commented
Status: Needs work » Needs review
StatusFileSize
new 2919562-17-use-uuid-as-transaction-ref.patch4.52 KB

Mocking the correct storage.

  • DeFr authored 499f114 on 8.x-1.x 
    Issue #2919562 by apsylone, DeFr, Thony: Mercanet and duplicate...
DeFr’s picture

DeFr commented
Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

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