-
Notifications
You must be signed in to change notification settings - Fork 3.5k
[NO QA] Add an “applied a workspace rule” system message in expense details #80562
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
Conversation
|
@sobitneupane Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 43c1701933
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
src/libs/ModifiedExpenseMessage.ts
Outdated
| const entries = Object.entries(policyRulesModifiedFields) as [keyof PolicyRulesModifiedFields, string | boolean][]; | ||
|
|
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.
Handle policy tax values without stringifying objects
PolicyRulesModifiedFields includes tax?: Record<string, TaxRate> (see OriginalMessage.ts), but here the entries are cast to string | boolean and later passed to updatedFieldTo. When a workspace rule modifies taxes, the value is an object and will stringify to "[object Object]", producing a misleading system message. Consider explicitly formatting tax values or skipping object fields instead of assuming primitives.
Useful? React with 👍 / 👎.
This comment has been minimized.
This comment has been minimized.
🦜 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/de.ts b/src/languages/de.ts
index 210bdf56..bd617260 100644
--- a/src/languages/de.ts
+++ b/src/languages/de.ts
@@ -1497,23 +1497,23 @@ const translations: TranslationDeepObject<typeof en> = {
return value ? 'hat die Ausgabe als „erstattungsfähig“ markiert' : 'hat die Ausgabe als „nicht erstattungsfähig“ markiert';
}
if (key === 'billable') {
- return value ? 'hat die Ausgabe als „verrechenbar“ markiert' : 'hat die Ausgabe als „nicht abrechenbar“ markiert';
+ return value ? 'hat die Ausgabe als „verrechenbar“ markiert' : 'hat die Ausgabe als „nicht verrechenbar“ markiert';
}
if (key === 'tax') {
const taxEntry = value as PolicyRulesModifiedFields['tax'];
const taxRateName = taxEntry?.field_id_TAX.name ?? '';
if (isFirst) {
- return `Steuersatz auf „${taxRateName}“ festlegen`;
+ return `den Steuersatz auf „${taxRateName}“ festlegen`;
}
- return `Steuersatz zu „${taxRateName}“`;
+ return `Steuersatz auf „${taxRateName}“`;
}
const updatedValue = value as string | boolean;
if (isFirst) {
- return `Setzen Sie ${translations.common[key].toLowerCase()} auf „${updatedValue}“`;
+ return `setze ${translations.common[key].toLowerCase()} auf „${updatedValue}“`;
}
- return `${translations.common[key].toLowerCase()} zu "${updatedValue}"`;
+ return `${translations.common[key].toLowerCase()} zu „${updatedValue}“`;
});
- return `${formatList(fragments)} über <a href="${policyRulesRoute}">Workspace-Regeln</a>`;
+ return `${formatList(fragments)} über <a href="${policyRulesRoute}">Arbeitsbereichsregeln</a>`;
},
},
transactionMerge: {
diff --git a/src/languages/fr.ts b/src/languages/fr.ts
index ce2a0170..9b4cc909 100644
--- a/src/languages/fr.ts
+++ b/src/languages/fr.ts
@@ -1497,7 +1497,7 @@ const translations: TranslationDeepObject<typeof en> = {
const fragments = entries.map(([key, value], i) => {
const isFirst = i === 0;
if (key === 'reimbursable') {
- return value ? 'a marqué la dépense comme « remboursable »' : 'a marqué la dépense comme « non remboursable »';
+ return value ? 'a marqué la dépense comme « remboursable »' : 'a marqué la dépense comme « non remboursable »';
}
if (key === 'billable') {
return value ? 'a marqué la dépense comme « facturable »' : 'a marqué la dépense comme « non refacturable »';
diff --git a/src/languages/it.ts b/src/languages/it.ts
index 2b128703..a2ba8068 100644
--- a/src/languages/it.ts
+++ b/src/languages/it.ts
@@ -1491,7 +1491,7 @@ const translations: TranslationDeepObject<typeof en> = {
const fragments = entries.map(([key, value], i) => {
const isFirst = i === 0;
if (key === 'reimbursable') {
- return value ? 'ha contrassegnato la spesa come "rimborsabile"' : 'ha contrassegnato la spesa come "non rimborsabile"';
+ return value ? 'ha contrassegnato la spesa come "rimborsabile"' : 'ha contrassegnato la spesa come “non rimborsabile”';
}
if (key === 'billable') {
return value ? 'ha contrassegnato la spesa come "fatturabile"' : 'ha contrassegnato la spesa come "non addebitabile"';
@@ -1502,7 +1502,7 @@ const translations: TranslationDeepObject<typeof en> = {
if (isFirst) {
return `imposta l'aliquota fiscale su "${taxRateName}"`;
}
- return `aliquota fiscale in "${taxRateName}"`;
+ return `aliquota fiscale su "${taxRateName}"`;
}
const updatedValue = value as string | boolean;
if (isFirst) {
@@ -1510,7 +1510,7 @@ const translations: TranslationDeepObject<typeof en> = {
}
return `${translations.common[key].toLowerCase()} a "${updatedValue}"`;
});
- return `${formatList(fragments)} tramite <a href="${policyRulesRoute}">regole dello spazio di lavoro</a>`;
+ return `${formatList(fragments)} tramite <a href="${policyRulesRoute}">regole della workspace</a>`;
},
},
transactionMerge: {
diff --git a/src/languages/ja.ts b/src/languages/ja.ts
index ce78c3ef..8d72afca 100644
--- a/src/languages/ja.ts
+++ b/src/languages/ja.ts
@@ -1489,10 +1489,10 @@ const translations: TranslationDeepObject<typeof en> = {
const fragments = entries.map(([key, value], i) => {
const isFirst = i === 0;
if (key === 'reimbursable') {
- return value ? '経費を「立替精算対象」にマークしました' : '経費を「非精算」としてマークしました';
+ return value ? '経費を「精算対象」としてマークしました' : 'この経費を「非精算」に設定しました';
}
if (key === 'billable') {
- return value ? '経費を「請求可能」とマークしました' : '経費を「請求対象外」としてマークしました';
+ return value ? '経費を「請求可能」としてマークしました' : '経費を「請求対象外」にしました';
}
if (key === 'tax') {
const taxEntry = value as PolicyRulesModifiedFields['tax'];
@@ -1506,9 +1506,9 @@ const translations: TranslationDeepObject<typeof en> = {
if (isFirst) {
return `${translations.common[key].toLowerCase()} を「${updatedValue}」に設定`;
}
- return `${translations.common[key].toLowerCase()} を「${updatedValue}」に`;
+ return `${translations.common[key].toLowerCase()} から「${updatedValue}」`;
});
- return `${formatList(fragments)}(<a href="${policyRulesRoute}">ワークスペースルール</a>経由)`;
+ return `${formatList(fragments)}(<a href="${policyRulesRoute}">ワークスペースルール</a> を介して)`;
},
},
transactionMerge: {
diff --git a/src/languages/nl.ts b/src/languages/nl.ts
index 3623ef42..c7159d60 100644
--- a/src/languages/nl.ts
+++ b/src/languages/nl.ts
@@ -1490,10 +1490,10 @@ const translations: TranslationDeepObject<typeof en> = {
const fragments = entries.map(([key, value], i) => {
const isFirst = i === 0;
if (key === 'reimbursable') {
- return value ? 'heeft de uitgave als „vergoedbaar” gemarkeerd' : 'markeerde de uitgave als ‘niet-terugbetaalbaar’';
+ return value ? 'heeft de uitgave gemarkeerd als “vergoedbaar”' : 'heeft de uitgave gemarkeerd als ‘niet-vergoedbaar’';
}
if (key === 'billable') {
- return value ? 'heeft de uitgave gemarkeerd als ‘factureerbaar’' : 'heeft de uitgave als ‘niet-declarabel’ gemarkeerd';
+ return value ? 'heeft de uitgave als ‘factureerbaar’ gemarkeerd' : 'heeft de uitgave als ‘niet-factureerbaar’ gemarkeerd';
}
if (key === 'tax') {
const taxEntry = value as PolicyRulesModifiedFields['tax'];
diff --git a/src/languages/pl.ts b/src/languages/pl.ts
index 2158f487..c5cb73e1 100644
--- a/src/languages/pl.ts
+++ b/src/languages/pl.ts
@@ -1489,10 +1489,10 @@ const translations: TranslationDeepObject<typeof en> = {
const fragments = entries.map(([key, value], i) => {
const isFirst = i === 0;
if (key === 'reimbursable') {
- return value ? 'oznaczył wydatek jako „podlegający zwrotowi”' : 'oznaczył wydatek jako „niepodlegający zwrotowi”';
+ return value ? 'oznaczył wydatek jako „podlegający zwrotowi”' : 'oznaczył(a) wydatek jako „niepodlegający zwrotowi”';
}
if (key === 'billable') {
- return value ? 'oznaczył wydatek jako „refakturowalny”' : 'oznaczył wydatek jako „niepodlegający refakturowaniu”';
+ return value ? 'oznaczył wydatek jako „fakturowalny”' : 'oznaczył(-a) wydatek jako „niepodlegający refakturowaniu”';
}
if (key === 'tax') {
const taxEntry = value as PolicyRulesModifiedFields['tax'];
@@ -1500,15 +1500,15 @@ const translations: TranslationDeepObject<typeof en> = {
if (isFirst) {
return `ustaw stawkę podatku na „${taxRateName}”`;
}
- return `stawka podatku na „${taxRateName}”`;
+ return `stawka podatkowa „${taxRateName}”`;
}
const updatedValue = value as string | boolean;
if (isFirst) {
return `ustaw ${translations.common[key].toLowerCase()} na „${updatedValue}”`;
}
- return `${translations.common[key].toLowerCase()} na „${updatedValue}”`;
+ return `${translations.common[key].toLowerCase()} do „${updatedValue}”`;
});
- return `${formatList(fragments)} przez <a href="${policyRulesRoute}">zasady przestrzeni roboczej</a>`;
+ return `${formatList(fragments)} poprzez <a href="${policyRulesRoute}">zasady przestrzeni roboczej</a>`;
},
},
transactionMerge: {
diff --git a/src/languages/pt-BR.ts b/src/languages/pt-BR.ts
index 5097a2b6..b985d1a9 100644
--- a/src/languages/pt-BR.ts
+++ b/src/languages/pt-BR.ts
@@ -1487,18 +1487,18 @@ const translations: TranslationDeepObject<typeof en> = {
const fragments = entries.map(([key, value], i) => {
const isFirst = i === 0;
if (key === 'reimbursable') {
- return value ? 'marcou a despesa como "reembolsável"' : 'marcou a despesa como “não reembolsável”';
+ return value ? 'marcou a despesa como “reembolsável”' : 'marcou a despesa como "não reembolsável"';
}
if (key === 'billable') {
- return value ? 'marcou a despesa como “faturável”' : 'marcou a despesa como "não faturável"';
+ return value ? 'marcou a despesa como “faturável”' : 'marcou a despesa como “não faturável”';
}
if (key === 'tax') {
const taxEntry = value as PolicyRulesModifiedFields['tax'];
const taxRateName = taxEntry?.field_id_TAX.name ?? '';
if (isFirst) {
- return `definir a taxa de imposto como "${taxRateName}"`;
+ return `definir a taxa de imposto para "${taxRateName}"`;
}
- return `alíquota de imposto para "${taxRateName}"`;
+ return `taxa de imposto para "${taxRateName}"`;
}
const updatedValue = value as string | boolean;
if (isFirst) {
diff --git a/src/languages/zh-hans.ts b/src/languages/zh-hans.ts
index d18cad80..374365d9 100644
--- a/src/languages/zh-hans.ts
+++ b/src/languages/zh-hans.ts
@@ -1466,10 +1466,10 @@ const translations: TranslationDeepObject<typeof en> = {
const fragments = entries.map(([key, value], i) => {
const isFirst = i === 0;
if (key === 'reimbursable') {
- return value ? '将该报销单标记为“可报销”' : '将该报销单标记为“不可报销”';
+ return value ? '将该报销单标记为“可报销”' : '已将该报销单标记为“不可报销”';
}
if (key === 'billable') {
- return value ? '将该报销标记为“可向客户收费”' : '已将该报销标记为“不可计费”';
+ return value ? '将该报销标记为“可计费”' : '将该报销标记为“不可向客户计费”';
}
if (key === 'tax') {
const taxEntry = value as PolicyRulesModifiedFields['tax'];
@@ -1481,11 +1481,11 @@ const translations: TranslationDeepObject<typeof en> = {
}
const updatedValue = value as string | boolean;
if (isFirst) {
- return `将 ${translations.common[key].toLowerCase()} 设置为 “${updatedValue}”`;
+ return `将 ${translations.common[key].toLowerCase()} 设置为“${updatedValue}”`;
}
return `${translations.common[key].toLowerCase()} 为 “${updatedValue}”`;
});
- return `${formatList(fragments)} 通过<a href="${policyRulesRoute}">工作区规则</a>`;
+ return `${formatList(fragments)},通过<a href="${policyRulesRoute}">工作区规则</a>`;
},
},
transactionMerge: {
Note You can apply these changes to your branch by copying the patch to your clipboard, then running |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / Safari |
cead22
left a comment
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.
Small change requested on the Spanish file
|
Please merge main. Lint fixed in #80667 |
|
Codex Review: Didn't find any major issues. You're on a roll. ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
|
There's no visual test on expense details as backend is not ready yet, right? |
|
@situchan no there are no visible tests yet |
ok unit tests should cover this enough |
|
@carlosmiceli Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
luacmartins
left a comment
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.
LGTM
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚀 Deployed to staging by https://github.com/luacmartins in version: 9.3.11-0 🚀
|
Explanation of Change
Generate a system message for report actions when we have modified an expense via policy rules
Fixed Issues
$ #80522
Tests
N/A
Offline tests
N/A
QA Steps
N/A
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.Screenshots/Videosundefined