[ACTION] Expensify | Create Report #18298
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
WalkthroughAdds a new Expensify "Create Report" action and supporting utilities; extends the Expensify app with propDefinitions, getPolicyList and createReport methods, refactors request handling, adds JSON/array parsing helpers, and updates several action versions and package version. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Action as Create Report Action
participant Utils as common/utils
participant App as Expensify App
participant API as Expensify Integration Server
User->>Action: provide employeeEmail, policyId, reportTitle, reportFields?, expenses[]
Action->>Utils: parseJson(reportFields) and parseArray(expenses)
Utils-->>Action: parsed fields & expenses
Action->>App: createReport({ employeeEmail, policyID, report:{ title, fields? }, expenses })
App->>API: _makeRequest(job submit with inputSettings.type="report", data)
API-->>App: response (reportID, reportName, ...)
App-->>Action: response
Action-->>User: summary + response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (6)
🚧 Files skipped from review as they are similar to previous changes (3)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
🔇 Additional comments (4)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/expensify/actions/create-expense/create-expense.ts (1)
46-67: Await the API call; current code returns a Promise and accesses its properties.This will break the summary and condition check.
- const response = this.expensify.createExpense({ + const response = await this.expensify.createExpense({ $, data: { employeeEmail: this.employeeEmail, transactionList: [ { created: this.created, comment: this.comment, currency: this.currency, amount: this.amount, merchant: this.merchant, }, ], }, });
🧹 Nitpick comments (6)
components/expensify/app/expensify.app.ts (3)
8-30: Confirm propDefinition wiring for policyId options.The options function expects a userEmail. Ensure actions pass it via propDefinition mapping, e.g.:
- policyId: { + policyId: { // ... async options({ userEmail }) { const { policyList } = await this.getPolicyList({ userEmail, }); // ... }, }And in the action using it:
policyId: { propDefinition: [ expensify, "policyId", ({ employeeEmail }) => ({ userEmail: employeeEmail }), ], }Can you confirm the create-report action supplies userEmail like above so the policy dropdown populates correctly?
62-66: Avoid responseCode check for non-JSON responses.downloadFile (arraybuffer) and file-name responses (string) won’t have responseCode; current check silently no-ops but is confusing. Guard the check.
- if (response.responseCode < 200 || response.responseCode >= 300) { - throw new Error(JSON.stringify(response, null, 2)); - } + if (response && typeof response === "object" && "responseCode" in response) { + const { responseCode } = response as { responseCode: number }; + if (responseCode < 200 || responseCode >= 300) { + throw new Error(JSON.stringify(response, null, 2)); + } + }
96-110: Fix linter warning and improve readability.Expand the conditional spread across lines.
- ...(userEmail && { userEmail }), + ...(userEmail && { + userEmail, + }),components/expensify/actions/create-report/create-report.ts (3)
97-103: Make JSON parsing resilient to both object and string inputs; parse expenses once.Avoid double-parsing or runtime errors if
reportFieldsarrives as an object and ensureexpensesare parsed once and validated.Apply:
async run({ $ }) { const { policyId, employeeEmail, reportTitle, reportFields, expenses, } = this; + // Parse inputs safely + const parsedReportFields = typeof reportFields === "string" + ? utils.parseJson(reportFields) + : reportFields; + const parsedExpenses = utils.parseArray(expenses); + if (!Array.isArray(parsedExpenses) || parsedExpenses.length === 0) { + throw new Error("At least one expense is required."); + } const response = await this.app.createReport({ $, data: { employeeEmail, policyID: policyId, report: { title: reportTitle, - ...(reportFields && { - fields: utils.parseJson(reportFields), + ...(parsedReportFields && { + fields: parsedReportFields, }), }, - expenses: utils.parseArray(expenses), + expenses: parsedExpenses, }, });Also applies to: 82-90
34-61: Align the “Expenses” prop doc with the prop type (string[])Doc shows a full JSON array, but the prop is
string[](one item per expense). Update the example to a single expense JSON object to reduce confusion.Apply:
-**Example:** -```json -[ - { - "date": "2024-01-15", - "currency": "USD", - "merchant": "Hotel ABC", - "amount": 15000 - }, - { - "date": "2024-01-16", - "currency": "USD", - "merchant": "Restaurant XYZ", - "amount": 5000 - } -] -``` +Note: Add one expense per list item. +**Example (one item):** +```json +{ + "date": "2024-01-15", + "currency": "USD", + "merchant": "Hotel ABC", + "amount": 15000 +} +```
39-43: Minor doc polish: use ISO-8601 date and clarify amount unitsTighten phrasing to prevent user error.
Apply:
-- `date`: The date the expense was made (format yyyy-mm-dd) +- `date`: The date the expense was made (ISO 8601, YYYY-MM-DD) - `currency`: Three-letter currency code (e.g., "USD", "EUR", "CAD") - `merchant`: The name of the merchant -- `amount`: The amount in cents (e.g., 2500 for $25.00) +- `amount`: Integer amount in cents (e.g., 2500 for $25.00)Also applies to: 67-69
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
components/expensify/actions/create-expense/create-expense.ts(1 hunks)components/expensify/actions/create-report/create-report.ts(1 hunks)components/expensify/actions/export-report-to-pdf/export-report-to-pdf.ts(1 hunks)components/expensify/app/expensify.app.ts(3 hunks)components/expensify/common/utils.ts(1 hunks)components/expensify/package.json(1 hunks)
🧰 Additional context used
🪛 GitHub Check: Lint Code Base
components/expensify/common/utils.ts
[failure] 16-16:
'e' is defined but never used
components/expensify/app/expensify.app.ts
[failure] 106-106:
Expected a line break before this closing brace
[failure] 106-106:
Expected a line break after this opening brace
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: pnpm publish
- GitHub Check: Verify TypeScript components
- GitHub Check: Publish TypeScript components
🔇 Additional comments (5)
components/expensify/package.json (1)
3-3: Semver bump looks good.Minor version bump aligns with adding a new action. No further changes needed here.
components/expensify/actions/export-report-to-pdf/export-report-to-pdf.ts (1)
7-7: Version bump acknowledged.Action metadata updated to 0.0.3. No logic changes in this PR segment.
components/expensify/actions/create-report/create-report.ts (3)
20-28: EnsurepolicyIdoptions refresh after selectingemployeeEmailDynamic dependency looks correct; confirm the UI updates options when
employeeEmailchanges.You can quickly validate by opening the action in the UI and switching emails, or by checking the propDefinition for
policyIdinexpensify.app.tsto ensure it consumes theuserEmailparam.
5-11: Overall: solid action structure and prop wiringGood use of
propDefinitions, clear descriptions, and clean payload assembly. Once the small parsing/docs tweaks are in, this looks ready.
106-108: Keep original $summary export: Expensify createReport always returns top-levelreportNameandreportID. (integrations.expensify.com)
jcortes
force-pushed
the
expensify-create-report-action
branch
from
d1efda3 to
89d2d1d
Compare
3 participants
WHY
Resolves #18190
Summary by CodeRabbit