components/quickbooks/actions/get-invoice/get-invoice.mjs (2)

Line range hint 12-16: Fix inconsistency in invoiceId requirement

The invoiceId prop is marked as optional (optional: true) but the run method requires it and throws an error if not provided. This creates a confusing developer experience.

Apply this diff to fix the inconsistency:

    invoiceId: {
      label: "Invoice ID",
      type: "string",
      description: "Id of the invoice to get details of.",
-     optional: true,
    },

---

Line range hint 24-40: Enhance error handling robustness

While the basic implementation is sound, consider improving error handling:

1. Add try-catch for API errors
2. Validate response structure before accessing nested properties

Consider applying this enhancement:

   async run({ $ }) {
     if (!this.invoiceId) {
       throw new ConfigurationError("Must provide invoiceId parameter.");
     }
 
-    const response = await this.quickbooks.getInvoice({
-      $,
-      invoiceId: this.invoiceId,
-      params: {
-        minorversion: this.minorVersion,
-      },
-    });
-
-    if (response) {
-      $.export("summary", `Successfully retrieved invoice with id ${response.Invoice.Id}`);
-    }
+    try {
+      const response = await this.quickbooks.getInvoice({
+        $,
+        invoiceId: this.invoiceId,
+        params: {
+          minorversion: this.minorVersion,
+        },
+      });
+
+      if (!response?.Invoice?.Id) {
+        throw new Error("Received invalid response structure from QuickBooks API");
+      }
+
+      $.export("summary", `Successfully retrieved invoice with id ${response.Invoice.Id}`);
+      return response;
+    } catch (error) {
+      throw new Error(`Failed to retrieve invoice: ${error.message}`);
+    }
-    return response;
   },

components/quickbooks/actions/sparse-update-invoice/sparse-update-invoice.mjs (3)

Line range hint 38-46: Enhance line items parsing and validation

While the current implementation handles JSON parsing errors, it could benefit from additional validation of the parsed line items structure to ensure they match QuickBooks' expected format.

Consider adding structure validation:

 async run({ $ }) {
   try {
-    this.lineItems = this.lineItems.map((lineItem) => typeof lineItem === "string"
-      ? JSON.parse(lineItem)
-      : lineItem);
+    this.lineItems = this.lineItems.map((lineItem) => {
+      const parsed = typeof lineItem === "string" ? JSON.parse(lineItem) : lineItem;
+      if (!parsed.Amount || !parsed.DetailType) {
+        throw new ConfigurationError(`Invalid line item structure. Each item must include Amount and DetailType`);
+      }
+      return parsed;
+    });
   } catch (error) {
     throw new ConfigurationError(`We got an error trying to parse the Line Items prop. Error: ${error}`);
   }

---

Line range hint 52-52: Add defensive check for Invoice.Line array

The current implementation might throw an error if Invoice.Line is undefined.

Consider this safer approach:

-if (this.lineItems.length) Invoice.Line?.push(...this.lineItems);
+if (this.lineItems.length) {
+  Invoice.Line = Array.isArray(Invoice.Line) ? Invoice.Line : [];
+  Invoice.Line.push(...this.lineItems);
+}

---

Line range hint 61-65: Add response validation before accessing nested properties

The code assumes the response structure without validation, which could lead to runtime errors.

Consider adding proper validation:

-if (response) {
+if (response?.Invoice?.Id) {
   $.export("summary", `Successfully updated invoice with id ${response.Invoice.Id}`);
+} else {
+  throw new Error("Unexpected response format from QuickBooks API");
 }

components/quickbooks/actions/search-vendors/search-vendors.mjs (3)

Line range hint 44-47: Improve error message specificity

The error message could be more specific about the validation requirements for each parameter.

-      throw new ConfigurationError("Must provide includeClause, whereClause parameters.");
+      throw new ConfigurationError(
+        `Missing required parameters: ${[
+          !this.includeClause && "includeClause",
+          !this.whereClause && "whereClause",
+        ].filter(Boolean).join(", ")}`
+      );

---

Line range hint 61-63: Fix unnecessary OR condition in maxResults concatenation

The OR condition with an empty string is redundant as the template literal will handle undefined/null values.

-      maxResults = ` MAXRESULTS ${this.maxResults}` || "";
+      maxResults = ` MAXRESULTS ${this.maxResults}`;

---

Line range hint 66-83: Enhance query construction and response handling

Consider the following improvements:

1. Use array join for better query construction
2. Add validation for numeric parameters (startPosition, maxResults)
3. Include vendor count or IDs in the success message

+    // Validate numeric parameters
+    if (this.startPosition && isNaN(this.startPosition)) {
+      throw new ConfigurationError("Start Position must be a number");
+    }
+    if (this.maxResults && isNaN(this.maxResults)) {
+      throw new ConfigurationError("Max Results must be a number");
+    }
+
+    // Construct query using array join
+    const queryParts = [
+      `select ${this.includeClause}`,
+      `from Vendor`,
+      `where ${this.whereClause}`,
+      this.orderClause && `ORDERBY ${this.orderClause}`,
+      this.startPosition && `STARTPOSITION ${this.startPosition}`,
+      this.maxResults && `MAXRESULTS ${this.maxResults}`,
+    ].filter(Boolean);
+
+    const query = queryParts.join(" ");
     const response = await this.quickbooks.query({
       $,
       params: {
         minorversion: this.minorVersion,
         query,
       },
     });

     if (response) {
-      $.export("summary", "Successfully retrieved vendors");
+      const vendorCount = response.QueryResponse?.Vendor?.length ?? 0;
+      $.export("summary", `Successfully retrieved ${vendorCount} vendors`);
     }

components/quickbooks/actions/create-invoice/create-invoice.mjs (1)

Line range hint 44-54: Enhance error handling for financial data validation

While the current error handling is good, consider these improvements for financial data integrity:

1. Separate the validation of lineItems and customerRefValue to provide more specific error messages
2. Add validation for the structure of parsed line items (e.g., required fields, numeric values)

Here's a suggested implementation:

- if (!this.lineItems || !this.customerRefValue) {
-   throw new ConfigurationError("Must provide lineItems, and customerRefValue parameters.");
+ if (!this.lineItems) {
+   throw new ConfigurationError("Must provide lineItems parameter.");
+ }
+ if (!this.customerRefValue) {
+   throw new ConfigurationError("Must provide customerRefValue parameter.");
+ }

  try {
    this.lineItems = this.lineItems.map((lineItem) => typeof lineItem === "string"
      ? JSON.parse(lineItem)
      : lineItem);
+   // Validate structure of each line item
+   this.lineItems.forEach((item, index) => {
+     if (!item.Amount || typeof item.Amount !== 'number') {
+       throw new Error(`Invalid Amount in line item ${index + 1}`);
+     }
+     if (!item.Description) {
+       throw new Error(`Missing Description in line item ${index + 1}`);
+     }
+   });
  } catch (error) {
-   throw new ConfigurationError(`We got an error trying to parse the LineItems. Error: ${error}`);
+   throw new ConfigurationError(`Invalid line items format or structure: ${error.message}`);
  }

components/quickbooks/common/constants.mjs (4)

5-32: LGTM! Consider adding JSDoc documentation.

The AP aging report columns are comprehensive and align well with QuickBooks API requirements. They include all necessary fields for vendor details, aging information, and multi-currency support.

Consider adding JSDoc documentation to describe the purpose and usage of this constant:

+/**
+ * Column options for Accounts Payable Aging Detail Report
+ * These columns align with the QuickBooks Online API response fields
+ * @type {string[]}
+ */
export const AP_AGING_REPORT_COLUMNS = [

---

72-96: LGTM! Consider adding JSDoc documentation.

The date macro options are comprehensive and well-structured, covering all standard financial reporting periods including fiscal year options.

Consider adding JSDoc documentation:

+/**
+ * Standard QuickBooks date macro options for report filtering
+ * Includes daily, weekly, monthly, quarterly, and yearly periods
+ * with support for fiscal year reporting
+ * @type {string[]}
+ */
export const DATE_MACRO_OPTIONS = [

---

98-106: Consider adding more payment methods.

The current list of payment methods might be incomplete. QuickBooks supports additional payment methods that are commonly used.

Consider adding these additional payment methods:

export const PAYMENT_METHOD_OPTIONS = [
  "Cash",
  "Check",
  "Dinners Club",
  "American Express",
  "Discover",
  "MasterCard",
  "Visa",
+  "ACH",
+  "Wire Transfer",
+  "PayPal",
+  "Bank Transfer",
+  "EFT",
+  "Other",
];

---

108-125: LGTM! Consider adding JSDoc documentation.

The account types are comprehensive and follow QuickBooks' standard categorization.

Consider adding JSDoc documentation:

+/**
+ * Standard QuickBooks account types
+ * Used for filtering and categorizing accounts in reports
+ * @type {string[]}
+ */
export const ACCOUNT_TYPE_OPTIONS = [

components/quickbooks/actions/create-bill/create-bill.mjs (2)

Line range hint 44-49: Consider enhancing input validation for lineItems

While the error handling for JSON parsing is good, consider adding validation for:

1. Line item structure against QuickBooks API requirements (DetailType, Amount, etc.)
2. Array size limits to prevent potential API limits issues

Example validation:

 try {
   this.lineItems = this.lineItems.map((lineItem) => typeof lineItem === "string"
     ? JSON.parse(lineItem)
     : lineItem);
+  // Validate structure of each line item
+  this.lineItems.forEach((item, index) => {
+    if (!item.DetailType || !item.Amount) {
+      throw new ConfigurationError(
+        `Line item at index ${index} missing required fields DetailType or Amount`
+      );
+    }
+  });
+  // Check array size
+  if (this.lineItems.length > 1000) { // adjust limit as per API docs
+    throw new ConfigurationError("Number of line items exceeds maximum limit");
+  }
 } catch (error) {
   throw new ConfigurationError(`We got an error trying to parse the LineItems. Error: ${error}`);
 }

---

Line range hint 65-77: Enhance security measures for financial data handling

Consider implementing these security improvements:

1. Sanitize vendor reference inputs to prevent injection attacks
2. Validate currency reference values against a whitelist of supported currencies
3. Limit exposure of internal IDs in success messages

Example implementation:

 const response = await this.quickbooks.createBill({
   $,
   data: {
     VendorRef: {
-      value: this.vendorRefValue,
-      name: this.vendorRefName,
+      value: this.vendorRefValue.trim(),
+      name: this.vendorRefName?.trim(),
     },
     Line: this.lineItems,
     CurrencyRef: {
-      value: this.currencyRefValue,
+      value: this.currencyRefValue?.toUpperCase(),
       name: this.currencyRefName,
     },
   },
   params: {
     minorversion: this.minorVersion,
   },
 });

 if (response) {
-  $.export("summary", `Successfully created bill with id ${response.Bill.Id}`);
+  $.export("summary", "Successfully created bill");
+  // Export ID separately if needed
+  $.export("billId", response.Bill.Id);
 }

components/quickbooks/actions/create-purchase/create-purchase.mjs (1)

Line range hint 79-81: Fix property name mismatch to prevent runtime error

There's a case mismatch between the prop definition and its usage:

• Prop is defined as minorversion (lowercase)
• But accessed as this.minorVersion (camelCase)

This will cause the property to be undefined at runtime as JavaScript is case-sensitive.

Apply this fix:

       params: {
-        minorversion: this.minorVersion,
+        minorversion: this.minorversion,
       },

components/quickbooks/actions/create-ap-aging-report/create-ap-aging-report.mjs (1)

1-9: LGTM! Consider enhancing the documentation.

The imports and action metadata are well-structured. The documentation link is helpful.

Consider adding examples of the expected input/output in the description to help users understand how to use this action effectively.

components/quickbooks/quickbooks.app.mjs (1)

219-234: Reduce code duplication in pagination logic

Multiple async options methods share similar pagination logic. Consider extracting the common pattern into a reusable utility function.

+ const getPagedOptions = async (query, entityType, labelField = "Name") => {
+   const position = 1 + (page * 10);
+   const { QueryResponse } = await this.query({
+     params: {
+       query: `select * from ${entityType} maxresults 10 ${page
+         ? `startposition ${position}`
+         : ""} `,
+     },
+   });
+   const records = QueryResponse[entityType];
+   return records?.map(({
+     Id: value, [labelField]: label,
+   }) => ({
+     value,
+     label,
+   })) || [];
+ };

Then use it in each options method:

 async options({ page }) {
-  const position = 1 + (page * 10);
-  const { QueryResponse: { Term: terms } } = await this.query({
-    params: {
-      query: `select * from term maxresults 10 ${page
-        ? `startposition ${position}`
-        : ""} `,
-    },
-  });
-  return terms?.map(({
-    Id: value, Name: label,
-  }) => ({
-    value,
-    label,
-  })) || [];
+  return await getPagedOptions("term", "Term");
 }

Also applies to: 241-256, 263-278, 285-300, 307-322, 329-344

components/quickbooks/actions/create-pl-report/create-pl-report.mjs (2)

9-9: Update description to match the action name and correct documentation link

The action is named "Create Profit and Loss Detail Report", but the description refers to "profit and loss report" and links to the general report documentation. For consistency and clarity, update the description to specify "profit and loss detail report" and update the documentation link to point to the correct API endpoint.

Apply this diff:

  name: "Create Profit and Loss Detail Report",
- description: "Creates a profit and loss report in Quickbooks Online. [See the documentation](https://developer.intuit.com/app/developer/qbo/docs/api/accounting/all-entities/profitandloss#query-a-report)",
+ description: "Creates a profit and loss detail report in QuickBooks Online. [See the documentation](https://developer.intuit.com/app/developer/qbo/docs/api/accounting/all-entities/profitandlossdetail#profitandlossdetail)",

---

58-58: Typo in description: change "gain" to "gains"

There's a minor typo in the description. It should read "gains and losses" instead of "gain and losses".

Apply this diff:

  description: "Specifies whether unrealized gain and losses are included in the report",
+ // Change "gain" to "gains"
- description: "Specifies whether unrealized gain and losses are included in the report",
+ description: "Specifies whether unrealized gains and losses are included in the report",

Configuration used: CodeRabbit UI
Review profile: CHILL

#!/bin/bash
# Description: Check for presence of changelog entries and related version bumps

# Check for CHANGELOG.md files
echo "Searching for changelog files..."
fd -g 'CHANGELOG*.md'

# Look for other QuickBooks components with version changes
echo -e "\nChecking for version changes in other QuickBooks components..."
rg -l '"version": "0.1.6"' components/quickbooks/

#!/bin/bash
# Let's try a different approach to find version changes

# Check version patterns in QuickBooks components
echo "Checking for version patterns in QuickBooks components..."
rg "version.*0\.1\.[56]" components/quickbooks/ -A 1 -B 1

# Check recent changes in changelog
echo -e "\nChecking recent changelog entries..."
tail -n 50 packages/sdk/CHANGELOG.md

# Look for related QuickBooks component changes
echo -e "\nChecking for recent QuickBooks component changes..."
rg "version.*0\.1\." components/quickbooks/actions/ --type js --type mjs

#!/bin/bash
# Search for rate limiting and error handling patterns across QuickBooks components
rg -l 'minorversion' components/quickbooks/ | xargs rg -A 5 'sparseUpdate|getInvoice'

# Look for retry mechanisms
rg -l 'retry' components/quickbooks/ | xargs cat

#!/bin/bash
# Search for similar query patterns in other QuickBooks actions
rg -l 'select .* from \w+ where' --type js

# Look for pagination implementations
rg -l 'STARTPOSITION|MAXRESULTS' --type js

# Search for rate limiting handling
rg -l 'RateLimitExceeded|ThrottleExceeded' --type js

#!/bin/bash
# Search for similar query patterns with more context
rg 'select .* from \w+' -A 3 -B 3 components/quickbooks/

# Look for query-related code in QuickBooks components
fd . components/quickbooks/ -e mjs -e js --exec grep -l "query"

# Search for query examples or documentation
fd README.md -X grep -A 5 -B 5 -i "query"

# Look for error handling patterns
rg "throw new Error|catch" components/quickbooks/

#!/bin/bash
# Description: Verify async options implementation

# Check propDefinitions in QuickBooks app
rg -A 10 "termIds.*propDefinition" 
rg -A 10 "vendorIds.*propDefinition"

#!/bin/bash
# Let's try a broader search to locate the QuickBooks app file and verify the implementation

# Find QuickBooks app file
fd -t f quickbooks.app

# Search for terms and vendors related code
rg -l "terms?" --type js --type mjs
rg -l "vendor" --type js --type mjs

# Get content of the specific action file
cat "components/quickbooks/actions/create-ap-aging-report/create-ap-aging-report.mjs"

#!/bin/bash
# Let's check the QuickBooks app file to verify the propDefinitions for termIds and vendorIds

cat "components/quickbooks/quickbooks.app.mjs"