-
Notifications
You must be signed in to change notification settings - Fork 3k
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
22875 Add API Calls when clicking “Reveal details” buttons #29017
Changes from 18 commits
f73d601
5985ec1
df10ff7
36fb11a
bfdb546
9a8962c
c7dd8fa
ccf8f4e
ef623a4
f7ba68a
9f173af
8361350
36a6d4e
28ca868
a7bcbb6
f1aad79
42cb473
6dac4ca
dcb188b
8254860
586f20e
aaa7042
95d4a43
934344a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,8 @@ | ||
import Onyx from 'react-native-onyx'; | ||
import ONYXKEYS from '../../ONYXKEYS'; | ||
import * as API from '../API'; | ||
|
||
import CONST from '../../CONST'; | ||
import * as Localize from '../Localize'; | ||
/** | ||
* @param {Number} cardID | ||
*/ | ||
|
@@ -146,4 +147,31 @@ function clearCardListErrors(cardID) { | |
Onyx.merge(ONYXKEYS.CARD_LIST, {[cardID]: {errors: null, isLoading: false}}); | ||
} | ||
|
||
export {requestReplacementExpensifyCard, activatePhysicalExpensifyCard, clearCardListErrors, reportVirtualExpensifyCardFraud}; | ||
/** | ||
* Makes an API call to get virtual card details (pan, cvv, expiration date, address) | ||
* This function purposefully uses `makeRequestWithSideEffects` method. For security reason | ||
* card details cannot be persisted in Onyx and have to be asked for each time a user want's to | ||
* reveal them. | ||
* | ||
* @param {String} cardID - virtual card ID | ||
* | ||
* @returns {Promise<Object>} - promise with card details object | ||
*/ | ||
function revealVirtualCardDetails(cardID) { | ||
return new Promise((resolve, reject) => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why are we wrapping the call in this Promise? We don't use this pattern anywhere else There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @lukemorawski if you could explain, just curious There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think something like this would also work.. function revealVirtualCardDetails(cardID) {
// eslint-disable-next-line rulesdir/no-api-side-effects-method
return API.makeRequestWithSideEffects('RevealVirtualCardDetails', {cardID})
.then((response) => {
if (response.jsonCode !== CONST.JSON_CODE.SUCCESS) {
throw Localize.translateLocal('cardPage.cardDetailsLoadingFailure');
}
return response;
})
.catch((err) => {
throw err.message;
});
} There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. makes sense. but this isn't a blocker, maybe @lukemorawski can remove the promise later? |
||
// eslint-disable-next-line rulesdir/no-api-side-effects-method | ||
API.makeRequestWithSideEffects('RevealVirtualCardDetails', {cardID}) | ||
.then((response) => { | ||
if (response.jsonCode !== CONST.JSON_CODE.SUCCESS) { | ||
reject(Localize.translateLocal('cardPage.cardDetailsLoadingFailure')); | ||
return; | ||
} | ||
resolve(response); | ||
}) | ||
.catch((err) => { | ||
reject(err.message); | ||
lukemorawski marked this conversation as resolved.
Show resolved
Hide resolved
lukemorawski marked this conversation as resolved.
Show resolved
Hide resolved
|
||
}); | ||
}); | ||
} | ||
|
||
export {requestReplacementExpensifyCard, activatePhysicalExpensifyCard, clearCardListErrors, reportVirtualExpensifyCardFraud, revealVirtualCardDetails}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this reads better
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assumed this copy was already approved by marketing.
@lukemorawski @MrMuzyk @grgia is this wrong?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's no error message copy for this in the Design Doc @marcaaron
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok I think we need to get the copy looked at. All copy needs to be approved. 🙃
Where did this error message come from?