-
Notifications
You must be signed in to change notification settings - Fork 5.5k
Enhance Fortnox component with new actions and properties #19208
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
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/list actions and a supplier-invoice get action, extends the Fortnox app with new propDefinitions and list/get methods, updates HTTP request headers, removes several props/fields from invoice-payment and invoice actions, bumps many action versions, and increments the Fortnox package version. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Action as Action Module
participant App as Fortnox App
participant API as Fortnox API
Action->>App: call this.app.list*/getSupplierInvoice({ $, params or supplierInvoiceNumber })
App->>API: HTTP request (Authorization, Content-Type: application/json, Accept: application/json)
API-->>App: JSON response (resource(s))
App-->>Action: return parsed resource(s)
Action-->>User: export $summary and return resource(s)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (1)📚 Learning: 2024-10-08T16:42:59.225ZApplied to files:
⏰ 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 (3)
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 |
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.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
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 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
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:
|
…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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
|
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:
|
…te property from create-invoice-payment action - Enhanced the description for the due date in the create-invoice action to specify the required format (YYYY-MM-DD). - Removed the dueDate property from the create-invoice-payment action to streamline the payment process. These changes improve clarity and usability in the Fortnox component.
|
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:
|
- Eliminated `termsOfDeliveries` and `termsOfPayments` properties from the Fortnox app and the create-invoice action to streamline the invoice creation process. - Updated the create-invoice action to reflect these changes, ensuring a more focused and simplified interface. This refactor enhances the clarity and usability of the Fortnox component.
|
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 unnecessary properties: `customerNumber`, `invoiceTotal`, and `modeOfPayment` from the create-invoice-payment action to simplify the payment process. - Updated the action's payload structure to reflect these changes, enhancing clarity and usability. This refactor streamlines the invoice payment process within the Fortnox component.
|
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:
|
- Changed the description of the paymentDate property to specify the required format as YYYY-MM-DD, enhancing clarity for users. This update improves the usability of the Fortnox component by providing clear formatting instructions.
|
Hi everyone, all test cases are passed! Ready for release! Test reports
|
|
/approve |
Resolves #19182
Summary by CodeRabbit
New Features
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.