search

Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.49k stars 2.85k forks source link

[HOLD for payment 2024-10-30] [CRI] [Search v2.2] Add `No category` and `No tag` search filters #49675

Open trjExpensify opened 4 weeks ago

trjExpensify commented 4 weeks ago

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: v9.0.39-2 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: customer in the NewDot feedback public room. Slack conversation: https://expensify.slack.com/archives/C036QM0SLJK/p1727216730118619

Action Performed:

  1. Create a workspace
  2. Enable tags in More features > add a couple of tags
  3. Go to the Search page
  4. Create an expense
  5. Click the Filters button
  6. Click category
  7. Click tag

Actual results

Expected results

Workaround:

Yes, use OldDot to filter, but we really don't want them to switch back to Classic.

Platforms:

Which of our officially supported platforms is this issue occurring on?

Screenshots/Videos

image image

View all open jobs on GitHub

Issue OwnerCurrent Issue Owner: @trjExpensify
melvin-bot[bot] commented 4 weeks ago

Current assignee @trjExpensify is eligible for the NewFeature assigner, not assigning anyone new.

trjExpensify commented 4 weeks ago

Put this on planning for a sec and assigned it to us @luacmartins while we just confirm on the Q here: https://expensify.slack.com/archives/C036QM0SLJK/p1727219750037359?thread_ts=1727216730.118619&cid=C036QM0SLJK

JmillsExpensify commented 4 weeks ago

I'm not sure this should be Uncategorized and Untagged given that we know we have customers that use Uncategorized.

trjExpensify commented 4 weeks ago

Why not? We've used that for years above the list of categories in the filters? I think that's fine.

trjExpensify commented 4 weeks ago

We could name the rows "No category" and "No tag" if we want? It would match the search syntax of no:category. I'm down for whatever.

luacmartins commented 3 weeks ago

I'm cool with matching the syntax and using No category or No tag

JmillsExpensify commented 3 weeks ago

Awesome that resolves my concern.

trjExpensify commented 3 weeks ago

Updated!

trjExpensify commented 3 weeks ago

You have everything you need here now to proceed, @luacmartins?

luacmartins commented 3 weeks ago

Yea, I think I have everything for now

luacmartins commented 2 weeks ago

Working on the draft PRs

melvin-bot[bot] commented 2 weeks ago

@trjExpensify, @luacmartins Whoops! This issue is 2 days overdue. Let's get this updated quick!

melvin-bot[bot] commented 2 weeks ago

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

luacmartins commented 2 weeks ago

PRs in draft. I was ooo Mon/Tue and working reduced hours today, will come back to them tomorrow.

luacmartins commented 1 week ago

@trjExpensify is this the behavior we want for the No category selector?

https://github.com/user-attachments/assets/80c1ea46-b050-4636-a237-c8aeed3d5477

trjExpensify commented 1 week ago

I find it strange being above the list when it's not selected. I'd vote to make it the top-most option in the list and then it moves above the list like selected category values do.

luacmartins commented 1 week ago

PRs in review

melvin-bot[bot] commented 1 week ago

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

ikevin127 commented 6 days ago

@luacmartins Just wanted to get some clarity on whether the regression was caused by FE and testing or comes from BE given the context mentioned in https://github.com/Expensify/App/pull/50122#issuecomment-2419505654. Was I missing something when I tested the PR here ?

I know the PR was reverted so if the error was on my part and it was 100% FE related then it would make sense that no payment is due here for reviweing the reverted PR. What's your take on what happened ?

Will jump on reviewing the v2 PR once the auth PR is deployed and HOLD is lifted 👍

luacmartins commented 6 days ago

There were two issues:

  1. No category and No tag options weren't persisted once we selected View results and opened the page again. That was 100% frontend and fixed on this commit
  2. The issue of returning no tags was in the backend, but we could have easily caught that while testing.
melvin-bot[bot] commented 5 days ago

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

melvin-bot[bot] commented 5 days ago

The solution for this issue has been :rocket: deployed to production :rocket: in version 9.0.50-8 and is now subject to a 7-day regression period :calendar:. Here is the list of pull requests that resolve this issue:

If no regressions arise, payment will be issued on 2024-10-25. :confetti_ball:

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

melvin-bot[bot] commented 5 days ago

BugZero Checklist: The PR adding this new feature has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

ikevin127 commented 5 days ago

No payment due here based on https://github.com/Expensify/App/issues/49675#issuecomment-2420615383 because the PR was reverted due to regression.

trjExpensify commented 2 days ago

Will jump on reviewing the v2 PR once the auth PR is deployed and HOLD is lifted 👍

Auth PR hit prod an hour ago: https://github.com/Expensify/Auth/pull/12812#issuecomment-2427681049

melvin-bot[bot] commented 21 hours ago

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

melvin-bot[bot] commented 21 hours ago

The solution for this issue has been :rocket: deployed to production :rocket: in version 9.0.52-5 and is now subject to a 7-day regression period :calendar:. Here is the list of pull requests that resolve this issue:

If no regressions arise, payment will be issued on 2024-10-30. :confetti_ball:

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

melvin-bot[bot] commented 21 hours ago

BugZero Checklist: The PR adding this new feature has been merged! The following checklist (instructions) will need to be completed before the issue can be closed: