Skip to content

Feature/v3 api upgrade#314

Open
taxcloud-havlek wants to merge 15 commits intodevelopfrom
feature/v3-api-upgrade
Open

Feature/v3 api upgrade#314
taxcloud-havlek wants to merge 15 commits intodevelopfrom
feature/v3-api-upgrade

Conversation

@taxcloud-havlek
Copy link
Copy Markdown
Contributor

"states" => $states
];
break;
case "get-exempt-certificates":
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

There isn't a post for this search

];
break;
case "carts/orders":
$xApiKey = $payload->getApiLoginID();
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

xAPIKey isn't 1-1 with api login id. It's actually a different concept that didn't exist in v1 and can be pulled from the API key section of the taxcloud UI.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

HI Ben,
For V3, it is 1-many. whether it means one connection id has many xapikey? if yes, what is the UI in wordpress tax dashboard?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Sorry when I said 1 - 1, I meant that the api login id is not equal to the xapikey value, it is a totally different concept created for v3. I don't know that it has a value in the wordpress tax dashboard. I was wondering if the plan was to re-use the api login id field, or if a new field needed to be added to that page?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Currently I reuse the api login id field. no need to add a new field to that page. I have change the text description at frontend to be TaxCloud X-API-Key .
图片

}
$destinationLine1 = $payload->getDestination()->getAddress1();
if(!$destinationLine1){
$destinationLine1 = "650 street";
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

usually if address1 is not known, we will default to unknown instead of an address so we don't accidentally match some address logic for a specific address.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

image For the V3 API, the requirement is that line1 must be provided. However, considering that the street field is not essential for tax calculation purposes, it would be beneficial to make line1 optional in the V3 API. This adjustment aligns with the behavior of the V1 API, which also does not mandate the street field. By modifying the V3 API to allow line1 to be optional then we do not need to process $destinationLine1. How do you think?

switch ($endpoint){
case "get-exempt-certificates":
$customerId = $payload->getCustomerID();
$url = 'https://api.v3.taxcloud.com/tax/exemption-certificates?customerId='.$customerId;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You probably want to use connectionID here where connectionID is the old api key. This will get all the exemption certificates for this currently hooked up connection.

Also this is a cursor paged endpoint, so if a user has over 20 exemption certificates, they will only be shown the first 20 results.

"line1" => $destinationLine1,
"line2" => $payload->getAddress2(),
"state"=> $payload->getState(),
"zip" => $payload->getZip5()
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Any chance there would be a +4 on this zip?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@bjongbloedt i have updated the code, please have a check again

@bjongbloedt
Copy link
Copy Markdown

Looks like some cypress tests are failing post update. @taxcloud-havlek can you look into these?

switch ($endpoint){
case "get-exempt-certificates":
$customerId = $payload->getCustomerID();
$url = 'https://api.v3.taxcloud.com/tax/exemption-certificates?customerId='.$customerId."&&limit=100";
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Do you have the connection id available here for the connectionID param? It would make the query quicker, and this current setup could return certs that may not be associated with the current setup

{
try {
$response = $this->postV3('refunds', $parameters);
if ($response->getResponseType() == 'OK') {
Copy link
Copy Markdown

@bjongbloedt bjongbloedt Feb 5, 2025

Choose a reason for hiding this comment

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

I don't think this old ok field applies anymore. It should check for status and errors

* @param Captured $parameters
* @return bool
*/
public function Captured(Captured $parameters)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This action would need to be done on an existing order with the update order endpoint where completedDate is the date it was captured

* @param AuthorizedWithCapture $parameters
* @return bool
*/
public function AuthorizedWithCapture(AuthorizedWithCapture $parameters)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Capturing is done in the new system by converting a cart to an order. To capture at the same time, you need to set the completed flag to true

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

image when call postV3 with carts/orders, default is true for completed flag

* @param Authorized $parameters
* @return bool
*/
public function Authorized(Authorized $parameters)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Authorizing is done in the new system by converting a cart to an order. To capture at the same time, you need to set the completed flag to false

public function AddTransactions(AddTransactions $parameters)
{
try {
$response = new AddTransactionsResponse($this->post('AddTransactions', $parameters));
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You might want to pivot this into the create transaction endpoint instead. It doesn't have the multiple at a time functionality, but it is v3 vs v1

break;
case "ping":
$connectionId = $payload->getApiKey();
$url = "https://api.v3.taxcloud.com/tax/connections/$connectionId/ping";
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Could you interpolate these 2 urls with the base url for v3 above?

"itemId" => (string)$eachItem->getItemID(),
"price" => (double)$eachItem->getPrice(),
"quantity" => $eachItem->getQty(),
"tic" => (int) $eachItem->getTIC()
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

In the new world, if there is no tic we would prefer this value to be set to null or omitted. That way the default tic and/or product mapping logic could automatically assign the tic

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

image Currently the tic is required. Could i use "tic" => 0 if there is no tic in the new world?

Copy link
Copy Markdown

@bjongbloedt bjongbloedt Feb 7, 2025

Choose a reason for hiding this comment

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

I think the required is coming from the response. The requests for both cart and order should allow null to be passed in

@omnicolor omnicolor self-assigned this Feb 12, 2025
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

Successfully merging this pull request may close these issues.

4 participants