Skip to content
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

Issues setting extended data on a Financial Transaction #34

Closed
bazgear opened this issue May 14, 2012 · 10 comments
Closed

Issues setting extended data on a Financial Transaction #34

bazgear opened this issue May 14, 2012 · 10 comments

Comments

@bazgear
Copy link
Contributor

bazgear commented May 14, 2012

Hi,

I'm looking for some direction in how to set extended data specifically for a financial transaction.

I use the setExtendedData method within the financial transaction class and then pull it back out and start setting data on it.

However, because the onPostLoad() method in FinancialTransaction does not setup extendedDataOriginal (as it is null), then the equals method within onPrePersist() throws an exception as null is not allowed to be passed through it.

I have had to allow the equals() method in ExtendedData to be null just to allow me to set extended data on the Financial Transaction.

Has anyone else come across this, or am I setting extended data on the Financial transaction the wrong way?

Would appreciated if you could clarify if this is an issue or just me using the class wrongly.

Many thanks

@bazgear
Copy link
Contributor Author

bazgear commented May 18, 2012

Has nobody else had this issue? It looks like you can't set extended data of a financial transaction? I can't believe it, where are you all storing payment transaction detail for transactions?

@schmittjoh
Copy link
Owner

Could you share your use-case?

@bazgear
Copy link
Contributor Author

bazgear commented May 18, 2012

Thanks for your reply (great bundle by-the-way).

Here is a description of my use-case:

I am implementing a plugin for Sagepay. I am building up the request parameters to send to the payment engine and I will expect a result back for the appropriate methods (approve, deposit, etc...). This has been implemented and works ok.

However I want to store some of the sent parameters and some of the response parameters I get back from Sagepay explicitly in the financial transaction extended data field.

The method FinancialTransaction::setExtendedData(ExtendedDataInterface $data) would allow me to set extended data for the financial transaction, however IF there is no extended data already set on the financial transaction then FinancialTransaction::onPrePersist() will throw an exception, this is because $this->extendedDataOriginal is null when extended data has never been set before on the fin. transaction (see extract below).

extract from FinancialTransaction::onPrePersist() ----------------->
if (null !== $this->extendedData && false === $this->extendedData->equals($this->extendedDataOriginal)) {
$this->extendedData = clone $this->extendedData;
}
<--------------------------------------------------------

So, I am doing the following in my plugin to set financial data:

//set the financial data
$this->currentTransaction->setExtendedData(new ExtendedData());

//get extended data
$data = $this->currentTransaction->getExtendedData();

//set some data
$data->set('foo' , 'bar');

But onPrePersist the equals method throws an exception, I have had to change the ExtendedData::equals method to allow null.

public function equals(ExtendedDataInterface $data = null)
{
if ($data === null){
return false;
}

    $data = $data->all();
    ksort($data);

    $cData = $this->data;
    ksort($cData);

    return $data === $cData;
}

Does this make sense - or can you give me some direction on how to set extendedData for a Financial Transaction without an exception being thrown.

Many thanks
Barry

@schmittjoh
Copy link
Owner

This seems like a bug to me.

Could you add an additional check to the onPrePersist method to avoid this?

Also keep in mind that setting an extended data object on the transaction will effectively hide the data from the payment instruction (if you have any on it).

@bazgear
Copy link
Contributor Author

bazgear commented May 18, 2012

Ok - I will add to the onPrePersist method.

Yes, I see what you mean about hiding the extended data of the payment instruction by setting it on the transaction. However as I will have at least 2 transaction for a payment (approve and then a separate deposit) then I though it was better to store each transactions parameters within their respective entities (although it might be ok to store against the instruction).


I have another quick question about PluginController::doCredit states.

Where you can only do a credit when the payment is either STATE_APPROVED or STATE_EXPIRED - I am trying to do a credit (refund) on a deposited payment - which surely makes sense as you need to have taken the money (not just approved it) before you can refund it. However when a payment is STATE_DEPOSITED you can't do a credit. So to get round this I have expired the payment before refunding.

I should put this on a different stream (separate issue) , sorry.


Many thanks for your time, it's appreciated.

@schmittjoh
Copy link
Owner

You're right on the doCredit method. This is an inconsistency, and needs to
be fixed.

Also see #11

On Fri, May 18, 2012 at 9:40 AM, Barry <
[email protected]

wrote:

Ok - I will add to the onPrePersist method.

Yes, I see what you mean about hiding the extended data of the payment
instruction by setting it on the transaction. However as I will have at
least 2 transaction for a payment (approve and then a separate deposit)
then I though it was better to store each transactions parameters within
their respective entities (although it might be ok to store against the
instruction).


I have another quick question about PluginController::doCredit states.

Where you can only do a credit when the payment is either STATE_APPROVED
or STATE_EXPIRED - I am trying to do a credit (refund) on a deposited
payment - which surely makes sense as you need to have taken the money (not
just approved it) before you can refund it. However when a payment is
STATE_DEPOSITED you can't do a credit. So to get round this I have expired
the payment before refunding.

I should put this on a different stream (separate issue) , sorry.


Many thanks for your time, it's appreciated.


Reply to this email directly or view it on GitHub:

#34 (comment)

@bazgear
Copy link
Contributor Author

bazgear commented May 18, 2012

Thanks for your help.

@bazgear
Copy link
Contributor Author

bazgear commented May 22, 2012

Hi - pull request (as discussed) is here by Calum Brodie, #31

This will allow extended data to be set on a financial transaction.

Many thanks

@calumbrodie
Copy link
Contributor

You can close this one I think (you merged my PR).

@schmittjoh
Copy link
Owner

yep, thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants