Skip to content

[v1] Remove sequenceNumber dependent functions - #932

Merged
chiragsalian merged 12 commits into
Expensify:chirag-mobile-oldfrom
s77rt:v1-remove-sequenceNumber
Jul 23, 2026
Merged

[v1] Remove sequenceNumber dependent functions#932
chiragsalian merged 12 commits into
Expensify:chirag-mobile-oldfrom
s77rt:v1-remove-sequenceNumber

Conversation

@s77rt

@s77rt s77rt commented Jul 17, 2026 •
edited
Loading

Copy link
Copy Markdown
Member
  • Replaced mergeItems with mergeHistoryByTimestamp
  • Replaced insertIntoCache with insertIntoCacheByActionID
  • Added mergeHistoryByReportActionID
  • Added getFlatHistory
  • Deprecated get and made it an alias to getFlatHistory

(some of changes here are same as #929)

Fixed Issues

https://github.com/Expensify/Expensify/issues/589297

Tests

n/a

QA

n/a

@s77rt
s77rt marked this pull request as ready for review July 17, 2026 15:04
@s77rt
s77rt requested a review from a team as a code owner July 17, 2026 15:04
@melvin-bot
melvin-bot Bot requested review from mjasikowski and removed request for a team July 17, 2026 15:04
@s77rt

s77rt commented Jul 17, 2026

Copy link
Copy Markdown
Member Author

@chiragsalian I need to merge this into a separate branch. Can you please create a branch based on 2e5cff5 (this is what we use in mobile OD - very old). This easier than updating the OD app to use what we have now

@chiragsalian

chiragsalian commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

cool, i created it here @s77rt - https://github.com/Expensify/expensify-common/tree/chirag-mobile-old

@s77rt
s77rt changed the base branch from main to chirag-mobile-old July 17, 2026 19:55
@s77rt

s77rt commented Jul 17, 2026

Copy link
Copy Markdown
Member Author

Thanks! I have changed the base to target that branch. This is good for review. I don't know how to handle the failing tests (seems related to the running environment)

@chiragsalian

chiragsalian commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

I don't know how to handle the failing tests

i pushed the fixes for that to my branch just merge the base branch to you branch.

@s77rt

s77rt commented Jul 17, 2026

Copy link
Copy Markdown
Member Author

Thank you! Just merged

chiragsalian
chiragsalian reviewed Jul 17, 2026
View reviewed changes
Comment thread lib/ReportHistoryStore.jsx
* @param {Number} reportID
* @param {Object[]} newHistory
*/
mergeHistoryByReportActionID(reportID, newHistory) {

@chiragsalian chiragsalian Jul 17, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

where is this used?

@s77rt s77rt Jul 17, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not needed atm, I just copied this from the current version just in case we needed it.

@chiragsalian chiragsalian Jul 23, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i doubt we need it. But either way. Sure let's proceed.

@s77rt
s77rt requested a review from chiragsalian July 23, 2026 03:07
@chiragsalian

chiragsalian commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

oops sorry i missed this. Thanks for the re-request.

@chiragsalian
chiragsalian merged commit 9c17c3d into Expensify:chirag-mobile-old Jul 23, 2026
4 checks passed
@jjcoffee jjcoffee mentioned this pull request Sep 10, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@chiragsalian chiragsalian chiragsalian approved these changes
@mjasikowski mjasikowski Awaiting requested review from mjasikowski

Assignees

@s77rt s77rt

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@s77rt @chiragsalian @quinthar