-
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
cleared terms and fees error message #8315
cleared terms and fees error message #8315
Conversation
@adeel0202 |
@Santhosh-Sellavel, I don't know the right steps to reach that page since I was never able to add a test bank account, so I followed these instructions for testing purposes. |
I think we need the steps for how to test it in a staging or production environment too. So @adeel0202 Kindly post in slack and ask for some help on this one. |
@Santhosh-Sellavel, I have updated the QA steps. |
PR updated. |
Thanks, @adeel0202! Avoid too many commits for simple changes like this one. |
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.
LGTM!
Sorry, @stitesExpensify I forget to approve & add the PR Reviewer Checklist here. Everything LGTM me as well! |
No worries. Thanks for the initial review :) |
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
@neil-marcellini @stitesExpensify Can you help QA this one internally? This is again about Transfer Balance. We are not able to QA |
🚀 Deployed to staging by @stitesExpensify in version: 1.1.47-0 🚀
|
@mvtglobally we need to hold off on QA because setting up activated wallets is currently broken, and causes the production reimbursement batch to fail. |
IMO that means we should just check it off on the deploy checklist for now, do you agree @neil-marcellini ? |
Yeah it's probably fine to let it through given that it's a small change. |
Cool. @mvtglobally feel free to check this off! |
🚀 Deployed to production by @roryabraham in version: 1.1.49-1 🚀
|
Details
Cleared the error message when both of the required checkboxes are checked on
Terms and fees
pageFixed Issues
$ #8244
Tests
PR Review Checklist
Contributor (PR Author) Checklist
### Fixed Issues
section aboveTests
sectionQA steps
sectiontoggleReport
and notonIconClick
)src/languages/*
filesSTYLE.md
) were followedAvatar
, I verified the components usingAvatar
are working as expected)/** comment above it */
displayName
propertythis
properly so there are no scoping issues (i.e. foronClick={this.submit}
the methodthis.submit
should be bound tothis
in the constructor)this
are necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);
ifthis.submit
is never passed to a component event handler likeonClick
)StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG
)Avatar
is modified, I verified thatAvatar
is working as expected in all cases)PR Reviewer Checklist
### Fixed Issues
section aboveTests
sectionQA steps
sectiontoggleReport
and notonIconClick
).src/languages/*
filesSTYLE.md
) were followed/** comment above it */
displayName
propertythis
properly so there are no scoping issues (i.e. foronClick={this.submit}
the methodthis.submit
should be bound tothis
in the constructor)this
are necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);
ifthis.submit
is never passed to a component event handler likeonClick
)StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG
)Avatar
is modified, I verified thatAvatar
is working as expected in all cases)QA Steps
Transfer Balance
(you must have some balance available)Additional details
step, fill in all the details and pressSave & continue
Onfido
step, verify your identity and presscontinue
Terms and fees
step, pressEnable payments
button without checking the required checkboxes to show the error messageScreenshots
Web
web.mov
Mobile Web
mWeb.mp4
Desktop
desktop.mov
iOS
ios.mp4
Android
android.mov