Repository navigation
[No QA] Add NetSuite SuiteTax setup guidance - #104401
Conversation
This comment has been minimized.
This comment has been minimized.
6531b44 to
681be6b
Compare
|
Scope update after reviewing the NetSuite integration code: this draft now changes only three files. I removed the standalone SuiteTax article and all customer-specific custom-record, approval-routing, form-field, and vendor-form guidance. The existing preview-bot comment reflects the superseded push; the current diff contains only the two SuiteTax role requirements, the Tax Code/Tax Group clarification, and the NS0056 foreign-currency note. |
681be6b to
67d8cef
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
| Expensify supports Tax Groups, not individual Tax Codes, except when you use SuiteTax. Under **Setup > Accounting > Set Up Taxes**, set **Tax Code Lists Include** to **Tax Groups And Tax Codes** or **Tax Groups Only**. In the UK and Ireland, make sure each Tax Group contains only one Tax Code. | ||
| SuiteTax accounts import individual Tax Codes. Legacy tax accounts import Tax Groups. | ||
|
|
||
| ### SuiteTax accounts |
There was a problem hiding this comment.
Heading-level violation: Only # and ## headings are allowed (HELP_AUTHORING_GUIDELINES.md Section 4 and Pre-Publish Checklist — "no ### or deeper"). ### SuiteTax accounts is also a noun-only heading, not task-based. Convert to a ## task-based heading such as ## How to configure taxes for SuiteTax accounts in NetSuite, or restructure as bold sub-labels within the existing ## How to configure taxes in NetSuite section.
There was a problem hiding this comment.
Fixed in e42f698. I changed ### SuiteTax accounts to a bold sub-label inside the existing ## How to configure taxes in NetSuite section. I kept one ## section so it still matches step 10 (OAuth) and step 11 (token-based) in the setup lists.
|
|
||
| Do not complete the Tax Group steps below. | ||
|
|
||
| ### Legacy tax accounts |
There was a problem hiding this comment.
Heading-level violation: ### headings are not allowed (only # and ## per HELP_AUTHORING_GUIDELINES.md Section 4). ### Legacy tax accounts is also noun-only rather than task-based. Convert to a ## task-based heading (e.g. ## How to configure taxes for legacy tax accounts in NetSuite) or use bold sub-labels under the existing ## section.
There was a problem hiding this comment.
Fixed in e42f698. ### Legacy tax accounts is now a bold sub-label under the existing ## section.
| 1. Go to **Setup > Users/Roles > Manage Roles**. | ||
| 2. Edit the role used for the Expensify connection. | ||
| 3. Under **Permissions > Lists**, set **Tax Details Tab** to **Full**. | ||
| 4. Click **Save**. |
There was a problem hiding this comment.
UI interaction verb violation: Use select, not "click" (HELPSITE_NAMING_CONVENTIONS.md — Actions). Change to Select **Save**.
| 5. Go to **Setup > SuiteTax Engine > SuiteTax Engine Roles**. | ||
| 6. Add the role used for the Expensify connection. | ||
| 7. Set the access level to **Full**. | ||
| 8. Click **Save**. |
There was a problem hiding this comment.
UI interaction verb violation: Use select, not "click" (HELPSITE_NAMING_CONVENTIONS.md — Actions). Change to Select **Save**.
| 7. Set the access level to **Full**. | ||
| 8. Click **Save**. | ||
|
|
||
| Do not complete the Tax Group steps below. |
There was a problem hiding this comment.
Cross-reference clarity: "the Tax Group steps below" is a vague directional reference. Governance discourages vague navigation phrasing (HELPSITE_NAMING_CONVENTIONS.md — Prohibited Language). Point to the specific section by name, e.g. "Do not complete the Legacy tax accounts steps."
There was a problem hiding this comment.
Fixed in e42f698. It now reads: "Do not complete the Legacy tax accounts steps."
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 67d8cef38e
ℹ️ 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".
| 5. Go to **Setup > SuiteTax Engine > SuiteTax Engine Roles**. | ||
| 6. Add the role used for the Expensify connection. | ||
| 7. Set the access level to **Full**. |
There was a problem hiding this comment.
Use the actual SuiteTax role-permission workflow
When a SuiteTax administrator follows these steps, NetSuite does not provide the described SuiteTax Engine Roles page or one aggregate access-level field, so the connection role can remain unconfigured. Oracle's documented workflow is Setup > SuiteTax Engine > Manage SuiteTax Engine Roles, followed by Assign SuiteTax Engine Permissions to Role, where a level must be selected for each required permission on the SuiteTax Engine Permissions subtab; update these steps to identify the permissions that Expensify requires at Full access. See Oracle's SuiteTax Engine Permissions documentation.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in e42f698. I confirmed the workflow against Oracle's SuiteTax Engine Permissions page. The steps now say:
- Go to Setup > SuiteTax Engine > Manage SuiteTax Engine Roles.
- Select Assign SuiteTax Engine Permissions to Role.
- In the Role field, choose the connection role.
- On the SuiteTax Engine Permissions subtab, set each permission to Full.
I kept "each permission to Full" to match the original guidance. I couldn't confirm the minimum set Expensify needs from the App repo. If only some permissions are required, for example Tax Schedules, please name them and I'll narrow the step.
HelpDot Documentation ReviewOverall AssessmentThis PR adds NetSuite SuiteTax setup guidance across three existing NetSuite articles. It separates SuiteTax tax setup (individual Tax Codes) from legacy tax setup (Tax Groups), documents two previously missing role permissions required for the Expensify connection, and clarifies a permission requirement in the NS0056 export-error article. The proposed changes are accurate, well-scoped, and improve the completeness of the NetSuite documentation. Scores below reflect only the added/modified lines. Scores Summary
Key Findings
Recommendations
Files Reviewed
Note: Detailed line-by-line feedback has been provided as inline comments. |
|
@MelvinBot please address the comments here. |
Co-authored-by: Ted Harris <[email protected]>
|
I addressed all 6 review comments in e42f698 and replied in each thread. All edits are in
|
|
A preview of your ExpensifyHelp changes have been deployed to https://8bff15dd.helpdot.pages.dev ⚡️ Updated articles: |
|
🚀 Deployed to staging by https://github.com/stephanieelliott in version: 9.5.15-1 🚀
|
|
🚀 Deployed to production by https://github.com/grgia in version: 9.5.15-3 🚀
Bundle Size Analysis (Sentry): |
Explanation of Change
A customer SuiteTax setup review showed that the existing NetSuite help did not document two permissions required by the role used for the Expensify connection. This PR updates the existing NetSuite articles rather than adding a separate SuiteTax guide.
The changes:
Connect-To-NetSuite.mdto:Configure-Netsuite.mdso the Tax import setting distinguishes SuiteTax Tax Codes from legacy Tax Groups.NS0056export-error article to clarify that Tax Details Tab remains required when Export Foreign Currency Amount is disabled.The customer review also surfaced custom records, approval routing, transaction-form fields, and vendor-bill form defaults. Those findings are not included because they are customer-specific or general NetSuite troubleshooting, not baseline SuiteTax setup requirements.
Fixed Issues
N/A — internal documentation update based on a customer setup review and a review of the NetSuite integration code.
PROPOSAL: N/A
Tests
git diff --checkand confirm it passes.npm run lint-changedand confirm it reports no lintable files changed.NS0056clarification appears in the existing export-error article.git diff --checkpassed.npm run lint-changedpassed with no lintable files changed.Local
npm run spell-changedcould not run becausenode_modules/.bin/cspellis not installed. A systemaspellreview found only expected product names, UI labels, and identifiers, and GitHub's spellcheck passed.Offline tests
N/A — documentation-only change; no application or offline behavior changes.
QA Steps
No QA
PR Author Checklist
Unchecked application/platform items are not applicable to this documentation-only change.
### Fixed Issuessection above (N/A — no tracking issue)TestssectionOffline stepssectionQA stepssectionAvatar, I verified the components usingAvatarare working as expected)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.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
N/A — documentation-only change.
Android: mWeb Chrome
N/A — documentation-only change.
iOS: Native
N/A — documentation-only change.
iOS: mWeb Safari
N/A — documentation-only change.
MacOS: Chrome / Safari
N/A — documentation-only change.