-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
[HOLD for payment 2024-04-09] [$500] Clicking on debit card option on the pay with Expensify button does nothing #38641
Comments
Triggered auto assignment to @dylanexpensify ( |
ProposalPlease re-state the problem that we are trying to solve in this issue.Clicking on debit card option on the pay with Expensify button does nothing What is the root cause of that problem?the App/src/components/AddPaymentMethodMenu.js Line 110 in d74fffe
then here we check if the selected option is debit card, then we navigate to the App/src/components/KYCWall/BaseKYCWall.tsx Lines 124 to 125 in 93c66ce
then we use the KYC in the App/src/components/SettlementButton.tsx Lines 222 to 233 in e9306bd
the issue appears because we dont pass the What changes do you think we should make in order to solve the problem?we should add we can use the App/src/components/MoneyReportHeader.tsx Lines 145 to 158 in 26ab4aa
App/src/components/MoneyReportHeader.tsx Lines 113 to 127 in 26ab4aa
App/src/components/ReportActionItem/ReportPreview.tsx Lines 285 to 304 in e8eac0a
|
That's what I was thinking the issue might be. |
ProposalPlease re-state the problem that we are trying to solve in this issue.What is the root cause of that problem?We have default empty value in App/src/components/SettlementButton.tsx Lines 99 to 100 in e9306bd
This in turn doesn't give us any action on click to the add debit card button as we have no route mentioned: App/src/components/SettlementButton.tsx Lines 221 to 225 in e9306bd
What changes do you think we should make in order to solve the problem?I assume we must always enable the debit card option on every request, so better is to update the default value of function SettlementButton({
addDebitCardRoute ={ROUTES.SETTINGS_ADD_DEBIT_CARD},
What alternative solutions did you explore? (Optional)N/A |
Note I think we should use |
Updated the proposal: optionally we can use the |
@abzokhattab's proposal makes sense to me. @dylanexpensify Let's roll here. |
@parasharrajat , what do you think about my proposal? Doesn’t it make sense to have the route in the base component itself, I guess in the roadmap we plan to add this settlement button in invoices too, so maybe it’s more future proof?? c.c. @dylanexpensify |
@GandalfGwaihir That is better but can not be given more weight regarding proposals. I would call that implementation detail. The main idea still lies around setting the missing param with the correct value. |
Job added to Upwork: https://www.upwork.com/jobs/~01d862972a492e6ec4 |
Triggered auto assignment to Contributor-plus team member for initial proposal review - @jjcoffee ( |
rock and roll @parasharrajat |
But i think it would be important to consider right approach over here :) |
I agree that we should move with the right approach but setting a default value or writing a code in a specific way is just implementation details. These things are cleared up in the PRs. I also thought of setting a default value when I found this bug. |
ohh cool, no worries, fine with whatever decision you and the internal engineer take |
@abzokhattab's proposal makes sense to me. IMO, ROUTES.IOU_SEND_ADD_DEBIT_CARD is the correct route for this. Thanks for the proposal @GandalfGwaihir. There are always new opportunities waiting for solutions. Looking forward to your contributions there. 🎀 👀 🎀 C+ reviewed |
Triggered auto assignment to @stitesExpensify, see https://stackoverflow.com/c/expensify/questions/7972 for more details. |
📣 @abzokhattab 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app! Offer link |
@abzokhattab When can we expect the PR? |
Thank you Rajat @parasharrajat for the ping, the PR will be ready today. |
The PR is ready! please check it out and let me know if you have any comments. |
Bump @stitesExpensify to final review for merge today if you can! ❤️ |
Merged! |
|
The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.58-8 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue: If no regressions arise, payment will be issued on 2024-04-09. 🎊 For reference, here are some details about the assignees on this issue:
|
BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:
|
BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:
Regression Test Steps
Do you agree 👍 or 👎 ? |
Payment summary:
Please apply or request! |
Done |
Payment requested as per #38641 (comment) |
$500 approved for @parasharrajat |
If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!
Version Number: 1.4.54-1
Reproducible in staging?: y
Reproducible in production?: y
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: @parasharrajat
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1710845227712549
Action Performed:
pay with expensify
option from the top right corner of the pagedebit card
Expected Result:
User taken to debit card adding flow
Actual Result:
Nothing happens clicking on
debit card
Workaround:
unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
Screenshots/Videos
Add any screenshot/video evidence
Recording.2876.mp4
Screen.Recording.2024-03-19.at.4.16.07.PM.mov
View all open jobs on GitHub
Upwork Automation - Do Not Edit
The text was updated successfully, but these errors were encountered: