Current assignee @puneetlath is eligible for the NewFeature assigner, not assigning anyone new.

⚠️ It looks like this issue is labelled as a New Feature but not tied to any GitHub Project. Keep in mind that all new features should be tied to GitHub Projects in order to properly track external CAP software time ⚠️

Triggered auto assignment to Design team member for new feature review - @dubielzyk-expensify (NewFeature)

Confirm whether this is the complete list of keys to migrate or whether there are others

I'll be taking this part since it requires looking at the back-end also.

Hey, Tomasz from Callstack here, I can help with this issue

@TMisiukiewicz I need to confirm the full list that we're working with. To start, can you look into:

• Why do we have isPolicyExpenseChat? It seems redundant with type/chatType
• Why do we have isChatRoom? It seems redundant with type/chatType
• Whether there are others in the list above that seem redundant with existing data

@puneetlath I checked this list and seems like majority of these properties are generated in the runtime and describe the options for LHN, not reports directly. Seems like they are not stored in Onyx at all, might the Report type be outdated? I think it'd be good to actually go through the properties defined in a type and verify if they are still used anywhere in the project.

So, I'd say both properties are replaceable by referring to chatType when it comes to LHN options, but I don't see those stuff being moved from report to reportMetadata since they are not stored in Onyx

Oh wow, ok that's great to know.

I think it'd be good to actually go through the properties defined in a type and verify if they are still used anywhere in the project.

This makes sense to me. Want to do this as a first step? Basically, raise a PR to update the type with only the things that are actually used by the client.

Also, if the back-end attempts to merge something into Onyx that isn't in the type, do we log that?

This makes sense to me. Want to do this as a first step? Basically, raise a PR to update the type with only the things that are actually used by the client.

yeah sure, I'll start exploring it tomorrow 👍

So i verified all the properties and looks like there is 15 of them that should be possible to remove:

• icons
• lastMessageTimestamp
• lastReadCreated
• isPolicyExpenseChat
• openOnAdminRoom
• displayName
• errors
• isLastMessageDeletedParentAction
• iouReportAmount
• isChatRoom
• participantsList
• text
• selected
• participantAccountIDs
• visibleChatMemberAccountIDs

I’ll remove each unused property along with its references, one at a time, committing after each change to ensure stability.

Nice! Sounds good. Perhaps we should break it up into a few different PRs so that it's easier to revert if one of them causes a problem, without needing to revert it all.

Sure, i'll start with a PR with those that are not used anywhere as it's the easiest one

Opened first PR #52182, now I'll move to work on the properties that have some references in the codebase but do not seem to be a part of Report

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

• [No QA] chore: remove reportActionID from report type #53817

If no regressions arise, payment will be issued on 2024-12-19. 🎊

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

• @TMisiukiewicz does not require payment (Contractor)
• @shubham1206agra requires payment (Needs manual offer from BZ)

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:

• [@shubham1206agra] Please propose regression test steps to ensure the new feature will work correctly on production in further releases.
• [@muttmuure] Link the GH issue for creating/updating the regression test once above steps have been agreed upon.

@puneetlath just a quick heads-up, I tried building a simple, basic schema validator, just to validate simplest possible keys during set and merge.

I'll try to set this up with collections to be able to verify the performance overhead

I moved it to reportMetadata, however while working on it, I noticed it's another key that is used by createOption so it will make an additional call to Onyx for each report. I'll try to figure out if there is a good solution for that and what is the performance impact of such change

Sounds good.

I guess this might be used to display common.attachment translation in the LHN optimistically, however I haven't been able to reproduce it's usage. When I add an attachment, first it displays file name in the LHN, and then changes for [Attachment]. Maybe it's worth digging deeper and verifying if we need this property at all.

Good to know! It would be awesome if it turns out we can remove lastMessageTranslationKey altogether.

@puneetlath just a quick heads-up, I tried building a simple, basic schema validator, just to validate simplest possible keys during set and merge.
I'll try to set this up with collections to be able to verify the performance overhead

Very cool! Interested to see what you find!

Quick heads-up, I already implemented a simple solution for validating collection schema. Now I am measuring performance of moving pendingChatMembers to metadata and validating collections

Moving pendingChatMembers - I ran createOptionList on HT account which is going through all the reports and now hitting the reportMetadata - I got pretty much the same results on Android so seems like it does not affect its execution time. I also ran Reassure tests locally and I got some significant changes:

 - [ReportActionsList] should render ReportActionsList with 500 reportActions stored [render]: 75.1 ms → 87.7 ms ( 12.6 ms,  16.8%) 🔴 | 1 → 1 
 - SidebarUtils [SidebarUtils] getOrderedReportIDs on 15k reports for GSD priorityMode [function]: 217.0 ms → 229.6 ms ( 12.5 ms,  5.8%) 🔴 | 1 → 1 
 - SidebarLinks [SidebarLinks] should render Sidebar with 500 reports stored [render]: 62.6 ms → 75.0 ms ( 12.4 ms,  19.8%) 🔴 | 8 → 8 
 - SidebarUtils [SidebarUtils] getOrderedReportIDs on 15k reports for default priorityMode [function]: 231.4 ms → 239.5 ms ( 8.1 ms,  3.5%)  | 1 → 1 
 - SidebarLinks [SidebarLinks] should click on list item [render]: 61.2 ms → 68.0 ms ( 6.8 ms,  11.1%) 🔴 | 9 → 9 
 - [SelectionList] should render multiple selection and select 3 items [render]: 73.2 ms → 79.9 ms ( 6.7 ms,  9.2%) 🔴 | 8 → 8 
 - [BaseOptionsList] Should scroll and select a few items [render]: 73.4 ms → 79.5 ms ( 6.1 ms,  8.3%) 🔴 | 14 → 14 
 - [BaseOptionsList] Should render multiple selection and select 4 items [render]: 64.5 ms → 68.6 ms ( 4.1 ms,  6.4%) 🔴 | 13 → 13 
 - [SelectionList] should press a list item [render]: 44.8 ms → 48.8 ms ( 4.0 ms,  8.9%) 🔴 | 4 → 4 

They do not look very scary, it's mostly a matter of milliseconds. CI can give different results. I'll work on a migration file and open a PR

Opened PR for pendingChatMembers: #54247

Hmm if I haven't missed anything, lastMessageTranslationKey is used only for common.attachment as I said earlier. I think it should be fine to remove it, I'll work on opening a PR

opened #54295

Payment Summary

Upwork Job

• Contributor: @TMisiukiewicz is from an agency-contributor and not due payment
• ROLE: @shubham1206agra paid $(AMOUNT) via Upwork (LINK)

BugZero Checklist (@muttmuure)

• I have verified the correct assignees and roles are listed above and updated the neccesary manual offers
• I have verified that there are no duplicate or incorrect contracts on Upwork for this job (https://www.upwork.com/ab/applicants//hired)
• I have paid out the Upwork contracts or cancelled the ones that are incorrect
• I have verified the payment summary above is correct

@muttmuure we've had a lot of PRs here and we're almost to the end. I think we can pay it all out in one batch to Shubham at the end.

@puneetlath I did some performance measurements for runtime validation.

What I did was:

• implementing basic schema validation in Onyx for one boolean key and for one collection
• extended init command with two new properties: schema, responsible for defining schema and onSchemaError, to be able to e.g. use it with Logger
• connected schema validation with merge and set methods

I did measurements for calling merge on report_ key on iOS and web, here are the results for iOS:

Seems like enabling it makes single merge calls much slower. I assume when calling multiple merge/set calls at the same time/using multiSet it might look the same.

Some other thoughts about it after building PoC:

• we could do this incrementally (key by key), but for more complex keys (like report), we have to implement them entirely per PR. Otherwise it will throw errors because app is trying to merge properties that does not exist in schema
• this might be a good tool to find out discrepancies between types. After implementing I found out that lastActorAccountID expects a string but got number when I'm trying to send a message
• the worst part is that I haven't found any tool that would help us automatically generate schemas based on our types. Ts-to-zod that I mentioned earlier does not support generics so it won't help. There is one more alternative that I could possibly investigate which is https://github.com/vega/ts-json-schema-generator . If not this one, it would mean we have to write all the schemas on our own and then maintain them. This seems like something very time-consuming.

If you want to test it on your own, you can check my fork of Exfy with onyx-schema branch: main...TMisiukiewicz:Expensify-App:onyx-schema together with react-native-onyx branch: Expensify/react-native-onyx@main...callstack-internal:react-native-onyx:onyx-schema

Let me know your thoughts 👀 FYI I'll be mostly OOO during the upcoming two weeks, I'll be back just for one day on December 30th.

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

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

• remove lastMessageTranslationKey from report collection #54295

If no regressions arise, payment will be issued on 2025-01-02. 🎊

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

• @TMisiukiewicz does not require payment (Contractor)
• @shubham1206agra requires payment (Needs manual offer from BZ)

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:

• [@shubham1206agra] Please propose regression test steps to ensure the new feature will work correctly on production in further releases.
• [@muttmuure] Link the GH issue for creating/updating the regression test once above steps have been agreed upon.

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