Skip to content

Commit 3adf75d

Browse files
glo05363glo05363
committed
#AC-8918::XML to REST API-Code review changes
1 parent dc5874d commit 3adf75d

File tree

6 files changed

+69
-86
lines changed

6 files changed

+69
-86
lines changed

‎app/code/Magento/Ups/Model/Carrier.php

Lines changed: 22 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -100,18 +100,14 @@ class Carrier extends AbstractCarrierOnline implements CarrierInterface
100100
*/
101101
protected $_baseCurrencyRate;
102102

103-
/**
104-
* @var string
105-
*/
106-
protected $_xmlAccessRequest;
107-
108103
/**
109104
* Test urls for shipment
110105
*
111106
* @var array
112107
*/
113108
protected $_defaultUrls = [
114109
'ShipConfirm' => 'https://wwwcie.ups.com/api/shipments/v1/ship',
110+
'AuthUrl' => 'https://wwwcie.ups.com/security/v1/oauth/token',
115111
];
116112

117113
/**
@@ -121,6 +117,7 @@ class Carrier extends AbstractCarrierOnline implements CarrierInterface
121117
*/
122118
protected $_liveUrls = [
123119
'ShipConfirm' => 'https://onlinetools.ups.com/api/shipments/v1/ship',
120+
'AuthUrl' => 'https://onlinetools.ups.com/security/v1/oauth/token',
124121
];
125122

126123
/**
@@ -556,7 +553,6 @@ protected function _getQuotes()
556553
$destPostal = $rowRequest->getDestPostal();
557554
}
558555
$params = [
559-
'accept_UPS_license_agreement' => 'yes',
560556
'10_action' => $rowRequest->getAction(),
561557
'13_product' => $rowRequest->getProduct(),
562558
'14_origCountry' => $rowRequest->getOrigCountry(),
@@ -972,7 +968,12 @@ protected function setAPIAccessRequest()
972968
{
973969
$userId = $this->getConfigData('username');
974970
$userIdPass = $this->getConfigData('password');
975-
return $this->upsAuth->getAccessToken($userId, $userIdPass);
971+
if ($this->getConfigData('is_account_live')) {
972+
$authUrl = $this->_liveUrls['AuthUrl'];
973+
} else {
974+
$authUrl = $this->_defaultUrls['AuthUrl'];
975+
}
976+
return $this->upsAuth->getAccessToken($userId, $userIdPass, $authUrl);
976977
}
977978

978979
/**
@@ -1515,65 +1516,6 @@ private function requestQuotes(DataObject $request): array
15151516
return $results;
15161517
}
15171518

1518-
/**
1519-
* Do shipment request to carrier web service, obtain Print Shipping Labels and process errors in response
1520-
*
1521-
* @param DataObject $request
1522-
* @return DataObject
1523-
* @deprecated 100.3.3 New asynchronous methods introduced.
1524-
* @see requestToShipment
1525-
*/
1526-
protected function _doShipmentRequest(DataObject $request)
1527-
{
1528-
$this->_prepareShipmentRequest($request);
1529-
$result = new DataObject();
1530-
$rawXmlRequest = $this->_formShipmentRequest($request);
1531-
$xmlRequest = $this->_xmlAccessRequest . $rawXmlRequest;
1532-
$xmlResponse = $this->_getCachedQuotes($xmlRequest);
1533-
$debugData = [];
1534-
1535-
if ($xmlResponse === null) {
1536-
$debugData['request'] = $this->filterDebugData($this->_xmlAccessRequest) . $rawXmlRequest;
1537-
$url = $this->getShipConfirmUrl();
1538-
try {
1539-
$deferredResponse = $this->asyncHttpClient->request(
1540-
new Request(
1541-
$url,
1542-
Request::METHOD_POST,
1543-
['Content-Type' => 'application/xml'],
1544-
$xmlRequest
1545-
)
1546-
);
1547-
$xmlResponse = $deferredResponse->get()->getBody();
1548-
$debugData['result'] = $xmlResponse;
1549-
$this->_setCachedQuotes($xmlRequest, $xmlResponse);
1550-
} catch (Throwable $e) {
1551-
$debugData['result'] = ['code' => $e->getCode(), 'error' => $e->getMessage()];
1552-
}
1553-
}
1554-
1555-
try {
1556-
$response = $this->_xmlElFactory->create(['data' => $xmlResponse]);
1557-
} catch (Throwable $e) {
1558-
$debugData['result'] = ['error' => $e->getMessage(), 'code' => $e->getCode()];
1559-
$result->setErrors($e->getMessage());
1560-
}
1561-
1562-
if (isset($response->Response->Error)
1563-
&& in_array($response->Response->Error->ErrorSeverity, ['Hard', 'Transient'])
1564-
) {
1565-
$result->setErrors((string)$response->Response->Error->ErrorDescription);
1566-
}
1567-
1568-
$this->_debug($debugData);
1569-
1570-
if ($result->hasErrors() || empty($response)) {
1571-
return $result;
1572-
} else {
1573-
return '';
1574-
}
1575-
}
1576-
15771519
/**
15781520
* Get ship confirm url
15791521
*
@@ -1802,4 +1744,18 @@ private function createPackages(float $totalWeight, array $packages): array
18021744

18031745
return $packages;
18041746
}
1747+
1748+
/**
1749+
* @inheritDoc
1750+
* @SuppressWarnings(PHPMD.UnusedFormalParameter)
1751+
* phpcs:disable
1752+
*/
1753+
protected function _doShipmentRequest(\Magento\Framework\DataObject $request)
1754+
{
1755+
/*
1756+
* This method intentionally has no implementation.
1757+
* TODO: Implement the logic here when needed in the future.
1758+
*/
1759+
//phpcs:enable
1760+
}
18051761
}

