-
Notifications
You must be signed in to change notification settings - Fork 3.9k
Implementing optimistic category and tag workspace violations #74389
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
Merged
Merged
Changes from all commits
Commits
Show all changes
40 commits
Select commit
Hold shift + click to select a range
4fb4f7b
Revert "Revert "Implementing optimistic violations for workspace cate…
Tony-MK 8c72145
refactor: ReportUtils performance tests to initialize Onyx derived va…
Tony-MK 75b1dd1
refactor: reverted ReportUtils.ts
Tony-MK 993ee5e
Merge branch 'main' of https://github.com/Expensify/App into violaions
Tony-MK 7057aa6
refactor: pushTransactionViolationsOnyxData
Tony-MK 19422e8
refactor: update ReportUtils to use SearchReport type in tests
Tony-MK 41d32db
lint
Tony-MK 0500e10
refactor: update import statement for ReportTransactionsAndViolations…
Tony-MK 5ecaa6c
refactor: extract reportsSelector function for better report filterin…
Tony-MK 33d55af
Merge branch 'main' of https://github.com/Expensify/App into violaions
Tony-MK b4cdd43
refactor: update import statements for Onyx types in usePolicyData an…
Tony-MK 92f32c0
Merge branch 'main' of https://github.com/Expensify/App into violaions
Tony-MK 612d441
lint
Tony-MK 3cae81e
test: clean up whitespace in pushTransactionViolationsOnyxData test
Tony-MK 603123c
refactor: change import statement for PolicyData to use type
Tony-MK c9c43ab
refactor: rename parameter for clarity in reportsSelector and usePoli…
Tony-MK 3ecb5b3
Merge branch 'main' of https://github.com/Expensify/App into violaions
Tony-MK 30bf860
refactor: usePolicyData to use a stable selector for reports and impr…
Tony-MK 0285ecb
refactor: clean up usePolicyData and improve report selector logic
Tony-MK 712b8c4
refactor: remove unused type import from usePolicyData
Tony-MK c34918a
Merge branch 'main' into violaions
Tony-MK 67261cd
Merge branch 'main' of https://github.com/Expensify/App into violaions
Tony-MK cc7863f
refacto: IOURequestStepCategory component for improved readability an…
Tony-MK acf3c0c
refactor: update dependencies
Tony-MK a9abd8a
Refactor dependencies in useEffect hook
Tony-MK 28eb2f6
deps
Tony-MK b4a0438
Merge branch 'violaions' of https://github.com/Tony-MK/Expensify-App …
Tony-MK 558157c
refactor: enhance usePolicyData hook with memoization and stable refe…
Tony-MK 35e7200
refactor: clean up unused components and improve code organization
Tony-MK 0f1a84b
Merge branch 'main' of https://github.com/Expensify/App into violaions
Tony-MK 95f86a8
prettier
Tony-MK f57d0e9
Merge branch 'main' into violaions
Tony-MK 11cc151
Merge branch 'main' into violaions
Tony-MK 3b2c2d0
Merge branch 'Expensify:main' into violaions
Tony-MK 293a34b
refactor: optional chaining for policy ID retrieval in Category and T…
Tony-MK 103b01e
refactor: simplify condition checks for category deletion and policy …
Tony-MK 54f07d8
refactor: add comments to suppress lint runOnUI deprecation warnings …
Tony-MK 7545ce4
Revert "refactor: add comments to suppress lint runOnUI deprecation w…
Tony-MK 58792ca
Merge branch 'Expensify:main' into violaions
Tony-MK ceb3e46
Merge branch 'Expensify:main' into violaions
Tony-MK File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,63 @@ | ||
| import {useCallback, useMemo} from 'react'; | ||
| import type {OnyxCollection} from 'react-native-onyx'; | ||
| import {useAllReportsTransactionsAndViolations} from '@components/OnyxListItemProvider'; | ||
| import useOnyx from '@hooks/useOnyx'; | ||
| import usePolicy from '@hooks/usePolicy'; | ||
| import ONYXKEYS from '@src/ONYXKEYS'; | ||
| import type {Policy, Report} from '@src/types/onyx'; | ||
| import type {ReportTransactionsAndViolationsDerivedValue} from '@src/types/onyx/DerivedValues'; | ||
| import type {OnyxValueWithOfflineFeedback} from '@src/types/onyx/OnyxCommon'; | ||
| import type PolicyData from './types'; | ||
|
|
||
| /** | ||
| * Retrieves policy tags, categories, reports and their associated transactions and violations. | ||
| * @param policyID The ID of the policy to retrieve data for. | ||
| * @returns An object containing policy data | ||
| */ | ||
| function usePolicyData(policyID?: string): PolicyData { | ||
| const policy = usePolicy(policyID); | ||
| const allReportsTransactionsAndViolations = useAllReportsTransactionsAndViolations(); | ||
|
|
||
| // Stable selector for useOnyx to avoid defining the selector inline | ||
| const reportsSelectorCallback = useCallback( | ||
| (allReports: OnyxCollection<Report>) => { | ||
| if (!policyID || !allReports || !allReportsTransactionsAndViolations) { | ||
| return {}; | ||
| } | ||
|
|
||
| // Filter reports to only include those that belong to the specified policy and have associated transactions | ||
| return Object.keys(allReportsTransactionsAndViolations).reduce<Record<string, Report>>((acc, reportID) => { | ||
| const policyReport = allReports[`${ONYXKEYS.COLLECTION.REPORT}${reportID}`]; | ||
| if (policyReport?.policyID === policyID) { | ||
| acc[reportID] = policyReport; | ||
| } | ||
| return acc; | ||
| }, {}); | ||
| }, | ||
| [policyID, allReportsTransactionsAndViolations], | ||
| ); | ||
|
|
||
| const [tags] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY_TAGS}${policyID}`, {canBeMissing: true}, [policyID]); | ||
| const [categories] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY_CATEGORIES}${policyID}`, {canBeMissing: true}, [policyID]); | ||
| const [reports] = useOnyx(ONYXKEYS.COLLECTION.REPORT, {canBeMissing: true, selector: reportsSelectorCallback}, [policyID, allReportsTransactionsAndViolations]); | ||
| const transactionsAndViolations = useMemo(() => { | ||
| if (!reports || !allReportsTransactionsAndViolations) { | ||
| return {}; | ||
| } | ||
| return Object.keys(reports).reduce<ReportTransactionsAndViolationsDerivedValue>((acc, reportID) => { | ||
| if (allReportsTransactionsAndViolations[reportID]) { | ||
| acc[reportID] = allReportsTransactionsAndViolations[reportID]; | ||
| } | ||
| return acc; | ||
| }, {}); | ||
| }, [reports, allReportsTransactionsAndViolations]); | ||
| return { | ||
| transactionsAndViolations, | ||
| tags: tags ?? {}, | ||
| categories: categories ?? {}, | ||
| policy: policy as OnyxValueWithOfflineFeedback<Policy>, | ||
| reports: Object.values(reports ?? {}) as Array<OnyxValueWithOfflineFeedback<Report>>, | ||
| }; | ||
| } | ||
|
|
||
| export default usePolicyData; | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| import type {Policy, PolicyCategories, PolicyTagLists, Report} from '@src/types/onyx'; | ||
| import type {ReportTransactionsAndViolationsDerivedValue} from '@src/types/onyx/DerivedValues'; | ||
| import type {OnyxValueWithOfflineFeedback} from '@src/types/onyx/OnyxCommon'; | ||
|
|
||
| type PolicyData = { | ||
| policy: OnyxValueWithOfflineFeedback<Policy>; | ||
| tags: PolicyTagLists; | ||
| categories: PolicyCategories; | ||
| reports: Array<OnyxValueWithOfflineFeedback<Report>>; | ||
| transactionsAndViolations: ReportTransactionsAndViolationsDerivedValue; | ||
| }; | ||
|
|
||
| export default PolicyData; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -23,6 +23,8 @@ | |
| import type {LocaleContextProps} from '@components/LocaleContextProvider'; | ||
| import type {MoneyRequestAmountInputProps} from '@components/MoneyRequestAmountInput'; | ||
| import type {TransactionWithOptionalSearchFields} from '@components/TransactionItemRow'; | ||
| import type PolicyData from '@hooks/usePolicyData/types'; | ||
| import type {PolicyTagList} from '@pages/workspace/tags/types'; | ||
| import type {ThemeColors} from '@styles/theme/types'; | ||
| import type {IOUAction, IOUType, OnboardingAccounting} from '@src/CONST'; | ||
| import CONST, {TASK_TO_FEATURE} from '@src/CONST'; | ||
|
|
@@ -47,6 +49,7 @@ | |
| PolicyCategory, | ||
| PolicyReportField, | ||
| PolicyTagLists, | ||
| PolicyTags, | ||
| Report, | ||
| ReportAction, | ||
| ReportAttributesDerivedValue, | ||
|
|
@@ -58,8 +61,8 @@ | |
| Task, | ||
| Transaction, | ||
| TransactionViolation, | ||
| TransactionViolations, | ||
| } from '@src/types/onyx'; | ||
| import type {ReportTransactionsAndViolations} from '@src/types/onyx/DerivedValues'; | ||
| import type {Attendee, Participant} from '@src/types/onyx/IOU'; | ||
| import type {SelectedParticipant} from '@src/types/onyx/NewGroupChatDraft'; | ||
| import type {OriginalMessageExportedToIntegration} from '@src/types/onyx/OldDotAction'; | ||
|
|
@@ -951,7 +954,7 @@ | |
| const parsedReportActionMessageCache: Record<string, string> = {}; | ||
|
|
||
| let conciergeReportID: OnyxEntry<string>; | ||
| Onyx.connect({ | ||
| key: ONYXKEYS.CONCIERGE_REPORT_ID, | ||
| callback: (value) => { | ||
| conciergeReportID = value; | ||
|
|
@@ -959,7 +962,7 @@ | |
| }); | ||
|
|
||
| const defaultAvatarBuildingIconTestID = 'SvgDefaultAvatarBuilding Icon'; | ||
| Onyx.connect({ | ||
| key: ONYXKEYS.SESSION, | ||
| callback: (value) => { | ||
| // When signed out, val is undefined | ||
|
|
@@ -977,7 +980,7 @@ | |
| let allPersonalDetails: OnyxEntry<PersonalDetailsList>; | ||
| let allPersonalDetailLogins: string[]; | ||
| let currentUserPersonalDetails: OnyxEntry<PersonalDetails>; | ||
| Onyx.connect({ | ||
| key: ONYXKEYS.PERSONAL_DETAILS_LIST, | ||
| callback: (value) => { | ||
| if (currentUserAccountID) { | ||
|
|
@@ -989,14 +992,14 @@ | |
| }); | ||
|
|
||
| let allReportsDraft: OnyxCollection<Report>; | ||
| Onyx.connect({ | ||
| key: ONYXKEYS.COLLECTION.REPORT_DRAFT, | ||
| waitForCollectionCallback: true, | ||
| callback: (value) => (allReportsDraft = value), | ||
| }); | ||
|
|
||
| let allPolicies: OnyxCollection<Policy>; | ||
| Onyx.connect({ | ||
| key: ONYXKEYS.COLLECTION.POLICY, | ||
| waitForCollectionCallback: true, | ||
| callback: (value) => (allPolicies = value), | ||
|
|
@@ -1011,7 +1014,7 @@ | |
|
|
||
| let allReports: OnyxCollection<Report>; | ||
| let reportsByPolicyID: ReportByPolicyMap; | ||
| Onyx.connect({ | ||
| key: ONYXKEYS.COLLECTION.REPORT, | ||
| waitForCollectionCallback: true, | ||
| callback: (value) => { | ||
|
|
@@ -1049,14 +1052,14 @@ | |
| }); | ||
|
|
||
| let allBetas: OnyxEntry<Beta[]>; | ||
| Onyx.connect({ | ||
| key: ONYXKEYS.BETAS, | ||
| callback: (value) => (allBetas = value), | ||
| }); | ||
|
|
||
| let allTransactions: OnyxCollection<Transaction> = {}; | ||
| let reportsTransactions: Record<string, Transaction[]> = {}; | ||
| Onyx.connect({ | ||
| key: ONYXKEYS.COLLECTION.TRANSACTION, | ||
| waitForCollectionCallback: true, | ||
| callback: (value) => { | ||
|
|
@@ -1082,7 +1085,7 @@ | |
| }); | ||
|
|
||
| let allReportActions: OnyxCollection<ReportActions>; | ||
| Onyx.connect({ | ||
| key: ONYXKEYS.COLLECTION.REPORT_ACTIONS, | ||
| waitForCollectionCallback: true, | ||
| callback: (actions) => { | ||
|
|
@@ -1095,7 +1098,7 @@ | |
|
|
||
| let allReportMetadata: OnyxCollection<ReportMetadata>; | ||
| const allReportMetadataKeyValue: Record<string, ReportMetadata> = {}; | ||
| Onyx.connect({ | ||
| key: ONYXKEYS.COLLECTION.REPORT_METADATA, | ||
| waitForCollectionCallback: true, | ||
| callback: (value) => { | ||
|
|
@@ -1954,61 +1957,131 @@ | |
| } | ||
|
|
||
| /** | ||
| * Pushes optimistic transaction violations to OnyxData for the given policy and categories onyx update. | ||
| * Updates optimistic transaction violations to OnyxData for the given policy and categories onyx update. | ||
| * | ||
| * @param policyUpdate Changed policy properties, if none pass empty object | ||
| * @param policyCategoriesUpdate Changed categories properties, if none pass empty object | ||
| * @param onyxData - The OnyxData object to push updates to | ||
| * @param policyData - The current policy Data | ||
| * @param policyUpdate - Changed policy properties, if none pass empty object | ||
| * @param categoriesUpdate - Changed categories properties, if none pass empty object | ||
| * @param tagListsUpdate - Changed tag properties, if none pass empty object | ||
| */ | ||
| function pushTransactionViolationsOnyxData( | ||
| onyxData: OnyxData, | ||
| policyID: string, | ||
| policyTagLists: PolicyTagLists, | ||
| policyCategories: PolicyCategories, | ||
| allTransactionViolations: OnyxCollection<TransactionViolations>, | ||
| policyData: PolicyData, | ||
| policyUpdate: Partial<Policy> = {}, | ||
| policyCategoriesUpdate: Record<string, Partial<PolicyCategory>> = {}, | ||
| ): OnyxData { | ||
| if (isEmptyObject(policyUpdate) && isEmptyObject(policyCategoriesUpdate)) { | ||
| return onyxData; | ||
| } | ||
| const optimisticPolicyCategories = Object.keys(policyCategories).reduce<Record<string, PolicyCategory>>((acc, categoryName) => { | ||
| acc[categoryName] = {...policyCategories[categoryName], ...(policyCategoriesUpdate?.[categoryName] ?? {})}; | ||
| categoriesUpdate: Record<string, Partial<PolicyCategory>> = {}, | ||
| tagListsUpdate: Record<string, Partial<PolicyTagList>> = {}, | ||
| ) { | ||
| const nonInvoiceReportTransactionsAndViolations = policyData.reports.reduce<ReportTransactionsAndViolations[]>((acc, report) => { | ||
| // Skipping invoice reports since they should not have any category or tag violations | ||
| if (isInvoiceReport(report)) { | ||
| return acc; | ||
| } | ||
| const reportTransactionsAndViolations = policyData.transactionsAndViolations[report.reportID]; | ||
| if (!isEmptyObject(reportTransactionsAndViolations) && !isEmptyObject(reportTransactionsAndViolations.transactions)) { | ||
| acc.push(reportTransactionsAndViolations); | ||
| } | ||
| return acc; | ||
| }, {}) as PolicyCategories; | ||
| }, []); | ||
|
|
||
| const optimisticPolicy = {...allPolicies?.[`${ONYXKEYS.COLLECTION.POLICY}${policyID}`], ...policyUpdate} as Policy; | ||
| const hasDependentTags = hasDependentTagsPolicyUtils(optimisticPolicy, policyTagLists); | ||
| if (nonInvoiceReportTransactionsAndViolations.length === 0) { | ||
| return; | ||
| } | ||
|
|
||
| getAllPolicyReports(policyID).forEach((report) => { | ||
| const isReportAnInvoice = isInvoiceReport(report); | ||
| if (!report?.reportID || isReportAnInvoice) { | ||
| return; | ||
| } | ||
| const updatedTagListsNames = Object.keys(tagListsUpdate); | ||
| const updatedCategoriesNames = Object.keys(categoriesUpdate); | ||
|
|
||
| getReportTransactions(report.reportID).forEach((transaction: Transaction) => { | ||
| const transactionViolations = allTransactionViolations?.[`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${transaction.transactionID}`] ?? []; | ||
| // If there are no updates to policy, categories or tags, return early | ||
| const isPolicyUpdateEmpty = isEmptyObject(policyUpdate); | ||
| const isTagListsUpdateEmpty = updatedTagListsNames.length === 0; | ||
| const isCategoriesUpdateEmpty = updatedCategoriesNames.length === 0; | ||
| if (isPolicyUpdateEmpty && isTagListsUpdateEmpty && isCategoriesUpdateEmpty) { | ||
| return; | ||
| } | ||
|
|
||
| const optimisticTransactionViolations = ViolationsUtils.getViolationsOnyxData( | ||
| // Merge the existing policy with the optimistic updates | ||
| const optimisticPolicy = isPolicyUpdateEmpty ? policyData.policy : {...policyData.policy, ...policyUpdate}; | ||
|
|
||
| // Merge the existing categories with the optimistic updates | ||
| const optimisticCategories = isCategoriesUpdateEmpty | ||
| ? policyData.categories | ||
| : { | ||
| ...Object.fromEntries(Object.entries(policyData.categories).filter(([categoryName]) => !(categoryName in categoriesUpdate) || !!categoriesUpdate[categoryName])), | ||
| ...Object.entries(categoriesUpdate).reduce<PolicyCategories>((acc, [categoryName, categoryUpdate]) => { | ||
| if (!categoryUpdate) { | ||
| return acc; | ||
| } | ||
| acc[categoryName] = { | ||
| ...(policyData.categories?.[categoryName] ?? {}), | ||
| ...categoryUpdate, | ||
| }; | ||
| return acc; | ||
| }, {}), | ||
| }; | ||
|
|
||
| // Merge the existing tag lists with the optimistic updates | ||
| const optimisticTagLists = isTagListsUpdateEmpty | ||
| ? policyData.tags | ||
| : { | ||
| ...Object.fromEntries(Object.entries(policyData.tags ?? {}).filter(([tagListName]) => !(tagListName in tagListsUpdate) || !!tagListsUpdate[tagListName])), | ||
| ...Object.entries(tagListsUpdate).reduce<PolicyTagLists>((acc, [tagListName, tagListUpdate]) => { | ||
| if (!tagListUpdate) { | ||
| return acc; | ||
| } | ||
|
|
||
| const tagList = policyData.tags?.[tagListName]; | ||
| const tags = tagList.tags ?? {}; | ||
|
Comment on lines
+2032
to
+2033
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There's an edge case of |
||
| const tagsUpdate = tagListUpdate?.tags ?? {}; | ||
|
|
||
| acc[tagListName] = { | ||
| ...tagList, | ||
| ...tagListUpdate, | ||
| tags: { | ||
| ...((): PolicyTags => { | ||
| const optimisticTags: PolicyTags = Object.fromEntries(Object.entries(tags).filter(([tagName]) => !(tagName in tagsUpdate) || !!tagsUpdate[tagName])); | ||
| for (const [tagName, tagUpdate] of Object.entries(tagsUpdate)) { | ||
| if (!tagUpdate) { | ||
| continue; | ||
| } | ||
| optimisticTags[tagName] = { | ||
| ...(tags[tagName] ?? {}), | ||
| ...tagUpdate, | ||
| }; | ||
| } | ||
| return optimisticTags; | ||
| })(), | ||
| }, | ||
| }; | ||
| return acc; | ||
| }, {}), | ||
| }; | ||
|
|
||
| const hasDependentTags = hasDependentTagsPolicyUtils(optimisticPolicy, optimisticTagLists); | ||
|
|
||
| // Iterate through all policy reports to find transactions that need optimistic violations | ||
| for (const {transactions, violations} of nonInvoiceReportTransactionsAndViolations) { | ||
| for (const transaction of Object.values(transactions)) { | ||
| const existingViolations = violations[transaction.transactionID]; | ||
| const optimisticViolations = ViolationsUtils.getViolationsOnyxData( | ||
| transaction, | ||
| transactionViolations, | ||
| existingViolations ?? [], | ||
| optimisticPolicy, | ||
| policyTagLists, | ||
| optimisticPolicyCategories, | ||
| optimisticTagLists, | ||
| optimisticCategories, | ||
| hasDependentTags, | ||
| isReportAnInvoice, | ||
| false, | ||
| ); | ||
|
|
||
| if (optimisticTransactionViolations) { | ||
| onyxData?.optimisticData?.push(optimisticTransactionViolations); | ||
| onyxData?.failureData?.push({ | ||
| if (!isEmptyObject(optimisticViolations)) { | ||
| onyxData.optimisticData?.push(optimisticViolations); | ||
| onyxData.failureData?.push({ | ||
| onyxMethod: Onyx.METHOD.SET, | ||
| key: `${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${transaction.transactionID}`, | ||
| value: transactionViolations, | ||
| value: existingViolations ?? null, | ||
| }); | ||
| } | ||
| }); | ||
| }); | ||
| return onyxData; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /** | ||
|
|
||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This field is not properly typed in TS.
usePolicyreturnsPolicy | undefinedAnd here, overriding the
as OnyxValueWithOfflineFeedback<Policy>type causes this hook to ignore the possibility that policy may beundefined.I suggest removing all
asoperators from this hook.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Handled by #75238.