-
Notifications
You must be signed in to change notification settings - Fork 3.2k
Adjust copy for various report actions #60171
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
@ahmedGaber93 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] |
@ahmedGaber93 I'm unable to test auto submit, payment, and export to accounting integration for company card expenses at the moment. But I think all the copy displays correctly |
I've started a new Slack discussion here to confirm the translation |
Reviewer Checklist
Screenshots/VideosAndroid: NativeAndroid: mWeb ChromeiOS: NativeiOS: mWeb SafariMacOS: Desktop |
@@ -1057,6 +1057,8 @@ const CONST = { | |||
COMPANY_CARDS_CONNECT_CREDIT_CARDS_HELP_URL: 'https://help.expensify.com/new-expensify/hubs/connect-credit-cards/', | |||
CUSTOM_REPORT_NAME_HELP_URL: 'https://help.expensify.com/articles/expensify-classic/spending-insights/Custom-Templates', | |||
CONFIGURE_REIMBURSEMENT_SETTINGS_HELP_URL: 'https://help.expensify.com/articles/expensify-classic/workspaces/Configure-Reimbursement-Settings', | |||
CONFIGURE_EXPENSE_REPORT_RULES_HELP_URL: 'https://help.expensify.com/articles/new-expensify/workspaces/Set-up-rules#configure-expense-report-rules', | |||
SELECT_WORKFLOWS_HELP_URL: 'https://help.expensify.com/articles/new-expensify/workspaces/Set-up-workflows#select-workflows', |
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.
I think DELAYED_SUBMISSION_HELP_URL
will be unused after declaring this, let's remove it if it is unused.
src/languages/en.ts
Outdated
@@ -1003,7 +1003,7 @@ const translations = { | |||
submitAmount: ({amount}: RequestAmountParams) => `Submit ${amount}`, | |||
submittedAmount: ({formattedAmount, comment}: RequestedAmountMessageParams) => `submitted ${formattedAmount}${comment ? ` for ${comment}` : ''}`, | |||
automaticallySubmittedAmount: ({formattedAmount}: RequestedAmountMessageParams) => | |||
`automatically submitted ${formattedAmount} via <a href="${CONST.DELAYED_SUBMISSION_HELP_URL}">delayed submission</a>`, | |||
`submitted ${formattedAmount} via <a href="${CONST.SELECT_WORKFLOWS_HELP_URL}">delayed submissions</a>`, |
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.
`submitted ${formattedAmount} via <a href="${CONST.SELECT_WORKFLOWS_HELP_URL}">delayed submissions</a>`, | |
`submitted ${formattedAmount} via <a href="${CONST.SELECT_WORKFLOWS_HELP_URL}">delay submissions</a>`, |
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.
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.
Could you also update the Spanish translation and the translation post on slack?
src/libs/ReportActionsUtils.ts
Outdated
if (!isPending) { | ||
result.push({ | ||
text: translateLocal('report.actions.type.exportedToIntegration.automaticActionThree'), | ||
url: '', | ||
}); | ||
} |
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.
I think the condition reimbursableUrls.length || nonReimbursableUrls.length
may be accurate than !isPending
, this will confirm that 'and successfully created a record for'
will always follow by a link.
PR updated |
@nyomanjyotisa could you ask again for translation on slack and link the old post for details? |
Sure, I've bumped the discussion here |
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! We just still need to confirm Spanish translation.
Maybe @joekaufmanexpensify @Julesssss can help 🙏 |
src/languages/en.ts
Outdated
reimburseableLink: 'View out-of-pocket expenses.', | ||
nonReimbursableLink: 'View company card expenses.', | ||
automaticActionThree: 'and successfully created a record for', | ||
and: 'and', |
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.
Lets use 'and' from a different string. We can use or make a common string for this.
I have requests copy review internally here. |
PR updated |
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.
Thanks for holding. I'll share the final translations below
|
The amount ($XX.XX) on approved and submitted translations has been removed in this PR |
@nyomanjyotisa @Julesssss @ahmedGaber93 We had a discussion on the removing of the Could you guys just fold removing the |
Yes, sure. |
Fixed! @Julesssss |
✋ 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/Julesssss in version: 9.1.48-0 🚀
|
🚀 Deployed to staging by https://github.com/Julesssss in version: 9.1.48-0 🚀
|
@nyomanjyotisa Testers are not seeing amounts when submitting, approving and paying expenses. Is this expected? Bug6839035_1747904315029.bandicam_2025-05-22_11-39-55-769.mp4 |
🚀 Deployed to staging by https://github.com/Julesssss in version: 9.1.49-5 🚀
|
Thanks for raising, no that's not expected. |
The issue is not reproducible on prod. There was a similar occurrence over here and I wanted to make sure before creating a new ticket. bandicam.2025-05-22.12-09-51-909.mp4 |
@Julesssss this is expected, we remove amount from from some system actions. I am afk, give me 10 minutes and I will share the context. |
Thnks. I'm just looking at the desired comments from @joekaufmanexpensify here, and I thought we still wanted the amount to show for submitting and approving, but not export. |
Sorry, I was away from the keyboard. Here is the comment #60171 (comment)
|
But it looks we missed updating the Test steps after the context changed. @nyomanjyotisa Please update Test steps to include this change. |
Ah I see. Thank you for sharing, I wasn't able to find that requirement 👍 |
[CP Staging] Revert #60171 "Adjust copy for various report actions"
Hi,it seems like the action which submitted without amount is expected.But your upwork job is still here |
🚀 Deployed to production by https://github.com/arosiclair in version: 9.1.50-0 🚀
|
🚀 Deployed to staging by https://github.com/Julesssss in version: 9.1.51-0 🚀
|
Explanation of Change
Fixed Issues
$ #60035
PROPOSAL: #60035 (comment)
Tests
Same as QA Steps
Offline tests
Same as QA Steps
QA Steps
Test 1 (Submitting Expense):
Test 2 (Approving Expense):
Test 3 (Paying Expense):
Test 4 (Exporting Expense):
Note: for both manual and automatic exports, if the report has both out of pocket and company card expenses, the system message will be:
"exported to %Accounting integration% and successfully created a record for [out of pocket expenses](link to out of pocket expenses in accounting integration) and [company card expenses](link to company card expenses in accounting integration)."
Test 5 (Manual export to export file):
Test 6 (Canceling Payment):
PR Author Checklist
### Fixed Issues
section aboveTests
sectionOffline steps
sectionQA steps
sectiontoggleReport
and notonIconClick
)src/languages/*
files and using the translation methodSTYLE.md
) were followedAvatar
, I verified the components usingAvatar
are working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG)
)Avatar
is modified, I verified thatAvatar
is working as expected in all cases)Design
label and/or tagged@Expensify/design
so the design team can review the changes.ScrollView
component to make it scrollable when more elements are added to the page.main
branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTest
steps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
MacOS: Desktop