[$250] [HOLD for payment 2024-10-17] Report actions pagination improvements

[janicduplessis]

Problem

We currently have an issue where pagination methods (GetNewerActions, GetOlderActions) are called unconditionally even if we should know that no newer or older messages are available.

Solution

To improve this OpenReport, GetNewerActions, GetOlderActions should all return pagination info. This should be stored in the REPORT_METADATA onyx collection.

• For OpenReport it should be hasNewerActions and hasOlderActions.
• For GetNewerActions it should be hasNewerActions.
• For GetOlderActions it should be hasOlderActions.

Then in loadNewerChats and loadOlderChats we should check that newer / older actions are available before loading them.

This will reduce the call to these methods significantly. For regular chats we never should have newer actions since we start loading from the start of the chat so GetNewerActions will never be called. If we are in a short chat, we probably loaded all messages initially so we don't need to call GetOlderActions. For comment linking we should still call both GetNewerActions and GetOlderActions since we are probably in the middle of a chat.

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021845841739039769728
• Upwork Job ID: 1845841739039769728
• Last Price Increase: 2024-10-14

Issue Owner

Current Issue Owner: @

[melvin-bot]

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

[melvin-bot]

@janicduplessis, this Monthly task hasn't been acted upon in 6 weeks; closing.

If you disagree, feel encouraged to reopen it -- but pick your least important issue to close instead.

[roryabraham]

put a PR in review for this: https://github.com/Expensify/Auth/pull/12239

[janicduplessis]

@roryabraham Awesome, let me know when this is merged I could work on the app integration.

[roryabraham]

It's merged, should be deployed within the next 2-3 hours. I'll let you know

[roryabraham]

FWIW, I made the following changes:

1. GetOlderActions includes a hasOlderActions key in the response
2. GetNewerActions includes a hasNewerActions key in the response
3. If called with an "anchor" reportActionID, OpenReport includes both hasOlderActions and hasNewerActions

I'll test this locally and make sure it's working. Might need to make some changes in our "middle" PHP layer to make sure the data makes it to the front-end in the response.

[roryabraham]

Tested it and the new keys are not making it to the front-end. Looks like I'll need another PR in the PHP layer, but should be simple

[roryabraham]

For GetOlderActions and GetNewerActions it's an easy change. For OpenReport, not so much because there's a lot happening in the PHP layer and I might need to refactor some PHP functions that are used in many locations.

Alternatively, I can try to refactor OpenReport to be what we call 1:1:1 (or 1 user action = 1 php API call = 1 C++ Auth/DB command). That would likely take me several days. Will probably chat internally about the best path forward

[janicduplessis]

If called with an "anchor" reportActionID, OpenReport includes both hasOlderActions and hasNewerActions

I think OpenReport should always include hasOlderActions even with no anchor since for a small chat there might be no older messages to load initially.

[roryabraham]

Good point, I'll work on adjusting that, as soon as I figure out what to do with the OpenReport

[roryabraham]

I've spent a lot of time working on this. I do just need a single PHP PR, but it involves ripping out some deprecated code that has a lot of hidden side-effects. So it's been much more challenging than it seemed like it should have been.

At this point I have only one failing test that I'm trying to wrap my head around

[roryabraham]

Still stuck on that PR, but have been driving myself crazy trying to figure it out. Pushed a few changes that I think are sound, but still haven't gotten the last test passing.

[melvin-bot]

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

[melvin-bot]

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

• Use pagination metadata for report actions #49958

If no regressions arise, payment will be issued on 2024-10-17. 🎊

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

• @janicduplessis does not require payment (Contractor)
• @ishpaul777 requires payment (Needs manual offer from BZ)

[rlinoz]

I guess we need a BZ here

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

Current assignee @ishpaul777 is eligible for the External assigner, not assigning anyone new.

[CortneyOfstad]

@ishpaul777 — sent you an offer in Upwork, please let me know once you accept!

[ishpaul777]

Accepted Thanks!

[CortneyOfstad]

I am heading OoO (10/15 to 10/23), so going to get this reassigned so the payment can be made!

[melvin-bot]

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

[lschurr]

Payment summary:

• Contributor: @janicduplessis (no payment needed)
• Reviewer: $250 @ishpaul777 (paid in Upwork)

[lschurr]

Oops I read the date wrong and paid it today. We'll just leave this open to make sure there are no regressions.

[lschurr]

Closing :)