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.34k stars 2.77k forks source link

[HOLD for payment 2024-06-24] [$125] [Search v1] Search page: Show dates for prior year transactions #43209

Closed JmillsExpensify closed 2 months ago

JmillsExpensify commented 3 months ago

Problem

We're on the cusp of launching a v1 Search page, which means it's possible to see transactions from prior years. What I mean by this is that we are now in 2024, and I have a transaction I can find on the Search from back in 2017. The issue is that I have no idea looking at the transaction that it's from that far back.

Here's the New Expensify transaction. CleanShot 2024-06-06 at 12 16 38@2x

Here's the Expensify Classic transaction CleanShot 2024-06-06 at 12 17 06@2x

Solution

When looking at a prior year transaction, show the year, in addition to the month and date.

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~017440ce6c14aca464
  • Upwork Job ID: 1798793329608614800
  • Last Price Increase: 2024-06-06
  • Automatic offers:
    • dukenv0307 | Reviewer | 102671230
    • ShridharGoel | Contributor | 102671233
Issue OwnerCurrent Issue Owner: @greg-schroeder
melvin-bot[bot] commented 3 months ago

Triggered auto assignment to @greg-schroeder (NewFeature), see https://stackoverflowteams.com/c/expensify/questions/14418#:~:text=BugZero%20process%20steps%20for%20feature%20requests for more details. Please add this Feature request to a GH project, as outlined in the SO.

JmillsExpensify commented 3 months ago

P.S. We're going to need web and mobile mocks from the design team. I reached out to them.

luacmartins commented 3 months ago

@Expensify/design would love your ideas on how to display this

shawnborton commented 3 months ago

I'm thinking it might be best to just make the column wider: image

Because it doesn't look nearly as good if we stack like OldDot: image

Mobile seems pretty straight forward: CleanShot 2024-06-06 at 20 26 37@2x

melvin-bot[bot] commented 3 months ago

Job added to Upwork: https://www.upwork.com/jobs/~017440ce6c14aca464

melvin-bot[bot] commented 3 months ago

Triggered auto assignment to Contributor-plus team member for initial proposal review - @dukenv0307 (External)

melvin-bot[bot] commented 3 months ago

Upwork job price has been updated to $125

JmillsExpensify commented 3 months ago

Looks dope! I'll go ahead and take this so we can open up to community. cc @luacmartins

JmillsExpensify commented 3 months ago

Ah...haha you beat me to it.

ShridharGoel commented 3 months ago

Proposal

Please re-state the problem that we are trying to solve in this issue.

Search page: Show dates for prior year transactions

What is the root cause of that problem?

New change.

What changes do you think we should make in order to solve the problem?

Change column width to w96 at this place:

columnWidth = getWidthStyle(variables.w80);

Change the date format at this place:

const date = TransactionUtils.getCreated(transactionItem, CONST.DATE.MONTH_DAY_YEAR_ABBR_FORMAT);
dannymcclain commented 3 months ago

I agree we should just keep it simple and make the column wider.

dukenv0307 commented 3 months ago

Let's go with @ShridharGoel's solution. In the meantime, can you confirm what the width should be? Is w96 good? @dannymcclain @shawnborton

🎀👀🎀 C+ reviewed

melvin-bot[bot] commented 3 months ago

Current assignee @luacmartins is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new.

shawnborton commented 3 months ago

@ShridharGoel why use 96 for the column width? In the mockup we're using 80.

ShridharGoel commented 3 months ago

I think I don't have access to that mockup where the width is mentioned, so I added a 96 as a suggestion. Can you link it?

Edit: Updated it to 80 now.

shawnborton commented 3 months ago

Perfect, 80 is all you need so you should be good there.

dukenv0307 commented 3 months ago

@luacmartins wdyt about my decision above? Thanks

trjExpensify commented 3 months ago

@mountiny can you keep this moving? We might be able to get it today. :)

melvin-bot[bot] commented 3 months ago

📣 @dukenv0307 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link Upwork job

melvin-bot[bot] commented 3 months ago

📣 @ShridharGoel 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link Upwork job Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻 Keep in mind: Code of Conduct | Contributing 📖

mountiny commented 3 months ago

Cool taking over, I think Carlos will have enough of other tasks

trjExpensify commented 3 months ago

Cool taking over, I think Carlos will have enough of other tasks

Nah, the guy barely works. :trollface:

greg-schroeder commented 3 months ago

PR is up/in review

melvin-bot[bot] commented 3 months ago

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

melvin-bot[bot] commented 3 months ago

The solution for this issue has been :rocket: deployed to production :rocket: in version 1.4.84-3 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-06-24. :confetti_ball:

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

melvin-bot[bot] commented 3 months 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:

ShridharGoel commented 3 months ago

@luacmartins @mountiny Since the scope of this task had increased, do you think it qualifies for a normal bounty instead of 125 ?

luacmartins commented 3 months ago

@ShridharGoel I'm not sure that I understand how the scope increased. We still just added the year to the date column if there are transactions from previous years which was the OP, no?

ShridharGoel commented 3 months ago

@luacmartins I think initially it was decided that we'll just make the column wider, then it was concluded that we'll only make it wider if there's at least one transaction from a past year. Do let me know your thoughts on this.

ShridharGoel commented 3 months ago

@luacmartins What I mean mostly is that this wasn't a simple UI change and it involved significant logical changes. Though I'm happy with the existing bounty as well, just thought to discuss about how it is decided in such scenarios.

luacmartins commented 3 months ago

I think the OP was to add the year, which is where the logic change came from and definitely in scope. I think we would just keep the existing bounty as is.

melvin-bot[bot] commented 3 months ago

The solution for this issue has been :rocket: deployed to production :rocket: in version 1.4.85-7 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-06-28. :confetti_ball:

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

melvin-bot[bot] commented 3 months 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:

dukenv0307 commented 2 months ago

IMO, It would be fair to increase the bounty 125 - 250 as usual since there're many code changes and discussions cc @luacmartins @ShridharGoel

luacmartins commented 2 months ago

This was a simple change, so I think the current bounty of $125 is applicable here.

shawnborton commented 2 months ago

Not overdue, we're holding for payment

luacmartins commented 2 months ago

PR was deployed to prod on the 17th, so the payment due date is actually the 24th. cc @greg-schroeder

melvin-bot[bot] commented 2 months ago

@shawnborton, @luacmartins, @greg-schroeder, @ShridharGoel, @mountiny, @dukenv0307 Whoops! This issue is 2 days overdue. Let's get this updated quick!

greg-schroeder commented 2 months ago

Oh. Okay. I'll process this now then

greg-schroeder commented 2 months ago

@ShridharGoel please accept the offer in Upwork so I can pay you, thanks!

greg-schroeder commented 2 months ago

bump @ShridharGoel

greg-schroeder commented 2 months ago

Bump @ShridharGoel - moving this to Monthly given no response.

ShridharGoel commented 2 months ago

Accepted, thanks.

luacmartins commented 2 months ago

@greg-schroeder seems like the offer was accepted. Can we close this now?

greg-schroeder commented 2 months ago

Yes it does appear it was finally accepted! I'll close it after I confirm payment!

greg-schroeder commented 2 months ago

Paid