Refactor User_IsUsingExpensifyCard part 1#9937
Conversation
|
Looks like you modified Instead, all new API commands should use API.js, and follow our guidelines for writing new API commands. Unsure if your change is okay? Drop a note in #expensify-open-source! |
|
@luacmartins Not sure if we have any visible component that will help us test if the refactoring has be done correctly or not but the ONYX data are all fine. |
| /** | ||
| * @returns {Promise} | ||
| */ | ||
| function User_IsUsingExpensifyCard() { |
There was a problem hiding this comment.
We are still calling User_IsUsingExpensifyCard in validateBankAccount.js, so we can't remove this just yet. We have to leave this for part 2!
There was a problem hiding this comment.
Good catch, just fixed it. You can review it now. Thanks
32ff185 to
617c7de
Compare
|
LGTM and tests well. |
|
@luacmartins looks like this was merged without passing tests. Please add a note explaining why this was done and remove the |
|
Tests passed. Removing emergency label. |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚀 Deployed to production by @luacmartins in version: 1.1.85-8 🚀
|
Details
Removed User_IsUsingExpensifyCard command from the codebase except for its usage with
BankAccount_Validate. We will remove this once theBankAccount_Validatepart is done.Fixed Issues
$ https://github.com/Expensify/Expensify/issues/211834
Tests
User_IsUsingExpensifyCardso we have to make sure that no network request forUser_IsUsingExpensifyCardis being fired. Rather the data in the USER key forisUsingExpensifyCardis coming forOpenAppcommand. So make sure in ONYX you are able to see the value for theisUsingExpensifyCard.PR Review Checklist
Contributor (PR Author) Checklist
### Fixed Issuessection aboveTestssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*filesSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)/** comment above it */displayNamepropertythisproperly so there are no scoping issues (i.e. foronClick={this.submit}the methodthis.submitshould be bound tothisin the constructor)thisare necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);ifthis.submitis never passed to a component event handler likeonClick)StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)Avataris modified, I verified thatAvataris working as expected in all cases)PR Reviewer Checklist
The Contributor+ will copy/paste it into a new comment and complete it after the author checklist is completed
### Fixed Issuessection aboveTestssectionQA stepssectiontoggleReportand notonIconClick).src/languages/*filesSTYLE.md) were followedAvatar, I verified the components usingAvatarhave been tested & I retested again)/** comment above it */displayNamepropertythisproperly so there are no scoping issues (i.e. foronClick={this.submit}the methodthis.submitshould be bound tothisin the constructor)thisare necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);ifthis.submitis never passed to a component event handler likeonClick)StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)Avataris modified, I verified thatAvataris working as expected in all cases)QA Steps
Screenshots
Web
Mobile Web
Desktop
iOS
Android