Skip to content

Plan and implement a system for optimistic report title formula computation #59689

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

Open
neil-marcellini opened this issue Apr 4, 2025 · 22 comments
Assignees
Labels
Daily KSv2 External Added to denote the issue can be worked on by a contributor NewFeature Something to build that is a new item. Overdue

Comments

@neil-marcellini
Copy link
Contributor

neil-marcellini commented Apr 4, 2025

Problem

We have been working on implementing automatic report titles for new Expensify to match the feature on classic. Automatic report titles use a formula defined the policy to compute the report name and the report name is continually updated as data changes. The formulas are made up of various different parts such as {report:type}, report:startdate, etc. You can check out how this feature works by going to Expensify Classic (expensify.com), workspace settings, choosing the report tab on the left side, and scrolling down to the automatic report title section. There is also a help site page linked there with more information. We're also updating the feature so that it sends out real time updates versus requiring a page refresh or loading the report.

In order to maintain our standards of an optimistic offline first application, we will need to implement report formula computation on the front end. When a new report is created, it should optimistically have the automatic report title set based on the formula in the policy settings. Whenever any data is optimistically updated such as the amount of an expense, for example, then the report title should update if it depends on that data. One example would be a formula that references the report total.

We will gradually be rolling out support for more formula parts in new Expensify, and the entire feature will be under a beta flag while development is still active. Please stay in touch with me (@neil-marcellini) for updates on our backend progress.

Solution

Let's design a system to optimistically update the report title based on the formula, and integrate it into the application such that any API request will be automatically handled. It might make a good middleware for example. Once the system is designed, implement it for a few report formula parts that we already support like report type, start date and total. After that, we can distribute the implementation across a large number of contributors to get the remaining parts done faster.

The system should be under a beta to start, and we'll let you know the name of the beta as soon as it is created.

Issue OwnerCurrent Issue Owner: @sosek108
@neil-marcellini neil-marcellini added External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors labels Apr 4, 2025
@neil-marcellini neil-marcellini self-assigned this Apr 4, 2025
Copy link

melvin-bot bot commented Apr 4, 2025

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

@melvin-bot melvin-bot bot added the Daily KSv2 label Apr 4, 2025
@neil-marcellini neil-marcellini added the NewFeature Something to build that is a new item. label Apr 4, 2025
Copy link

melvin-bot bot commented Apr 4, 2025

