Merge Main #84829
Conversation
|
@MelvinBot can you resolve the conflicts? Please Accept Current Change for:
Please accept incoming change for:
|
|
I attempted to resolve the conflicts on this PR, but the The 4 files you specified instructions for are only a small subset of the total conflicts. Resolving all 2,266 files is beyond what I can safely do without risking unintended changes across the entire codebase. Recommended approach: This merge may need to be handled manually, or the branch strategy may need to be reconsidered (e.g., rebasing |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 88df7ab0be
ℹ️ 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".
| core.warning(`PR #${prNumber} diff is too large for the GitHub API. Skipping incremental change detection.`); | ||
| core.setOutput('CHANGED_FILES', JSON.stringify([])); | ||
| core.setOutput('HAS_CHANGES', false); |
There was a problem hiding this comment.
too_large PR diffs
When the PR diff API returns too_large, this branch forces HAS_CHANGES to false and exits. In .github/workflows/generateTranslations.yml, every downstream translation step is guarded by steps.check-en-changes.outputs.HAS_CHANGES == 'true', so a large PR that actually updates src/languages/en.ts will silently skip translation generation and the PR hint comment. This should fall back to the local diff signal (or fail explicitly) instead of suppressing changes.
Useful? React with 👍 / 👎.
| per_page: queueLimit, | ||
| }); | ||
|
|
||
| return response.data.workflow_runs.filter((workflowRun) => workflowRun.id < currentRunID && ACTIVE_STATUSES.has(workflowRun.status ?? '')); |
There was a problem hiding this comment.
This action only checks a single page of workflow runs (per_page: queueLimit) and then decides whether older runs are still active. In .github/workflows/testBuildOnPush.yml the action is called with defaults, so only the newest 20 runs are considered; if an older run is still active but has been pushed past that window by many newer completed runs, the queue step can incorrectly proceed and violate FIFO ordering.
Useful? React with 👍 / 👎.
HelpDot Documentation ReviewOverall AssessmentThis "Merge Main" PR brings in two sets of HelpDot documentation changes from previously merged PRs: (1) 30 new Xero troubleshooting articles for Expensify Classic covering authentication, connection, export, and sync errors (from PR #84822), and (2) a file move of the UK-and-EU-Expensify-Card article to a Hidden folder with noindex/sitemap flags plus a redirect entry (from PR #84469). The Xero articles represent a substantial and well-structured addition to the help site, while the UK/EU card move is a straightforward content management operation. Scores Summary
Key FindingsPositive aspects:
Issues to address:
Recommendations
Files Reviewed
Note: This review covers only the HelpDot documentation changes within this Merge Main PR. The majority of the 3,915 changed files are infrastructure, CI/CD, and application code changes that are outside the scope of this documentation review. |
|
|
||
| --- | ||
|
|
||
| ## 1. How to enable the Expensify Card on your workspace |
There was a problem hiding this comment.
Readability / heading inconsistency: This heading uses a numbered prefix ("1.") but the subsequent peer-level headings (line 79, 87) do not use numbers. Either number all step headings consistently or remove the "1." prefix here to match the rest.
| ## 1. How to enable the Expensify Card on your workspace | |
| ## How to enable the Expensify Card on your workspace |
|
|
||
| --- | ||
|
|
||
| ## How to link a bank account to settle the Expensify Card |
There was a problem hiding this comment.
Markdown formatting violation: Extra whitespace between ## and the heading text (## How to link). This should be a single space.
| ## How to link a bank account to settle the Expensify Card | |
| ## How to link a bank account to settle the Expensify Card |
|
|
||
| 1. Have a GBP or EUR bank account connected. To connect one, [Enable Global Reimbursement](https://help.expensify.com/articles/new-expensify/wallet-and-payments/Enable-Global-Reimbursement). | ||
| 2. Complete the **[Know Your Customer (KYC)](https://launch-workflow.trulioo.com/68223577c5dae52a6ec9347f)** and **[Know Your Business (KYB)](https://launch-workflow.trulioo.com/681a7cc4e65d2e48f19de9c3)** forms. | ||
| 3. Share the last 90 days of statements from your connected GBP or EUR bank account with Concierge so they can assess your eligible limit |
There was a problem hiding this comment.
Readability: This sentence is missing a period at the end, inconsistent with the other numbered items in this list.
| 3. Share the last 90 days of statements from your connected GBP or EUR bank account with Concierge so they can assess your eligible limit | |
| 3. Share the last 90 days of statements from your connected GBP or EUR bank account with Concierge so they can assess your eligible limit. |
| - Sweden | ||
| - United Kingdom | ||
|
|
||
| ## What are the pre-requisites to set up the Expensify Card in the UK and EU |
There was a problem hiding this comment.
AI Readiness / heading clarity: The heading "What are the pre-requisites" uses an informal phrasing and the hyphenated "pre-requisites" is nonstandard. Consider rephrasing to a clearer task-oriented heading.
| ## What are the pre-requisites to set up the Expensify Card in the UK and EU | |
| ## Prerequisites for setting up the Expensify Card in the UK and EU |
|
|
||
| --- | ||
|
|
||
| ## Reopen the Invoice in the Workspace and Create a New Invoice |
There was a problem hiding this comment.
Heading level violation: This ## heading and the one on line 60 are fix options that fall under the "How to Fix the XERO44 Export Error" section (line 34). They should be ### subheadings to maintain proper heading hierarchy. Using ## makes them appear as top-level peers of the "How to Fix" section, which breaks the document's logical structure.
| ## Reopen the Invoice in the Workspace and Create a New Invoice | |
| ### Reopen the Invoice in the Workspace and Create a New Invoice |
|
|
||
| --- | ||
|
|
||
| ## Manually Recreate the Invoice in Xero |
There was a problem hiding this comment.
Heading level violation: Same as line 40 -- this should be a ### subheading under the "How to Fix" section to maintain correct hierarchy.
| ## Manually Recreate the Invoice in Xero | |
| ### Manually Recreate the Invoice in Xero |
|
|
||
| --- | ||
|
|
||
| ## Recreate the Report in the Workspace |
There was a problem hiding this comment.
Heading level violation: This ## heading and the one on line 60 are fix options under the "How to Fix the XERO76 Export Error" section (line 34). They should be ### subheadings to preserve heading hierarchy, consistent with all other Xero articles in this PR.
| ## Recreate the Report in the Workspace | |
| ### Recreate the Report in the Workspace |
|
|
||
| --- | ||
|
|
||
| ## Manually Recreate the Bill in Xero |
There was a problem hiding this comment.
Heading level violation: Should be ### to nest under the "How to Fix" section.
| ## Manually Recreate the Bill in Xero | |
| ### Manually Recreate the Bill in Xero |
| https://help.expensify.com/unlisted/avoiding-duplicates,https://help.expensify.com/expensify-classic/hubs/expenses/How-to-prevent-duplicate-expenses | ||
| https://help.expensify.com/articles/new-expensify/settings/title:%20Personal-Expense-Rules,https://help.expensify.com/articles/new-expensify/settings/Personal-Expense-Rules | ||
| https://help.expensify.com/articles/expensify-classic/expenses/Export-%20Expenses-from-the-Expenses-Page,https://help.expensify.com/articles/expensify-classic/expenses/Export-Expenses-from-the-Expenses-Page | ||
| https://help.expensify.com/articles/new-expensify/expensify-card/docs/articles/new-expensify/expensify-card/UK-and-EU-Expensify-Card,https://help.expensify.com/unlisted/UK-and-EU-Expensify-Card.md |
There was a problem hiding this comment.
Readability / broken redirect URL: Two issues with this redirect entry:
-
Source URL has a duplicate path segment:
articles/new-expensify/expensify-card/docs/articles/new-expensify/expensify-card/appears to be a malformed URL that concatenates the path twice. The source should likely be justhttps://help.expensify.com/articles/new-expensify/expensify-card/UK-and-EU-Expensify-Card. -
Destination URL includes
.mdextension: The target URL ends withUK-and-EU-Expensify-Card.md, but help site URLs should not include file extensions. It should also point to the Hidden path:https://help.expensify.com/Hidden/UK-and-EU-Expensify-Card.
Suggested fix:
https://help.expensify.com/articles/new-expensify/expensify-card/UK-and-EU-Expensify-Card,https://help.expensify.com/Hidden/UK-and-EU-Expensify-Card
| internalScope: Audience is Workspace Admins using the Xero integration. Covers resolving the XERO01 error caused by connection issues between Expensify and Xero. Does not cover specific Xero permission or configuration troubleshooting steps. | ||
| --- | ||
|
|
||
| # XERO01 Error in Xero Integration |
There was a problem hiding this comment.
AI Readiness / content duplication concern: This file is a byte-identical duplicate of XERO01-Error.md in three other directories: Authentication-and-Login-errors/, Export-Errors/, and Sync-Errors/. Similarly, XERO11, XERO45, and XERO84 are duplicated across multiple directories.
Duplicate content creates maintenance risk (updates must be applied in 3-4 places) and can confuse AI retrieval systems that surface the same answer from different URLs. Consider keeping each error article in one canonical location and using redirects from the other paths, or removing the duplicates entirely.
|
|
||
| ## Does the XERO01 Error Affect All Exports? | ||
|
|
||
| It can. If the Xero connection is disrupted, multiple exports may fail until the connection is restored. |
There was a problem hiding this comment.
AI Readiness / vague reference: The FAQ answer "It can." begins with a vague pronoun. For AI retrieval and standalone readability, restate the subject so the answer is self-contained without needing to read the question.
Suggested fix:
| It can. If the Xero connection is disrupted, multiple exports may fail until the connection is restored. | |
| Yes, the XERO01 error can affect all exports. If the Xero connection is disrupted, multiple exports may fail until the connection is restored. |
This same pattern (It can., It affects..., It typically...) appears in the FAQ answers of most Xero articles in this PR. Consider restating the subject in each FAQ answer across all new files.
|
|
||
| If you see the error: | ||
|
|
||
| XERO08 Export Error: PDF of report is too large to export (>10MB). Please reach out to Concierge for additional assistance. |
There was a problem hiding this comment.
Readability / scannability: The error message text is displayed as plain paragraph text, making it hard to visually distinguish from the surrounding explanation. Consider wrapping error messages in a blockquote (>) or code block so they stand out. For example:
> XERO08 Export Error: PDF of report is too large to export (>10MB). Please reach out to Concierge for additional assistance.
This same pattern applies to all new Xero troubleshooting articles in this PR -- each displays the error message as unstyled text on line 12.
|
|
||
| --- | ||
|
|
||
| ## 1. How to enable the Expensify Card on your workspace |
There was a problem hiding this comment.
Expensify style / terminology: "workspace" should be capitalized as "Workspace" to match Expensify terminology standards. (This is also flagged for the numbered heading issue in another comment.)
| ## 1. How to enable the Expensify Card on your workspace | |
| ## How to enable the Expensify Card on your Workspace |
0773435 to
a3842c8
Compare
Explanation of Change
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 stepssectiontoggleReportand 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/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari