-
Notifications
You must be signed in to change notification settings - Fork 3.2k
[Due for payment 2025-03-10] [$250] "Approve" button showing for pending card transactions in search
#56791
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
Comments
Triggered auto assignment to @miljakljajic ( |
This has been labelled "Needs Reproduction". Follow the steps here: https://stackoverflowteams.com/c/expensify/questions/16989 |
Reproduction Step:
Screen.Recording.2025-02-13.at.10.54.32.mov@miljakljajic As this Slack message, the first C+ to post on the GH issue with reliable reproduction steps and video or screenshots confirming reproduction will be assigned as C+. |
In my opinion, the problem here is that the snapshot hasn't been updated correctly Screen.Recording.2025-02-13.at.10.56.02.mov |
search
search
Job added to Upwork: https://www.upwork.com/jobs/~021890041032822918106 |
Triggered auto assignment to @dylanexpensify ( |
Triggered auto assignment to Contributor-plus team member for initial proposal review - @thesahindia ( |
@m-natarajan @shawnborton Hi I am a new contributor and I am not clear how to send card transaction . Could someone please share the steps to send card transaction. Thank you |
Hmm good question. @mountiny any good advice there for getting pending transactions for our contributors? |
@shawnborton Are these reports you've approved within the same session? The A simulation of a pending card transaction in the correct state would be: Onyx.merge('transactions_<TRANSACTIONID>', {bank: "Expensify Card", cardId: 4, status: "Pending"})
Onyx.merge('snapshot_<SNAPSHOT_ID>', {data: {transactions_<TRANSACTIONID>: {bank: "Expensify Card", cardId: 4, status: "Pending"}}}) |
Ah actually maybe this is indeed fixed. That being said, I still think that maybe the "Pending" badge is more helpful than a View button in this case? So I think we should still move forward and change the View button to a Pending badge. |
@shawnborton If it happened for you recently, there's probably still a problem since the fix I mentioned was merged quite a while ago and heavily depended on simulated data 😄 If you're able to see if it still happens on login, that'd help see if there's still a (potentially BE) issue. |
I'll keep my eye out the next time I have pending transactions sent to me. Regardless, let's move forward with the badge idea. |
🚨 Edited by proposal-police: This proposal was edited at 2025-02-17 15:44:47 UTC. ProposalPlease re-state the problem that we are trying to solve in this issue.Currently we display only the What is the root cause of that problem?This is a new feature. What changes do you think we should make in order to solve the problem?In order to display a pending card icon in the
So that if there's a pending card transaction, we just output const isPendingExpensifyCardTransaction = isExpensifyCardTransaction(transactionItem) && isPending(transactionItem);
const typeIcon = isPendingExpensifyCardTransaction ? Expensicons.CreditCardHourglass : getTypeIcon(transactionItem.transactionType); (We could also pass Optionally, we can display a tooltip for the icon by wrapping it with a const tooltipText = isPendingExpensifyCardTransaction ? translate('iou.pending') : '';
return (
<Tooltip text={tooltipText}>
<View>
<Icon
src={typeIcon}
fill={theme.icon}
height={isLargeScreenWidth ? 20 : 12}
width={isLargeScreenWidth ? 20 : 12}
/>
</View>
</Tooltip>
); Result: What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?N/A as this would be a UI tweak. What alternative solutions did you explore? (Optional) |
Hmmm good call out, though I think I would just use a "Pending" badge there too instead of making it say View. But actually, this makes me wonder if we should just change the card icon to something like this: And then just keeping the View button as it is, since the column is indeed called the "Action column"? Thoughts @Expensify/design @trjExpensify @JmillsExpensify ? |
Let's do it! @jjcoffee let us know when to expect a PR 😄 |
PR ready for review 🙏 |
Deployed to production 2025-02-27, so payment should be due 2025-03-06. cc @dylanexpensify |
search
search
|
The solution for this issue has been 🚀 deployed to production 🚀 in version 9.1.7-2 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 2025-03-10. 🎊 For reference, here are some details about the assignees on this issue:
|
@jjcoffee / @DylanDylann @dylanexpensify @jjcoffee / @DylanDylann The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button] |
Nice! Coming up! |
Paying today! |
This is not a bug, seem like a new featureBugZero Checklist:
Bug classificationSource of bug:
Where bug was reported:
Who reported the bug:
Regression Test ProposalTest:
Do we agree 👍 or 👎 |
Payment summary: Contributor: @jjcoffee $250 via NewDot Please apply/request! |
done! |
$250 approved for @jjcoffee |
Uh oh!
There was an error while loading. Please reload this page.
If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!
Version Number:
Reproducible in staging?: need reproduction
Reproducible in production?: need reproduction
If this was caught on HybridApp, is this reproducible on New Expensify Standalone?:
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: @shawnborton
Slack conversation (hyperlinked to channel name): Expense
Action Performed:
Expected Result:
There should not be a approve button
Let's use a
Pending
badge like so:Actual Result:
Approve button is displayed
Workaround:
unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
Screenshots/Videos
Add any screenshot/video evidence
CleanShot.2025-02-12.at.15.48.29.mp4
View all open jobs on GitHub
Upwork Automation - Do Not Edit
Issue Owner
Current Issue Owner: @dylanexpensifyThe text was updated successfully, but these errors were encountered: