Skip to content
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

[Due for payment 2025-02-10] [HOLD for payment 2025-02-07] [HOLD for payment 2025-02-06] Revert #52569 #54054

Open
JmillsExpensify opened this issue Dec 12, 2024 · 61 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Internal Requires API changes or must be handled by Expensify staff

Comments

@JmillsExpensify
Copy link

JmillsExpensify commented Dec 12, 2024

After giving the solution in #52569 a try, we've realized that it creates a confusing situation in the Inbox, which is:

  • Workspace chat is GBRed
  • Report has a grey Approved button

So in short, our solution for approve and pay (specifically with a report that has some held expenses and some unheld expenses) ends up breaking GBR.

As a result, we're going to revert the PR from #52569. That restores GBR to the Inbox and workspace chat. In addition, for the Search page specifically, we are going to disable the Approve button for this case, just like we landed on doing for Pay.

Issue OwnerCurrent Issue Owner: @JmillsExpensify
@JmillsExpensify JmillsExpensify added Daily KSv2 Internal Requires API changes or must be handled by Expensify staff Bug Something is broken. Auto assigns a BugZero manager. labels Dec 12, 2024
Copy link

melvin-bot bot commented Dec 12, 2024

Current assignee @JmillsExpensify is eligible for the Bug assigner, not assigning anyone new.

@mananjadhav
Copy link
Collaborator

@JmillsExpensify The expectation is that we revert the PR, and then disable Approve button as well as Pay as a new requirement? Would it be a single PR or two PRs - one revert and then second to disable Approve?

I can help with the review on this one if needed.

@melvin-bot melvin-bot bot added the Overdue label Dec 15, 2024
Copy link

melvin-bot bot commented Dec 16, 2024

@JmillsExpensify, @luacmartins Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

@luacmartins
Copy link
Contributor

I think we want to:

  1. Revert the behavior of changing the button colors.
  2. On Search, disable the option in the bulk action dropdown if any selected items are on hold

@luacmartins
Copy link
Contributor

@mananjadhav you're welcome to work on the PRs too if you'd like

@mananjadhav
Copy link
Collaborator

Yes, I can pick this.

@luacmartins
Copy link
Contributor

Great! All yours ❤️

@garrettmknight garrettmknight moved this to Bugs and Follow Up Issues in [#whatsnext] #expense Dec 17, 2024
@luacmartins
Copy link
Contributor

@mananjadhav how's this one going? Do you have an ETA for the PR?

@mananjadhav
Copy link
Collaborator

I am a bit sick, I'll have the Pr by tomorrow.

@luacmartins
Copy link
Contributor

Ah no worries. Hope you feel better soon.

Copy link

melvin-bot bot commented Dec 23, 2024

@JmillsExpensify, @mananjadhav, @luacmartins Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

@melvin-bot melvin-bot bot added the Overdue label Dec 23, 2024
@mananjadhav
Copy link
Collaborator

Apologies I was unwell. I am just getting back and will have this ready in 1-2 days.

@melvin-bot melvin-bot bot removed the Overdue label Dec 24, 2024
Copy link

melvin-bot bot commented Dec 26, 2024

@JmillsExpensify @mananjadhav @luacmartins this issue was created 2 weeks ago. Are we close to a solution? Let's make sure we're treating this as a top priority. Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

Copy link

melvin-bot bot commented Dec 30, 2024

@JmillsExpensify, @mananjadhav, @luacmartins Huh... This is 4 days overdue. Who can take care of this?

@melvin-bot melvin-bot bot added the Overdue label Dec 30, 2024
@mananjadhav
Copy link
Collaborator

Will raise the PR today.

@melvin-bot melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Weekly KSv2 labels Jan 30, 2025
@melvin-bot melvin-bot bot added Weekly KSv2 and removed Weekly KSv2 labels Jan 30, 2025
@melvin-bot melvin-bot bot added Weekly KSv2 and removed Weekly KSv2 labels Jan 31, 2025
@melvin-bot melvin-bot bot changed the title [HOLD for payment 2025-02-06] Revert #52569 [HOLD for payment 2025-02-07] [HOLD for payment 2025-02-06] Revert #52569 Jan 31, 2025
@melvin-bot melvin-bot bot removed the Reviewing Has a PR in review label Jan 31, 2025
Copy link

melvin-bot bot commented Jan 31, 2025

Reviewing label has been removed, please complete the "BugZero Checklist".

Copy link

melvin-bot bot commented Jan 31, 2025

The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.92-6 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-02-07. 🎊

For reference, here are some details about the assignees on this issue:

Copy link

melvin-bot bot commented Jan 31, 2025

@mananjadhav / @parasharrajat @JmillsExpensify @mananjadhav / @parasharrajat 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]

@melvin-bot melvin-bot bot added Weekly KSv2 and removed Weekly KSv2 labels Feb 3, 2025
@melvin-bot melvin-bot bot changed the title [HOLD for payment 2025-02-07] [HOLD for payment 2025-02-06] Revert #52569 [Due for payment 2025-02-10] [HOLD for payment 2025-02-07] [HOLD for payment 2025-02-06] Revert #52569 Feb 3, 2025
Copy link

melvin-bot bot commented Feb 3, 2025

The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.93-3 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-02-10. 🎊

For reference, here are some details about the assignees on this issue:

Copy link

melvin-bot bot commented Feb 3, 2025

@mananjadhav / @parasharrajat @JmillsExpensify @mananjadhav / @parasharrajat 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]

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Feb 5, 2025
@parasharrajat
Copy link
Member

parasharrajat commented Feb 5, 2025

We had to do a few back-and-forths to refactor the default string ID lint warning. Do we have a discussion somewhere on Slack about why this change was needed @mananjadhav except #56103 (comment).

@parasharrajat
Copy link
Member

Ok, I found it. No worries.

@parasharrajat
Copy link
Member

Do we need a Bug-zero checklist for this task?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Internal Requires API changes or must be handled by Expensify staff
Projects
Status: Bugs and Follow Up Issues
Development

No branches or pull requests

7 participants