Skip to content

Commit 6ceecb7

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

File tree

6 files changed

+45
-71
lines changed

6 files changed

+45
-71
lines changed

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

Lines changed: 10 additions & 54 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
/**
@@ -972,7 +969,12 @@ protected function setAPIAccessRequest()
972969
{
973970
$userId = $this->getConfigData('username');
974971
$userIdPass = $this->getConfigData('password');
975-
return $this->upsAuth->getAccessToken($userId, $userIdPass);
972+
if ($this->getConfigData('is_account_live')) {
973+
$authUrl = $this->_liveUrls['AuthUrl'];
974+
} else {
975+
$authUrl = $this->_defaultUrls['AuthUrl'];
976+
}
977+
return $this->upsAuth->getAccessToken($userId, $userIdPass, $authUrl);
976978
}
977979

978980
/**
@@ -1516,7 +1518,7 @@ private function requestQuotes(DataObject $request): array
15161518
}
15171519

15181520
/**
1519-
* Do shipment request to carrier web service, obtain Print Shipping Labels and process errors in response
1521+
* Not Required for UPS
15201522
*
15211523
* @param DataObject $request
15221524
* @return DataObject
@@ -1525,53 +1527,7 @@ private function requestQuotes(DataObject $request): array
15251527
*/
15261528
protected function _doShipmentRequest(DataObject $request)
15271529
{
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-
}
1530+
return '';
15751531
}
15761532

15771533
/**

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

Lines changed: 4 additions & 5 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
/**
@@ -77,7 +76,7 @@ public function __construct(
7776
* @throws LocalizedException
7877
* @throws NoSuchEntityException
7978
*/
80-
public function getAccessToken($clientId, $clientSecret)
79+
public function getAccessToken($clientId, $clientSecret, $clientUrl)
8180
{
8281
$cacheKey = self::CACHE_KEY_PREFIX;
8382
$result = $this->cache->load($cacheKey);
@@ -92,7 +91,7 @@ public function getAccessToken($clientId, $clientSecret)
9291
]);
9392
try {
9493
$asyncResponse = $this->asyncHttpClient->request(new Request(
95-
self::UPS_AUTH_URL,
94+
$clientUrl,
9695
Request::METHOD_POST,
9796
$headers,
9897
$authPayload
@@ -123,8 +122,8 @@ public function getAccessToken($clientId, $clientSecret)
123122
return $result;
124123
}
125124

126-
// phpcs:ignore
127-
public function collectRates(RateRequest $rateRequest)
125+
//phpcs:ignore
126+
public function collectRates(RateRequest $request)
128127
{
129128
// TODO: Implement collectRates() method.
130129
}

‎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)