‎app/code/Magento/Ups/Model/UpsAuth.php

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,6 @@
3535

3636
class UpsAuth extends AbstractCarrier
3737
{
38-
public const UPS_AUTH_URL = 'https://wwwcie.ups.com/security/v1/oauth/token';
3938
public const CACHE_KEY_PREFIX = 'ups_api_token_';
4039

4140
/**
@@ -73,11 +72,12 @@ public function __construct(
7372
*
7473
* @param String $clientId
7574
* @param String $clientSecret
76-
* @return mixed
75+
* @param String $clientUrl
76+
* @return bool|string
7777
* @throws LocalizedException
78-
* @throws NoSuchEntityException
78+
* @throws \Throwable
7979
*/
80-
public function getAccessToken($clientId, $clientSecret)
80+
public function getAccessToken($clientId, $clientSecret, $clientUrl)
8181
{
8282
$cacheKey = self::CACHE_KEY_PREFIX;
8383
$result = $this->cache->load($cacheKey);
@@ -92,7 +92,7 @@ public function getAccessToken($clientId, $clientSecret)
9292
]);
9393
try {
9494
$asyncResponse = $this->asyncHttpClient->request(new Request(
95-
self::UPS_AUTH_URL,
95+
$clientUrl,
9696
Request::METHOD_POST,
9797
$headers,
9898
$authPayload
@@ -123,9 +123,17 @@ public function getAccessToken($clientId, $clientSecret)
123123
return $result;
124124
}
125125

126-
// phpcs:ignore
127-
public function collectRates(RateRequest $rateRequest)
126+
/**
127+
* @inheritDoc
128+
* @SuppressWarnings(PHPMD.UnusedFormalParameter)
129+
* phpcs:disable
130+
*/
131+
public function collectRates(RateRequest $request)
128132
{
129-
// TODO: Implement collectRates() method.
133+
/*
134+
* This method intentionally has no implementation.
135+
* TODO: Implement the logic here when needed in the future.
136+
*/
130137
}
138+
// phpcs:enable
131139
}

‎app/code/Magento/Ups/Test/Unit/Model/CarrierTest.php

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -520,6 +520,20 @@ public function allowedMethodsDataProvider(): array
520520
'UPS Next Day Air',
521521
'01,02,03',
522522
['01' => 'UPS Next Day Air']
523+
],
524+
[
525+
'originShipment',
526+
'02',
527+
'UPS Second Day Air',
528+
'01,02,03',
529+
['02' => 'UPS Second Day Air']
530+
],
531+
[
532+
'originShipment',
533+
'03',
534+
'UPS Ground',
535+
'01,02,03',
536+
['03' => 'UPS Ground']
523537
]
524538
];
525539
}