Triggered auto assignment to @bfitzexpensify (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.

@sosek108
Copy link
Contributor

@neil-marcellini Hi, I'm Hubert from Callstack - expert contributor group - and I would like to take a look at this issue.

@melvin-bot melvin-bot bot added the Overdue label Apr 22, 2025
@neil-marcellini
Copy link
Contributor Author

Ok, you're assigned. Please post an initial plan, either in a GH comment or a google doc if that's easier.

@melvin-bot melvin-bot bot removed Help Wanted Apply this label when an issue is open to proposals by contributors Overdue labels Apr 25, 2025
@neil-marcellini neil-marcellini added Daily KSv2 and removed Weekly KSv2 labels Apr 25, 2025
@melvin-bot melvin-bot bot added the Overdue label Apr 28, 2025
@sosek108
Copy link
Contributor

sosek108 commented Apr 28, 2025

High-Level Strategy

We need a front-end system that:

  • Parses the report title formula. to check for potential code reuse from OD
  • Tracks the required dependencies (fields like report total, start date, etc.). to check for potential code reuse from OD
  • Updates the report title optimistically and reactively (no page refresh).
  • Integrates seamlessly with API requests via middleware
  • Starts under a beta flag for safe rollout.
  • Support these parts first:
    • {report:type}
    • {report:startdate}
    • {report:total}

Phase 1 - foundation

  • define a dependency mapping (which parts of the formula depend on which report fields.)
  • build a lightweight parser that extracts parts (e.g., {report:type}, {report:startdate}, {report:total}) and map these into report fields
  • create an extendable list of fields that should be watched on create / update

Phase 2 - integration

  • build a middleware inside prepareRequest that
    • Intercepts report-related updates
    • determines if name should be updated
      • automatic report naming method is enabled
      • user did not change name
      • is not a chat
      • is not approved
    • Detects if the update affects any fields tied to report title formula.
    • Re-computes and updates the report title optimistically.
    • determines if name should be updated
  • wrap implementation under a beta flag

Phase 3 - quality assurance

  • create unit and reassure performance tests
  • ensure proper error handling

Phase 4 - continous integration

  • write information how to add support for new formula parts
  • optional add dedicated view that user can go to to check what option does he have. This would replace current Help page and give user the ability to peek on options without leaving the application
  • distribute work on implementation across contributors

@sosek108
Copy link
Contributor

sosek108 commented Apr 28, 2025

@neil-marcellini Let me know if I can describe anything in more clear fashion.

🌴 Please be informed that I will be off on vacation since Wednesday until Monday. I can start this first thing after my return

@neil-marcellini
Copy link
Contributor Author

Thanks @sosek108, that's a pretty good start to the plan. I will look at classic code to see if there's any existing JS code that parses report formulas. I don't think there is since classic doesn't do optimistic updates, buy maybe. If not, I will copy some of the existing backend implementation to a private google doc and share it with you for reference to design the frontend system.

I like your idea to create a dependency mapping. Here is a small example of that which I recently made for a couple formulas.

Formula Part Regex Attributes
report:policyname policy.name
report:submit.*:(userid|customfield1|payrollid|customField2)\b policy.glCodes
report:autoreporting policy.autoReporting, policy.autoReportingFrequency, policy.autoReportingOffset

Keep in mind that report formulas should not update if the report has been approved, which also means that the reimbursement related formulas aren't really supported. We have an issue to decide if we're going to deprecate that or fix it, but we'll leave as is for now.

Here is the google doc with the JS code from classic. We only really care about the report fields in this case, expense can be ignored.

Would you please go into more detail about the middleware / optimistic update system and how it will work? Maybe middleware is the wrong approach since we only need to handle optimistic updates. Sorry if I mislead you there. Perhaps it should just be a layer that subscribes to various Onyx collections and makes updates as needed? Or maybe adjusting utils to build optimistic values will be enough?

Let's take the formula part {report:total} as an initial example. It depends on report transactions. Whenever any transactions are optimistically updated on the report, we need to optimistically recompute the title accordingly. It may be sufficient to do that in the functions which build optimistic transactions or reports. The backend will be sending title updates when the values change on the server. Whenever the report title formula in the policy changes, all eligible reports (non-chat and non-approved), need to have their titles updated according to the new formula. Please describe how the system will work in these cases. Also include an example for a formula part that depends on some attributes of the report, and one that depends on attributes of the policy.

For the beta, let's start with a hard coded list of emails to which the beta applies. It's slightly different than normal betas but that's the approach we're using on the backend for efficiency, since the value needs to be checked on many requests.

  • build a simple form in Workspace config to write custom formulas (simple text field at this moment)

We already have this under workspace rules, which is admittedly a bit hard to find.

Image

Please take all that into consideration and update your plan in the original GH comment.

@melvin-bot melvin-bot bot removed the Overdue label Apr 28, 2025
@sosek108
Copy link
Contributor

sosek108 commented Apr 29, 2025

Thanks @neil-marcellini for elaborated response. That is very helpful.

First of all - I have access to doc and thanks for that!

Keep in mind that report formulas should not update if the report has been approved, which also means that the reimbursement related formulas aren't really supported.

Our mechanism wheter its middleware / class / hook could have additional method to decide if name should update eg. nameShouldUpdate. This will help us keep code extensible.
What if an user manually change report name? I belive that in this case we shouldn't update report name

Would you please go into more detail about the middleware / optimistic update system and how it will work? Maybe middleware is the wrong approach since we only need to handle optimistic updates. Sorry if I mislead you there. Perhaps it should just be a layer that subscribes to various Onyx collections and makes updates as needed? Or maybe adjusting utils to build optimistic values will be enough?

Questions about backend. Backend will discard our report name and return this computed on BE side. If report changes due to other enity modification (eg. Workspace name modified) will application get Pusher update for new report name or should we recompute that?

I see several ways to tackle that:

A: we stay with the middleware idea and we use that only to intercept all updates and we can create holistic solution that could intercept every change based on configuration (we could configure it to watch for workspace name change).

  • pros: holistic way to catch everything, easy to test
  • cons: update to report name will be generated after users' optimistic change is posted to UI so there will be another update. Solution is quite overkill. Solution would require changes in generic application's code unrelated to domain logic

B: we implement small function that should be used on every action we would to run this action to.

  • pros: simple implementation, we can
  • cons: we need to edit every place that could trigger report name update. Our job would be to predict these places and create a list of tasks to perform and this list could be spread across contributors..

C: we could subsribe to Onyx for every key we would like to investigate and change report name based on these changes

  • cons: these changes could be late and therefore backend could respond earlier.
  • pros: one place to modify them all.

For the beta, let's start with a hard coded list of emails to which the beta applies. It's slightly different than normal betas but that's the approach we're using on the backend for efficiency, since the value needs to be checked on many requests.

I will appreciate every help on how it's done in Expensify as this is my first time to create beta flag for new feature.

@sosek108
Copy link
Contributor

@neil-marcellini Kind reminder that I start vacation and I'll be back on tuesday. I will work full time on this if we agree on details

@neil-marcellini
Copy link
Contributor Author

Ah shoot I was 90% done writing a reply to you, then restarted my browser and forgot to save it 😢. I'll have to get back to you later.

@neil-marcellini
Copy link
Contributor Author

Our mechanism wheter its middleware / class / hook could have additional method to decide if name should update eg. nameShouldUpdate. This will help us keep code extensible.

Yeah sure. But also I don't think we'll reuse this system for anything else so I don't think it needs to necessarily be configurable. We can define that logic directly in the function.

What if an user manually change report name? I belive that in this case we shouldn't update report name

I'll check. You can always experiment on classic (staging.expensify.com/inbox) with a test account.

Yeah so the name isn't updated if the user has manually set a report name and the policy formula is updated. However the name will be updated to use the formula if enforcement is enabled.

2025-04-30_14-02-41.mp4

@neil-marcellini
Copy link
Contributor Author

Questions about backend. Backend will discard our report name and return this computed on BE side. If report changes due to other enity modification (eg. Workspace name modified) will application get Pusher update for new report name or should we recompute that?

Yeah I wouldn't say "discard" but the BE will compute the title itself and send an update to overwrite the optimistic data. It will also send Pusher updates for those cases. This issue is only about handling the optimistic updates when queueing API write requests to change data the formula references.

B: we implement small function that should be used on every action we would to run this action to.

  • pros: simple implementation, we can
  • cons: we need to edit every place that could trigger report name update. Our job would be to predict these places and create a list of tasks to perform and this list could be spread across contributors..

I like this approach best because it seems to be the most simple and we have a relatively limited number of object types which the formula can depend on. These include but are not limited to report, transaction, policy, account, personalDetails. It also prevents issues where these updates happen after the initial optimistic updates.

However I would like to suggest another option which has those benefits without the other drawbacks. It's basically option A but runs before the request. Within prepareRequest before we apply the optimistic data, we could run a function on it which would identify any updates to the formula dependencies, then add optimistic updates for the titles. I think we'll be able to make it relatively efficient by doing only one pass over the updates and returning early if no extra updates are needed. It's also a nice way to centralize it.

@melvin-bot melvin-bot bot added the Overdue label May 5, 2025
Copy link

melvin-bot bot commented May 6, 2025

@sosek108 Eep! 4 days overdue now. Issues have feelings too...

@sosek108
Copy link
Contributor

sosek108 commented May 6, 2025

I like the idea with prepareRequest. My initial implementation would use that

@melvin-bot melvin-bot bot removed the Overdue label May 6, 2025
@sosek108
Copy link
Contributor

sosek108 commented May 6, 2025

@neil-marcellini I've updated my initial plan.

I've modified Phase 1 and Phase 2 to incorporate our discussed changes. As Phase 1 is quite simple we would merge P1 and P2.

@neil-marcellini
Copy link
Contributor Author

Ok great thanks. I'm going to hold off on reviewing that for a little bit, because I have decided that we should write a design doc for the backend and frontend changes. That's our standard practice and I was seeing how far I could get without it, but it's reached a level of complexity where I think that's the best path forward. Once I create that I'll share it with you and you can help me write it.

@sosek108
Copy link
Contributor

sosek108 commented May 7, 2025

To be sure. I should hold implementation phase until you return to me with design doc draft, right?

@neil-marcellini
Copy link
Contributor Author

Yes please

Copy link

melvin-bot bot commented May 12, 2025

@sosek108 Huh... This is 4 days overdue. Who can take care of this?

@melvin-bot melvin-bot bot added the Overdue label May 12, 2025
Copy link

melvin-bot bot commented May 14, 2025

@sosek108 6 days overdue. This is scarier than being forced to listen to Vogon poetry!

Copy link

melvin-bot bot commented May 14, 2025

⚠️ 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.

Copy link

melvin-bot bot commented May 16, 2025

@sosek108 Now this issue is 8 days overdue. Are you sure this should be a Daily? Feel free to change it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Daily KSv2 External Added to denote the issue can be worked on by a contributor NewFeature Something to build that is a new item. Overdue
Projects
Status: HIGH
Development

No branches or pull requests

4 participants