Enhance Fortnox component with new actions and properties #19208
Conversation
- Added new actions: `listAccounts`, `listFinancialYears`, `listSupplierInvoices`, `getSupplierInvoice`, `listArticles`, `listCustomers`, `listInvoices`, `listSuppliers`, and `sendInvoice`. - Introduced a new property for `supplierInvoiceNumber` in the Fortnox app. - Updated existing actions to version 0.0.2 and incremented the package version to 0.2.0 in package.json. - Improved request headers for better API compatibility. This update expands the functionality of the Fortnox component, allowing users to manage and retrieve various financial records more effectively.
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
WalkthroughAdds multiple Fortnox read actions and a get-supplier-invoice action, extends the Fortnox app with new propDefinitions and list/get methods, adjusts request headers, and increments version metadata for several actions and the Fortnox package. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Action as Action Module
participant App as Fortnox App
participant API as Fortnox API
rect rgb(230,248,230)
Action->>App: this.app.list*/getSupplierInvoice({ $, params or supplierInvoiceNumber })
end
rect rgb(235,240,255)
App->>API: HTTP request (Authorization, Content-Type: application/json, Accept: application/json)
API-->>App: JSON response (resource(s))
end
App-->>Action: return parsed resource(s)
Action-->>User: export $summary and return resource(s)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (2 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/fortnox/actions/create-article/create-article.mjs (1)
108-135: Numeric fields treat"0"as “not set” due to truthy checksUsing
this.directCost ? +this.directCost : undefined(same pattern for other numeric fields) prevents sending an explicit0value, since"0"is falsy. If0is a valid value for any of these fields, consider checking fornull/undefinedinstead of truthiness, e.g.:- DirectCost: this.directCost - ? +this.directCost - : undefined, + DirectCost: this.directCost === undefined || this.directCost === null + ? undefined + : +this.directCost,and similarly for
FreightCost,PurchasePrice,SalesPrice,QuantityInStock,Weight,Width, andHeight.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (17)
components/fortnox/actions/create-article/create-article.mjs(1 hunks)components/fortnox/actions/create-customer/create-customer.mjs(1 hunks)components/fortnox/actions/create-invoice-payment/create-invoice-payment.mjs(1 hunks)components/fortnox/actions/create-invoice/create-invoice.mjs(1 hunks)components/fortnox/actions/get-supplier-invoice/get-supplier-invoice.mjs(1 hunks)components/fortnox/actions/list-accounts/list-accounts.mjs(1 hunks)components/fortnox/actions/list-articles/list-articles.mjs(1 hunks)components/fortnox/actions/list-customers/list-customers.mjs(1 hunks)components/fortnox/actions/list-financial-years/list-financial-years.mjs(1 hunks)components/fortnox/actions/list-invoices/list-invoices.mjs(1 hunks)components/fortnox/actions/list-supplier-invoices/list-supplier-invoices.mjs(1 hunks)components/fortnox/actions/list-suppliers/list-suppliers.mjs(1 hunks)components/fortnox/actions/send-invoice/send-invoice.mjs(1 hunks)components/fortnox/actions/update-article/update-article.mjs(1 hunks)components/fortnox/actions/update-customer/update-customer.mjs(1 hunks)components/fortnox/fortnox.app.mjs(3 hunks)components/fortnox/package.json(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-12-12T19:23:09.039Z
Learnt from: jcortes
Repo: PipedreamHQ/pipedream PR: 14935
File: components/sailpoint/package.json:15-18
Timestamp: 2024-12-12T19:23:09.039Z
Learning: When developing Pipedream components, do not add built-in Node.js modules like `fs` to `package.json` dependencies, as they are native modules provided by the Node.js runtime.
Applied to files:
components/fortnox/package.json
⏰ 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)
- GitHub Check: pnpm publish
- GitHub Check: Verify TypeScript components
- GitHub Check: Publish TypeScript components
- GitHub Check: Lint Code Base
🔇 Additional comments (13)
components/fortnox/package.json (1)
3-3: Package version bump is consistent and dependency set is cleanBumping the Fortnox package to
0.2.0matches the expanded app surface and action updates, and you haven’t introduced any unnecessary dependencies (e.g., built‑in Node modules), which aligns with prior guidance. Based on learnings, this manifest looks good as‑is.components/fortnox/actions/create-invoice-payment/create-invoice-payment.mjs (1)
7-7: Action version metadata updated appropriatelyThe version bump to
"0.0.2"forfortnox-create-invoice-paymentis consistent with the Fortnox package upgrade and other action version increments; no runtime behavior change is introduced here.components/fortnox/actions/create-customer/create-customer.mjs (1)
7-7: Create Customer action version bump is consistentUpdating this action to version
"0.0.2"keeps it in sync with the Fortnox package and related actions; no further changes needed here.components/fortnox/actions/send-invoice/send-invoice.mjs (1)
7-7: Send Invoice action version updated in line with the packageThe
fortnox-send-invoiceaction’s version bump to"0.0.2"is consistent with the rest of the Fortnox changes and doesn’t alter runtime behavior.components/fortnox/actions/create-invoice/create-invoice.mjs (1)
2-8: Import reordering and version bump look safeBringing the
fortnoximport alongsideparseObjectand bumping the action version to"0.0.2"don’t change runtime behavior and keep this action consistent with the rest of the Fortnox package.components/fortnox/actions/update-customer/update-customer.mjs (1)
7-7: Update Customer action version bump is alignedThe change to version
"0.0.2"forfortnox-update-customermatches the other Fortnox actions and the package version; no functional changes here.components/fortnox/actions/update-article/update-article.mjs (1)
7-7: Update Article action version increment is consistentSetting the
fortnox-update-articleaction’s version to"0.0.2"is in line with the rest of the Fortnox integration changes and doesn’t affect the existing update logic.components/fortnox/actions/create-article/create-article.mjs (1)
7-7: Version bump is consistent with other Fortnox actionsBumping the action
versionto"0.0.2"is straightforward and aligns with the broader package version update.components/fortnox/actions/list-financial-years/list-financial-years.mjs (1)
16-21: Verify query parameter name for financial year dateYou’re sending the date as
params: { Date: this.date }, while the linked issue example mentionsGET /financialyears?date=<today>. IflistFinancialYearsforwards params directly, a casing mismatch (Datevsdate) could break filtering.Please double‑check the Fortnox docs /
fortnox.app.mjsimplementation and adjust either the key or the app method mapping if needed.Also applies to: 24-29
components/fortnox/actions/list-suppliers/list-suppliers.mjs (1)
3-26: List Suppliers action looks consistent and minimalThe action cleanly wraps
listSuppliers, returns theSuppliersarray, and exposes a clear summary with correct pluralization. No issues from this file alone.components/fortnox/actions/list-accounts/list-accounts.mjs (1)
3-26: Straightforward wrapper aroundlistAccountsAssuming
this.app.listAccountsreturns an array, the action and summary wiring are correct and match the pattern used by other list actions.Ensure
fortnox.app.mjs:listAccountsindeed returns an array of accounts (not an object with a nested property) soaccounts.lengthbehaves as expected.components/fortnox/actions/get-supplier-invoice/get-supplier-invoice.mjs (1)
3-31: Get Supplier Invoice action is wired correctlyProps and
runlogic correctly delegate togetSupplierInvoiceand return theSupplierInvoice, with a clear summary based on the selected invoice number.components/fortnox/actions/list-articles/list-articles.mjs (1)
14-85: List Articles filters and params are consistently wiredAll defined filter/sort props are correctly passed into the
paramsobject, and the summary logic is consistent with other list actions. No functional issues evident here.Also applies to: 87-108
components/fortnox/actions/list-supplier-invoices/list-supplier-invoices.mjs
Show resolved
Hide resolved
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
- Introduced `gln`, `glndelivery`, `lastmodified`, and `sortby` properties to enhance customer data handling. - This update improves the functionality and flexibility of the list-customers action.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
components/fortnox/actions/list-customers/list-customers.mjs(1 hunks)
⏰ 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: Publish TypeScript components
- GitHub Check: Verify TypeScript components
- GitHub Check: Lint Code Base
🔇 Additional comments (3)
components/fortnox/actions/list-customers/list-customers.mjs (3)
1-13: LGTM! Action metadata and annotations are correctly configured.The action is properly configured as a read-only list operation with appropriate annotations and version.
97-114: LGTM! Previous issue has been resolved.All filter props (including
gln,glndelivery,lastmodified, andsortby) are now correctly included in the params object passed tolistCustomers. The implementation properly handles all the filter parameters.
116-119: LGTM! Summary generation and return value are correct.The summary correctly handles singular/plural grammar, and the function appropriately returns the Customers array.
GTFalcao
left a comment
GTFalcao
left a comment
There was a problem hiding this comment.
I made some description suggestions and questions, moving it to QA since they don't impact functionality
components/fortnox/actions/list-supplier-invoices/list-supplier-invoices.mjs
Outdated
Show resolved
Hide resolved
components/fortnox/actions/list-supplier-invoices/list-supplier-invoices.mjs
Outdated
Show resolved
Hide resolved
components/fortnox/actions/list-supplier-invoices/list-supplier-invoices.mjs
Outdated
Show resolved
Hide resolved
components/fortnox/actions/list-supplier-invoices/list-supplier-invoices.mjs
Outdated
Show resolved
Hide resolved
components/fortnox/actions/list-supplier-invoices/list-supplier-invoices.mjs
Outdated
Show resolved
Hide resolved
components/fortnox/actions/list-supplier-invoices/list-supplier-invoices.mjs
Outdated
Show resolved
Hide resolved
components/fortnox/actions/list-supplier-invoices/list-supplier-invoices.mjs
Outdated
Show resolved
Hide resolved
|
Hello everyone, I have tested this PR and there're some test cases failed or needed improvement. Please check test reports below for more information:
|
- Removed the `booked` property from the `create-invoice-payment` action. - Updated descriptions in the `list-supplier-invoices` action to clarify invoice filtering options, correcting grammatical errors for better readability. These changes enhance the usability and accuracy of the Fortnox component.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
components/fortnox/actions/list-supplier-invoices/list-supplier-invoices.mjs (2)
6-6: Description suggests per‑supplier listing, but action doesn’t accept a supplier identifierThis action lists all supplier invoices (optionally filtered by status), not “all invoices for a supplier”. To avoid confusion, align the description with the actual behavior.
- description: "List all invoices for a supplier. [See the documentation](https://api.fortnox.se/apidocs#tag/fortnox_SupplierInvoices/operation/list_39)", + description: "List supplier invoices, optionally filtered by status. [See the documentation](https://api.fortnox.se/apidocs#tag/fortnox_SupplierInvoices/operation/list_39)",
16-51:filterprop is never sent to the APIThe
filterprop is defined but not passed intolistSupplierInvoices, so user input has no effect. This breaks the status filtering behavior and was already raised in a previous review.- props: { - app, - filter: { + props: { + app, + filter: { @@ - optional: true, - }, - }, - async run({ $ }) { - const { SupplierInvoices } = await this.app.listSupplierInvoices({ - $, - }); + optional: true, + }, + }, + async run({ $ }) { + const params = {}; + if (this.filter) { + params.filter = this.filter; + } + + const { SupplierInvoices } = await this.app.listSupplierInvoices({ + $, + params, + });Also applies to: 53-56
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
components/fortnox/actions/create-invoice-payment/create-invoice-payment.mjs(2 hunks)components/fortnox/actions/list-supplier-invoices/list-supplier-invoices.mjs(1 hunks)
⏰ 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)
- GitHub Check: Lint Code Base
- GitHub Check: pnpm publish
- GitHub Check: Verify TypeScript components
- GitHub Check: Publish TypeScript components
🔇 Additional comments (1)
components/fortnox/actions/create-invoice-payment/create-invoice-payment.mjs (1)
7-7: Version bump matches the behavior changeBumping the action version after removing the
Bookedprop and payload field is appropriate; no issues with this change.
components/fortnox/actions/create-invoice-payment/create-invoice-payment.mjs
Show resolved
Hide resolved
components/fortnox/actions/list-supplier-invoices/list-supplier-invoices.mjs
Outdated
Show resolved
Hide resolved
|
Hello everyone, I have tested this PR and there're some test cases failed or needed improvement. Please check test reports below for more information:
|
…r-invoices.mjs Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
- Introduced `termsOfDeliveries` and `termsOfPayments` properties in the Fortnox app for invoice handling. - Added corresponding methods to fetch delivery and payment terms asynchronously. - Updated `create-invoice` action to utilize the new properties. This enhancement improves the invoice creation process by allowing users to specify terms of delivery and payment more effectively.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
components/fortnox/fortnox.app.mjs (2)
109-120: PaginatesupplierInvoiceNumberoptions to handle large datasetsUnlike the other ID props, this options loader doesn’t accept
{ page }or forward it tolistSupplierInvoices, so the dropdown can’t paginate and may only expose a subset of invoices (or fetch a large page at once).To align with
customerNumber/articleNumberand keep the UI responsive, consider:- supplierInvoiceNumber: { + supplierInvoiceNumber: { type: "string", label: "Supplier Invoice Number", description: "The number of the supplier invoice", - async options() { - const { SupplierInvoices: supplierInvoices } = await this.listSupplierInvoices(); + async options({ page }) { + const { SupplierInvoices: supplierInvoices } = await this.listSupplierInvoices({ + params: { + page: page + 1, + }, + }); return supplierInvoices?.map((invoice) => ({ label: `${invoice.InvoiceNumber} - ${invoice.Total}`, value: invoice.GivenNumber, })) || []; }, },This mirrors the existing pagination pattern used for customers, articles, etc.
313-315: Merge custom headers with defaults to avoid droppingAuthorizationRight now, any caller that passes
opts.headerswill overwrite the entire headers object, potentially droppingAuthorizationand making it harder to override justAccept/Content-Type.Prefer merging custom headers into the defaults while ensuring
Authorizationremains correct:- _makeRequest({ - $ = this, path, ...opts - }) { - return axios($, { - url: `${this._baseUrl()}${path}`, - headers: { - "Authorization": `Bearer ${this.$auth.oauth_access_token}`, - "Content-Type": "application/json", - "Accept": "application/json", - }, - ...opts, - }); - }, + _makeRequest({ + $ = this, path, ...opts + }) { + const { + headers: customHeaders, + ...restOpts + } = opts; + + const headers = { + "Authorization": `Bearer ${this.$auth.oauth_access_token}`, + "Content-Type": "application/json", + "Accept": "application/json", + ...customHeaders, + }; + + return axios($, { + url: `${this._baseUrl()}${path}`, + headers, + ...restOpts, + }); + },This keeps JSON defaults while still allowing per-call overrides.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
components/fortnox/actions/create-invoice/create-invoice.mjs(2 hunks)components/fortnox/actions/list-accounts/list-accounts.mjs(1 hunks)components/fortnox/fortnox.app.mjs(3 hunks)
⏰ 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)
- GitHub Check: pnpm publish
- GitHub Check: Publish TypeScript components
- GitHub Check: Lint Code Base
- GitHub Check: Verify TypeScript components
🔇 Additional comments (1)
components/fortnox/fortnox.app.mjs (1)
420-457: New list/get helpers are consistent with existing patternsThe new helpers (
listAccounts,listFinancialYears,listSupplierInvoices,getSupplierInvoice,listTermsOfPayments,listTermsOfDeliveries) follow the same shape as existing list/get methods and keep concerns centralized in_makeRequest. No issues from a structural/API-usage perspective.
| termsOfDeliveries: { | ||
| propDefinition: [ | ||
| fortnox, | ||
| "termsOfDeliveries", | ||
| ], | ||
| optional: true, | ||
| }, | ||
| termsOfPayment: { | ||
| type: "string", | ||
| label: "Terms of Payment", | ||
| description: "The terms of payment of the invoice", | ||
| propDefinition: [ | ||
| fortnox, | ||
| "termsOfPayments", | ||
| ], | ||
| optional: true, |
There was a problem hiding this comment.
Fix mismatch between termsOfDeliveries prop and TermsOfDelivery payload field
You renamed the prop to termsOfDeliveries, but the payload still reads from this.termsOfDelivery, so TermsOfDelivery is always undefined and never sent.
Update the payload to use the actual prop name:
termsOfDeliveries: {
propDefinition: [
fortnox,
"termsOfDeliveries",
],
optional: true,
},
@@
- TermsOfDelivery: this.termsOfDelivery,
+ TermsOfDelivery: this.termsOfDeliveries,
TermsOfPayment: this.termsOfPayment,This wires the selected term of delivery into the created invoice correctly.
Also applies to: 148-150
🤖 Prompt for AI Agents
In components/fortnox/actions/create-invoice/create-invoice.mjs around lines
108-120 (and similarly at 148-150), the code renamed the prop to
termsOfDeliveries but the payload still reads this.termsOfDelivery, causing
TermsOfDelivery to be undefined; update the payload to read
this.termsOfDeliveries (and any other occurrences) so the TermsOfDelivery field
is set from the actual prop name and sent in the created invoice.
| async run({ $ }) { | ||
| const { Accounts: accounts } = await this.app.listAccounts({ | ||
| $, | ||
| }); | ||
|
|
||
| $.export("$summary", `Successfully retrieved ${accounts.length} account${accounts.length === 1 | ||
| ? "" | ||
| : "s"}`); | ||
| return accounts; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Default accounts to an empty array to avoid potential runtime errors
If the API ever omits or nulls the Accounts field, accounts.length will throw. A small defensive tweak keeps behavior the same when data is present:
- const { Accounts: accounts } = await this.app.listAccounts({
+ const { Accounts: accounts = [] } = await this.app.listAccounts({
$,
});This also guarantees the action always returns an array.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| async run({ $ }) { | |
| const { Accounts: accounts } = await this.app.listAccounts({ | |
| $, | |
| }); | |
| $.export("$summary", `Successfully retrieved ${accounts.length} account${accounts.length === 1 | |
| ? "" | |
| : "s"}`); | |
| return accounts; | |
| async run({ $ }) { | |
| const { Accounts: accounts = [] } = await this.app.listAccounts({ | |
| $, | |
| }); | |
| $.export("$summary", `Successfully retrieved ${accounts.length} account${accounts.length === 1 | |
| ? "" | |
| : "s"}`); | |
| return accounts; |
🤖 Prompt for AI Agents
In components/fortnox/actions/list-accounts/list-accounts.mjs around lines 17 to
25, the code assumes this.app.listAccounts() always returns an Accounts array so
accounts.length can be read; if Accounts is null/undefined this will throw.
Change the destructuring to default Accounts to an empty array (e.g., const {
Accounts: accounts = [] } = ...) or assign accounts = Accounts || [] after the
call, then use accounts.length for the $summary and return accounts so the
action always returns an array.
|
Hello everyone, I have tested this PR and there're some test cases failed or needed improvement. Please check test reports below for more information:
|
Resolves #19182
Summary by CodeRabbit
New Features
Bug Fixes
bookedinput from Invoice Payment and clarified date formats for due/payment dates.Chores
✏️ Tip: You can customize this high-level summary in your review settings.