‎app/code/Magento/Ups/view/adminhtml/templates/system/shipping/carrier_config.phtml

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -70,25 +70,25 @@ require(["prototype"], function(){
7070
return false;
7171
}
7272
73-
var upsXml = Class.create();
74-
upsXml.prototype = {
73+
var upsRest = Class.create();
74+
upsRest.prototype = {
7575
initialize: function()
7676
{
7777
this.carriersUpsActiveId = 'carriers_ups_active';
7878
if (!$(this.carriersUpsActiveId)) {
7979
return;
8080
}
8181
82-
this.checkingUpsXmlId = ['carriers_ups_gateway_url','carriers_ups_username',
82+
this.checkingUpsId = ['carriers_ups_gateway_url','carriers_ups_username',
8383
'carriers_ups_password'];
8484
this.originShipmentTitle = '';
8585
this.allowedMethodsId = 'carriers_ups_allowed_methods';
8686
this.freeShipmentId = 'carriers_ups_free_method';
87-
this.onlyUpsXmlElements = ['carriers_ups_gateway_url','carriers_ups_tracking_url',
87+
this.onlyUpsElements = ['carriers_ups_gateway_url','carriers_ups_tracking_url',
8888
'carriers_ups_username','carriers_ups_password',
8989
'carriers_ups_origin_shipment','carriers_ups_negotiated_active','carriers_ups_shipper_number',
9090
'carriers_ups_mode_xml','carriers_ups_include_taxes'];
91-
this.authUpsXmlElements = ['carriers_ups_username',
91+
this.authUpsElements = ['carriers_ups_username',
9292
'carriers_ups_password'];
9393
9494
script;
@@ -155,15 +155,15 @@ $scriptString .= <<<script
155155
setFormValues: function()
156156
{
157157
var a;
158-
for (a = 0; a < this.checkingUpsXmlId.length; a++) {
159-
$(this.checkingUpsXmlId[a]).addClassName('required-entry');
160-
this.changeFieldsDisabledState(this.checkingUpsXmlId, a);
158+
for (a = 0; a < this.checkingUpsId.length; a++) {
159+
$(this.checkingUpsId[a]).addClassName('required-entry');
160+
this.changeFieldsDisabledState(this.checkingUpsId, a);
161161
}
162162
Event.observe($('carriers_ups_origin_shipment'), 'change', this.changeOriginShipment.bind(this));
163-
showRowArrayElements(this.onlyUpsXmlElements);
163+
showRowArrayElements(this.onlyUpsElements);
164164
this.changeOriginShipment(null, null);
165165
if (\$F(this.carriersUpsActiveId) !== '1'){
166-
hideRowArrayElements(this.onlyUpsXmlElements);
166+
hideRowArrayElements(this.onlyUpsElements);
167167
}
168168
169169
},
@@ -183,7 +183,7 @@ $scriptString .= <<<script
183183
}
184184
}
185185
};
186-
xml = new upsXml();
186+
xml = new upsRest();
187187
//]]>
188188
189189
});

‎dev/tests/integration/testsuite/Magento/Ups/Model/CarrierTest.php

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -274,6 +274,10 @@ public function collectRatesDataProvider()
274274
[0, 1, 2, '03', 136.09 ],
275275
[1, 0, 3, '03', 92.12 ],
276276
[1, 1, 4, '03', 92.12 ],
277+
[0, 0, 1, '13', 330.35 ],
278+
[0, 1, 2, '13', 331.79 ],
279+
[1, 0, 3, '13', 178.70 ],
280+
[1, 1, 4, '13', 178.70 ],
277281
];
278282
}
279283

‎dev/tests/integration/testsuite/Magento/Ups/Model/UpsAuthTest.php

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@ public function testGetAccessToken()
6868
// Prepare test data
6969
$clientId = 'user';
7070
$clientSecret = 'pass';
71+
$clientUrl = 'https://wwwcie.ups.com/security/v1/oauth/token';
7172

7273
// Prepare the expected response data
7374
$expectedAccessToken = 'abcdefghijklmnop';
@@ -100,7 +101,7 @@ public function testGetAccessToken()
100101
);
101102

102103
// Call the getAccessToken method and assert the result
103-
$accessToken = $this->upsAuth->getAccessToken($clientId, $clientSecret);
104+
$accessToken = $this->upsAuth->getAccessToken($clientId, $clientSecret, $clientUrl);
104105
$this->assertEquals($expectedAccessToken, $accessToken);
105106
}
106107
}

0 commit comments

Comments
 (0)