Skip to content
This repository was archived by the owner on Feb 27, 2025. It is now read-only.

Support reporting by individual device ID#39

Open
MSmedal wants to merge 2 commits intohps:masterfrom
MSmedal:patch-1
Open

Support reporting by individual device ID#39
MSmedal wants to merge 2 commits intohps:masterfrom
MSmedal:patch-1

Conversation

@MSmedal
Copy link

@MSmedal MSmedal commented Jan 30, 2019

This should allow users to report by individual Device IDs. Reporting by individual Device IDs is useful for merchant using multiple Portico devices.

This should allow users to report by individual Device IDs. Reporting by individual Device IDs is useful for merchant using multiple Portico devices.
* @throws \HpsInvalidRequestException
*/
public function listTransactions($startDate, $endDate, $filterBy = null)
public function listTransactions($startDate, $endDate, $deviceId = null, $filterBy = null)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's swap $deviceId and $filterBy parameters to retain backwards compatibility for potential current users of $filterBy.

$hpsReportActivity = $xml->createElement('hps:ReportActivity');
$hpsReportActivity->appendChild($xml->createElement('hps:RptStartUtcDT', $startDate));
$hpsReportActivity->appendChild($xml->createElement('hps:RptEndUtcDT', $endDate));
if ($deviceId != null) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's use strict inequality here (!==) or check for non-empty (!empty($deviceId)). The choice between those two should depend on how the gateway treats empty values for the DeviceId data element.

$hpsReportActivity->appendChild($xml->createElement('hps:RptEndUtcDT', $endDate));
if ($deviceId != null) {
$hpsReportActivity->appendChild($xml->createElement('hps:DeviceId', $deviceId));
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This trailing semicolon isn't necessary.

Copy link
Author

@MSmedal MSmedal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requested changes included

@slogsdon
Copy link
Contributor

slogsdon commented Feb 1, 2019

Looks good. We'll get this included in the next deployment.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Comments