-
Notifications
You must be signed in to change notification settings - Fork 5.4k
Jobber - Search & Filter Quotes #17263
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
base: master
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 3 Skipped Deployments
|
WalkthroughThis update introduces two new Jobber action components: "Filter Quotes" and "Search Quotes," enabling users to retrieve and filter quotes using the Jobber GraphQL API. Supporting utilities, including paginated resource fetching and GraphQL field constants, were added. Several existing actions and the package version were incremented, with no functional changes to those actions. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Action (Filter/Search Quotes)
participant Jobber App
participant Jobber API
User->>Action (Filter/Search Quotes): Provide filter/search criteria
Action (Filter/Search Quotes)->>Jobber App: Request paginated quotes (with criteria)
loop For each page
Jobber App->>Jobber API: Fetch quotes (GraphQL)
Jobber API-->>Jobber App: Return quotes page
Jobber App-->>Action (Filter/Search Quotes): Yield quotes
end
Action (Filter/Search Quotes)-->>User: Return filtered/searched quotes
Assessment against linked issues
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
components/jobber/jobber.app.mjsOops! Something went wrong! :( ESLint: 8.57.1 Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'jsonc-eslint-parser' imported from /eslint.config.mjs components/jobber/actions/filter-quotes/filter-quotes.mjsOops! Something went wrong! :( ESLint: 8.57.1 Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'jsonc-eslint-parser' imported from /eslint.config.mjs 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms (4)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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: 1
🧹 Nitpick comments (6)
components/jobber/actions/search-quotes/search-quotes.mjs (2)
7-7
: Fix typo in description.There's a typo in the description: "termin" should be "term".
- description: "Search for quotes using a search termin Jobber. [See the documentation](https://developer.getjobber.com/docs)", + description: "Search for quotes using a search term in Jobber. [See the documentation](https://developer.getjobber.com/docs)",
26-52
: Consider adding error handling for GraphQL queries.The current implementation doesn't include explicit error handling for potential GraphQL query failures or network issues. Consider wrapping the
getPaginatedResources
call in a try-catch block to provide more informative error messages.async run({ $ }) { + try { const query = `query SearchQuotes($first: Int, $after: String, $searchTerm: String) { quotes(first: $first, after: $after, searchTerm: $searchTerm) { nodes { ${constants.QUOTE_FIELDS} } pageInfo { endCursor hasNextPage } } }`; const args = { searchTerm: this.searchTerm, }; const quotes = await this.jobber.getPaginatedResources({ query, args, resourceKey: "quotes", max: this.maxResults, }); $.export("$summary", `Successfully found ${quotes.length} quote${quotes.length === 1 ? "" : "s"}`); return quotes; + } catch (error) { + throw new Error(`Failed to search quotes: ${error.message}`); + } },components/jobber/actions/filter-quotes/filter-quotes.mjs (3)
83-83
: Remove debug console.log statement.The console.log statement appears to be leftover debug code and should be removed before production.
- console.log(args);
72-80
: Consider safer type conversion and validation.The current implementation uses the unary
+
operator for type conversion, which can produceNaN
for invalid inputs. Consider adding validation or using safer conversion methods.quoteNumber: this.quoteNumber ? { - eq: +this.quoteNumber, + eq: parseInt(this.quoteNumber, 10), } : undefined, cost: this.cost ? { - eq: +this.cost, + eq: parseFloat(this.cost), } : undefined,Alternatively, add validation:
+ // Validate numeric inputs + if (this.quoteNumber && isNaN(+this.quoteNumber)) { + throw new Error("Quote number must be a valid number"); + } + if (this.cost && isNaN(+this.cost)) { + throw new Error("Cost must be a valid number"); + } + const args = { filter: { status: this.status, clientId: this.clientId, quoteNumber: this.quoteNumber ? { eq: +this.quoteNumber, } : undefined, cost: this.cost ? { eq: +this.cost, } : undefined, }, };
54-94
: Add error handling consistent with search action.Similar to the search action, this component would benefit from explicit error handling around the GraphQL query execution.
async run({ $ }) { + try { const query = `query FilterQuotes($first: Int, $after: String, $filter: QuoteFilterAttributes) { quotes(first: $first, after: $after, filter: $filter) { nodes { ${constants.QUOTE_FIELDS} } pageInfo { endCursor hasNextPage } } }`; const args = { filter: { status: this.status, clientId: this.clientId, quoteNumber: this.quoteNumber ? { eq: +this.quoteNumber, } : undefined, cost: this.cost ? { eq: +this.cost, } : undefined, }, }; const quotes = await this.jobber.getPaginatedResources({ query, args, resourceKey: "quotes", max: this.maxResults, }); $.export("$summary", `Successfully found ${quotes.length} quote${quotes.length === 1 ? "" : "s"}`); return quotes; + } catch (error) { + throw new Error(`Failed to filter quotes: ${error.message}`); + } },components/jobber/jobber.app.mjs (1)
127-134
: Consider adding error handling to getPaginatedResources.While the method itself is simple, adding error handling could provide better debugging information when pagination fails.
async getPaginatedResources(args) { + try { const results = []; const resources = this.paginate(args); for await (const resource of resources) { results.push(resource); } return results; + } catch (error) { + throw new Error(`Failed to paginate resources: ${error.message}`); + } },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (8)
components/jobber/actions/create-client/create-client.mjs
(1 hunks)components/jobber/actions/create-quote/create-quote.mjs
(1 hunks)components/jobber/actions/create-request/create-request.mjs
(1 hunks)components/jobber/actions/filter-quotes/filter-quotes.mjs
(1 hunks)components/jobber/actions/search-quotes/search-quotes.mjs
(1 hunks)components/jobber/common/constants.mjs
(1 hunks)components/jobber/jobber.app.mjs
(1 hunks)components/jobber/package.json
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Lint Code Base
- GitHub Check: Publish TypeScript components
- GitHub Check: pnpm publish
- GitHub Check: Verify TypeScript components
🔇 Additional comments (7)
components/jobber/actions/create-request/create-request.mjs (1)
9-9
: LGTM! Routine version bump.The version increment is consistent with other action components in this PR and contains no functional changes.
components/jobber/actions/create-client/create-client.mjs (1)
8-8
: LGTM! Consistent version increment.The version bump aligns with other action components and introduces no functional changes.
components/jobber/actions/create-quote/create-quote.mjs (1)
7-7
: LGTM! Version bump follows established pattern.The version increment is consistent with other action components in this PR.
components/jobber/common/constants.mjs (1)
1-55
: LGTM! Well-structured GraphQL field definitions.The QUOTE_FIELDS constant provides a comprehensive and well-organized GraphQL fragment for retrieving quote data. The field selection covers all essential quote information including nested relationships (client, jobs, lineItems, etc.) and follows proper GraphQL syntax.
This centralization will support consistency across the new quote-related actions.
components/jobber/package.json (1)
3-3
: LGTM! Appropriate minor version increment.The version bump from "0.1.1" to "0.2.1" correctly reflects the addition of new quote filtering and searching functionality, following semantic versioning principles for new features.
components/jobber/actions/search-quotes/search-quotes.mjs (1)
1-2
: Verify constants.QUOTE_FIELDS is properly defined.The code imports and uses
constants.QUOTE_FIELDS
but we should ensure this constant exists and contains the correct GraphQL fields for quotes.#!/bin/bash # Description: Verify that QUOTE_FIELDS constant exists in the constants file # Expected: Find the QUOTE_FIELDS constant definition rg -A 10 "QUOTE_FIELDS" components/jobber/common/constants.mjscomponents/jobber/actions/filter-quotes/filter-quotes.mjs (1)
16-23
: Verify quote status options are complete and accurate.The hardcoded status options should be verified against the Jobber API documentation to ensure they're complete and accurate.
What are the valid quote status values in the Jobber GraphQL API?
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.
Hi @michelle0927 lgtm! Ready for QA!
Resolves #17158
Summary by CodeRabbit
New Features
Chores