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

Report title formula is different in NewDot #44340

Open m-natarajan opened 2 months ago

m-natarajan commented 2 months 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: 9.0.1-10 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: @heyjennahay Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1719222853627669

Action Performed:

  1. Create a workspace and set the report title format in the OD
  2. Submit reports in the workspace
  3. Check the report title

    Expected Result:

    Should match with one set in OD

    Actual Result:

    Showing same format Expense Report #{long report ID] for all the reports

    Workaround:

    unknown

    Platforms:

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

    • [ ] Android: Native
    • [ ] Android: mWeb Chrome
    • [ ] iOS: Native
    • [ ] iOS: mWeb Safari
    • [x] MacOS: Chrome / Safari
    • [ ] MacOS: Desktop

Screenshots/Videos

image (21)

image (20)

View all open jobs on GitHub

Issue OwnerCurrent Issue Owner: @thienlnam
melvin-bot[bot] commented 2 months ago

Triggered auto assignment to @trjExpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

trjExpensify commented 2 months ago

@caitlinwhite1 @thienlnam is this on your radar? Will it need to be Internal?

Adding it to #wave-control in the meantime. CC: @JmillsExpensify

thienlnam commented 2 months ago

Is there a photo of what the report title looks like in NewDot?

IIRC this is partially expected. NewDot generates the report title based on the formula optimistically since it could be offline, but the thing it can't support is long report IDs, or anything that requires server side knowledge.

However, if you are online it should just take the report name. So depending on the circumstances this is likely external

heyjennahay commented 2 months ago

If I look at the reports in my personal online web NewDot they have the incorrect format

image

thienlnam commented 2 months ago

Oh gotcha, so it's not just a visual bug in NewDot.

Hmm, I was testing this on dev and it seems to work alright for me.

Screenshot 2024-06-26 at 10 24 12 AM

My only other guess is NewDot Test - Expensify a collect policy not a control policy?

heyjennahay commented 2 months ago

@thienlnam can you please check your personal NewDot Test reports in NewDot? To confirm your theory about the Collect v Control workspace being the cause? My NL Test workspace is Collect but the US Test workspace has been Control for about a month now so if your theory is correct then I would expect to see a difference.

That said, why would the formula not be applied to both Collect and Control workspaces (this feature applies to both types in OldDot today)? Also if the intention is to require you to upgrade to Control to use Report Formulas then I would still expect the default report title for Collect workspaces to show the short report ID and not the long. So I still think there is an issue that needs to be resolved.

thienlnam commented 2 months ago

I agree there's a bug here, just trying to isolate where the problem might be. It does seem to be a control policy so that rules out that theory.

Screenshot 2024-06-26 at 1 20 03 PM

So it looks like the report fields are not being returned into this policy - going to have some queries run to see why that is the case. This would be internal

Screenshot 2024-06-26 at 1 24 43 PM
thienlnam commented 2 months ago

Created an internal GH for a query request here

trjExpensify commented 2 months ago

Nice, what are the next steps?

trjExpensify commented 2 months ago

Bump on this, @thienlnam.

thienlnam commented 2 months ago

Just got back from OOO - got the results of the query back and everything looks as expected.

However the report field is still not getting returned in NewDot so there is likely something wrong in the flow.

Next is investigating the query to see if there's something that might be impacting the return of this data

melvin-bot[bot] commented 2 months ago

@trjExpensify @thienlnam 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!

thienlnam commented 2 months ago

Haven't looked into this further yet

trjExpensify commented 2 months ago

Any luck?

thienlnam commented 2 months ago

Sorry, haven't looked into this further due to instantscan. I've also got app deploy duty this week, so I am hoping to get around to this sometime next week. Feel free to re-assign if needed

heyjennahay commented 2 months ago

Noticed this is also happening on reports created on our OldDot production workspace when they are created via Expensify Travel. Example report ID 6471996360309383

image

Although I just found one which isn't travel or NewDot testing report ID 8929959707960611 image

trjExpensify commented 2 months ago

@thienlnam putting this customer bug report on your radar as potentially related: https://github.com/Expensify/Expensify/issues/413468

melvin-bot[bot] commented 1 month ago

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

thienlnam commented 1 month ago

Looks related, I think the flow is broken on OldDot too

trjExpensify commented 1 month ago

Okay, well this might be a CRITICAL bug where we're messing with the report title formulas set on OldDot. CC: @JmillsExpensify

thienlnam commented 1 month ago

Okay as I was looking into this again, this has just started to work again. Like all my reports are now created with the correct report name. Are you seeing the same? After Jul 16th all of my reports have started to get the correct reportName

Screenshot 2024-07-23 at 4 56 43 PM

melvin-bot[bot] commented 1 month ago

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

thienlnam commented 1 month ago

This seems to be resolved, but wanted to get some confirmation

trjExpensify commented 1 month ago

@heyjennahay can maybe confirm from accounting perspective on the prod policy.

Interestingly, I was just testing something and I'm seeing this in the report history:

image

I didn't take that action myself. Not sure where it's coming from.. 🤔

thienlnam commented 1 month ago

Oh yeah looks like we should not generate a policy history when a report is updated from a template value

trjExpensify commented 1 month ago

That's a reportAction though on the report, not a workspace change log in #admins.

thienlnam commented 1 month ago

Ah whoops, yeah I mean no edited reportAction when a title is updated from a template value on a report

trjExpensify commented 1 month ago

Cool, let's address that then. 👍

melvin-bot[bot] commented 1 month ago

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

thienlnam commented 1 month ago

There was a conversation about this here - https://expensify.slack.com/archives/C06ML6X0W9L/p1722534299058619 and here https://expensify.slack.com/archives/C06ML6X0W9L/p1722534322032729

Turns out this will require moving the report title template generation to auth

melvin-bot[bot] commented 1 month ago

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

JmillsExpensify commented 1 month ago

Ok so sounds like I should remove this from Hot Picks for now, and then we'll need to start prioritizing report fields in Auth – probably around the November release since we're pretty full on initiatives until then. Thoughts on that approach?

thienlnam commented 1 month ago

That sounds good to me

melvin-bot[bot] commented 1 month ago

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

thienlnam commented 1 month ago

Updating priority based on this, we will re-prioritize Auth Report Fields in Nov