[WIP] fix: follow up Reject bugs for Bulk Reject#73932
[WIP] fix: follow up Reject bugs for Bulk Reject#73932
Conversation
🦜 Polyglot Parrot! 🦜Squawk! Looks like you added some shiny new English strings. Allow me to parrot them back to you in other tongues: View the translation diffdiff --git a/src/languages/fr.ts b/src/languages/fr.ts
index bb3ba598..46e8d2e4 100644
--- a/src/languages/fr.ts
+++ b/src/languages/fr.ts
@@ -6280,7 +6280,7 @@ ${amount} pour ${merchant} - ${date}`,
delete: 'Supprimer',
hold: 'Attente',
unhold: 'Supprimer la suspension',
- reject: 'Refuser',
+ reject: 'Rejeter',
noOptionsAvailable: 'Aucune option disponible pour le groupe de dépenses sélectionné.',
},
filtersHeader: 'Filtres',
diff --git a/src/languages/pl.ts b/src/languages/pl.ts
index 81201975..2f17cfe9 100644
--- a/src/languages/pl.ts
+++ b/src/languages/pl.ts
@@ -4432,7 +4432,6 @@ ${amount} dla ${merchant} - ${date}`,
displayedAsTagDescription: 'Dział będzie można wybrać dla każdego indywidualnego wydatku w raporcie pracownika.',
displayedAsReportFieldDescription: 'Wybór działu będzie dotyczył wszystkich wydatków w raporcie pracownika.',
toggleImportTitle: ({mappingTitle}: ToggleImportTitleParams) => `Wybierz, jak obsługiwać Sage Intacct <strong>${mappingTitle}</strong> w Expensify.`,
-
expenseTypes: 'Typy wydatków',
expenseTypesDescription: 'Twoje typy wydatków Sage Intacct zostaną zaimportowane do Expensify jako kategorie.',
accountTypesDescription: 'Twój plan kont Sage Intacct zostanie zaimportowany do Expensify jako kategorie.',
diff --git a/src/languages/pt-BR.ts b/src/languages/pt-BR.ts
index 208e8439..80b324b1 100644
--- a/src/languages/pt-BR.ts
+++ b/src/languages/pt-BR.ts
@@ -4431,7 +4431,6 @@ ${amount} para ${merchant} - ${date}`,
displayedAsTagDescription: 'O departamento será selecionável para cada despesa individual no relatório de um funcionário.',
displayedAsReportFieldDescription: 'A seleção de departamento será aplicada a todas as despesas no relatório de um funcionário.',
toggleImportTitle: ({mappingTitle}: ToggleImportTitleParams) => `Escolha como lidar com o Sage Intacct <strong>${mappingTitle}</strong> in Expensify.`,
-
expenseTypes: 'Tipos de despesas',
expenseTypesDescription: 'Seus tipos de despesas do Sage Intacct serão importados para o Expensify como categorias.',
accountTypesDescription: 'Seu plano de contas do Sage Intacct será importado para o Expensify como categorias.',
@@ -7369,7 +7368,6 @@ ${amount} para ${merchant} - ${date}`,
exportInProgress: 'Exportação em andamento',
conciergeWillSend: 'Concierge enviará o arquivo em breve.',
},
-
avatarPage: {
title: 'Editar foto de perfil',
upload: 'Carregar',
diff --git a/src/languages/zh-hans.ts b/src/languages/zh-hans.ts
index a428c925..9ad4aa39 100644
--- a/src/languages/zh-hans.ts
+++ b/src/languages/zh-hans.ts
@@ -6130,15 +6130,7 @@ ${merchant}的${amount} - ${date}`,
searchName: '搜索名称',
savedSearchesMenuItemTitle: '已保存',
groupedExpenses: '分组费用',
- bulkActions: {
- approve: '批准',
- pay: '支付',
- delete: '删除',
- hold: '保持',
- unhold: '移除保留',
- reject: '拒绝',
- noOptionsAvailable: '所选费用组没有可用选项。',
- },
+ bulkActions: {approve: '批准', pay: '支付', delete: '删除', hold: '保持', unhold: '移除保留', reject: '拒绝', noOptionsAvailable: '所选费用组没有可用选项。'},
filtersHeader: '筛选器',
filters: {
date: {
Note You can apply these changes to your branch by copying the patch to your clipboard, then running |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| import type {SearchResults, Transaction} from '@src/types/onyx'; | ||
| import type {FileObject} from '@src/types/utils/Attachment'; | ||
| import SearchPageNarrow from './SearchPageNarrow'; | ||
| import { getTransactionViolationsOfTransaction } from '@libs/TransactionUtils'; |
There was a problem hiding this comment.
Code Style Issue
The import statement has improper spacing. According to the codebase conventions, there should be no spaces after the opening brace or before the closing brace.
Suggested fix:
import {getTransactionViolationsOfTransaction} from '@libs/TransactionUtils';
| const errors: FormInputErrors<typeof ONYXKEYS.FORMS.MONEY_REQUEST_REJECT_FORM> = getFieldRequiredErrors(values, [INPUT_IDS.COMMENT]); | ||
| return errors; | ||
| }, | ||
| [translate], |
There was a problem hiding this comment.
Unnecessary Dependency
The translate variable is included in the dependency array but is not used within the validate callback function. This creates an unnecessary dependency that could cause the function to be recreated when translate changes.
Suggested fix:
const validate = useCallback(
(values: FormOnyxValues<typeof ONYXKEYS.FORMS.MONEY_REQUEST_REJECT_FORM>) => {
const errors: FormInputErrors<typeof ONYXKEYS.FORMS.MONEY_REQUEST_REJECT_FORM> = getFieldRequiredErrors(values, [INPUT_IDS.COMMENT]);
return errors;
},
[], // Remove translate from dependencies since it's not used
);
| return {areHydrated: true, missingReportIDs: [], missingPolicyIDs: []}; | ||
| } | ||
| return checkBulkRejectHydration(selectedTransactions, policies); | ||
| }, [selectedTransactions, policies]); |
There was a problem hiding this comment.
❌ PERF-6 (docs)
Passing entire objects (selectedTransactions, policies) as dependencies causes this hook to re-execute whenever any property in these objects changes, even unrelated ones. This creates unnecessary computation overhead.
Suggested fix:
Extract the specific values needed and use them as dependencies:
const selectedTransactionsLength = Object.keys(selectedTransactions).length;
const bulkRejectHydrationStatus = useMemo(() => {
if (selectedTransactionsLength === 0) {
return {areHydrated: true, missingReportIDs: [], missingPolicyIDs: []};
}
return checkBulkRejectHydration(selectedTransactions, policies);
}, [selectedTransactions, policies, selectedTransactionsLength]);Note: In this case, since checkBulkRejectHydration needs the full objects to iterate over them, you may need to keep these dependencies. However, consider if the hydration check could be performed more efficiently or if you can use more specific dependencies based on what triggers the need for re-checking.
Codecov Report❌ Looks like you've decreased code coverage for some files. Please write tests to increase, or at least maintain, the existing level of code coverage. See our documentation here for how to interpret this table.
|
|
@truph01 will take over to finish the remaining bug fixes, and @mananjadhav will then review it. |
|
@truph01 when do you think you'll be able to get moving on this one? |
|
@garrettmknight I already have an idea for how to fix the bugs, but I'm currently stuck on checking out the branch It seems I need the permissions to checkout. |
|
I've created a fork and now @truph01 will be able to work on the PR. |
|
@truph01 can you give an update on progress? |
|
@trjExpensify We're still waiting on a few backend PRs: https://expensify.slack.com/archives/C094PPQTDM4/p1762524305795849?thread_ts=1762366091.897819&cid=C094PPQTDM4 |
5 participants
Explanation of Change
Undoes the revert of #72855.
Also fixes bugs reported by QA:
openReport()to fetch missing Onyx details needed forcanRejectReportAction()(open to another solution)Bugs remaining:
Fixed Issues
$
PROPOSAL:
Tests
Offline tests
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps./** comment above it */thisproperly so there are no scoping issues (i.e. foronClick={this.submit}the methodthis.submitshould be bound tothisin the constructor)thisare necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);ifthis.submitis never passed to a component event handler likeonClick)Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
MacOS: Desktop