Pipedrive - search-notes & remove-duplicate-notes actions #17045
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 3 Skipped Deployments
|
WalkthroughThis update introduces two new actions for Pipedrive: one for searching notes with advanced filtering and another for detecting and removing duplicate notes, including optional keyword-based filtering. Supporting methods for note management and pagination are added to the app module. Several existing actions and sources receive version number bumps, with no other logic changes. Changes
Sequence Diagram(s)Remove Duplicate Notes ActionsequenceDiagram
participant User
participant RemoveDuplicateNotesAction
participant PipedriveApp
User->>RemoveDuplicateNotesAction: Run action with object ID(s) and optional keyword
RemoveDuplicateNotesAction->>PipedriveApp: getNotes (with filters)
PipedriveApp-->>RemoveDuplicateNotesAction: Return all notes
RemoveDuplicateNotesAction->>RemoveDuplicateNotesAction: Filter notes by keyword (if provided)
RemoveDuplicateNotesAction->>RemoveDuplicateNotesAction: Identify duplicates by normalized content
loop For each duplicate note
RemoveDuplicateNotesAction->>PipedriveApp: deleteNote(noteId)
end
RemoveDuplicateNotesAction-->>User: Return summary and unique notes
Search Notes ActionsequenceDiagram
participant User
participant SearchNotesAction
participant PipedriveApp
User->>SearchNotesAction: Run action with filters/search term
SearchNotesAction->>PipedriveApp: getNotes (with filters)
PipedriveApp-->>SearchNotesAction: Return notes
SearchNotesAction->>SearchNotesAction: Filter notes by search term (if provided)
SearchNotesAction-->>User: Return filtered notes and summary
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/pipedrive/actions/remove-duplicate-notes/remove-duplicate-notes.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 (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (3)
✨ 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (3)
components/pipedrive/actions/search-notes/search-notes.mjs (1)
136-167: Consider performance implications of client-side filtering.The current implementation fetches all notes matching the API filters and then applies the search term filter locally. For large datasets, this could be inefficient and may hit memory or timeout limits.
Consider implementing server-side search if the Pipedrive API supports it, or add pagination limits when using search terms to prevent excessive data retrieval.
components/pipedrive/actions/remove-duplicate-notes/remove-duplicate-notes.mjs (2)
129-131: Consider implementing parallel deletion for better performance.Sequential deletion of duplicates could be slow for large numbers of duplicates. Consider implementing parallel deletion with appropriate rate limiting.
- for (const note of duplicates) { - try { - await this.pipedriveApp.deleteNote(note.duplicate.id); - deletionResults.push({ id: note.duplicate.id, success: true }); - } catch (error) { - console.error(`Failed to delete note ${note.duplicate.id}:`, error); - deletionResults.push({ id: note.duplicate.id, success: false, error: error.message }); - } - } + const batchSize = 5; // Adjust based on API rate limits + const deletionResults = []; + + for (let i = 0; i < duplicates.length; i += batchSize) { + const batch = duplicates.slice(i, i + batchSize); + const batchPromises = batch.map(async (note) => { + try { + await this.pipedriveApp.deleteNote(note.duplicate.id); + return { id: note.duplicate.id, success: true }; + } catch (error) { + console.error(`Failed to delete note ${note.duplicate.id}:`, error); + return { id: note.duplicate.id, success: false, error: error.message }; + } + }); + + const batchResults = await Promise.all(batchPromises); + deletionResults.push(...batchResults); + }
3-8: Consider adding a dry-run mode for destructive operations.Since this action performs destructive operations (deleting notes), consider adding a dry-run mode that identifies duplicates without actually deleting them.
Add a
dryRunprop to allow users to preview what would be deleted:props: { pipedriveApp, + dryRun: { + type: "boolean", + label: "Dry Run", + description: "If true, identify duplicates without actually deleting them", + default: false, + optional: true, + }, leadId: {Then modify the deletion logic to conditionally skip actual deletion:
for (const note of duplicates) { - try { - await this.pipedriveApp.deleteNote(note.duplicate.id); - deletionResults.push({ id: note.duplicate.id, success: true }); - } catch (error) { - console.error(`Failed to delete note ${note.duplicate.id}:`, error); - deletionResults.push({ id: note.duplicate.id, success: false, error: error.message }); - } + if (!this.dryRun) { + try { + await this.pipedriveApp.deleteNote(note.duplicate.id); + deletionResults.push({ id: note.duplicate.id, success: true }); + } catch (error) { + console.error(`Failed to delete note ${note.duplicate.id}:`, error); + deletionResults.push({ id: note.duplicate.id, success: false, error: error.message }); + } + } else { + deletionResults.push({ id: note.duplicate.id, success: true, dryRun: true }); + } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (17)
components/pipedrive/actions/add-activity/add-activity.mjs(1 hunks)components/pipedrive/actions/add-deal/add-deal.mjs(1 hunks)components/pipedrive/actions/add-lead/add-lead.mjs(1 hunks)components/pipedrive/actions/add-note/add-note.mjs(1 hunks)components/pipedrive/actions/add-organization/add-organization.mjs(1 hunks)components/pipedrive/actions/add-person/add-person.mjs(1 hunks)components/pipedrive/actions/remove-duplicate-notes/remove-duplicate-notes.mjs(1 hunks)components/pipedrive/actions/search-notes/search-notes.mjs(1 hunks)components/pipedrive/actions/search-persons/search-persons.mjs(1 hunks)components/pipedrive/actions/update-deal/update-deal.mjs(1 hunks)components/pipedrive/actions/update-person/update-person.mjs(1 hunks)components/pipedrive/package.json(1 hunks)components/pipedrive/pipedrive.app.mjs(2 hunks)components/pipedrive/sources/new-deal-instant/new-deal-instant.mjs(1 hunks)components/pipedrive/sources/new-person-instant/new-person-instant.mjs(1 hunks)components/pipedrive/sources/updated-deal-instant/updated-deal-instant.mjs(1 hunks)components/pipedrive/sources/updated-person-instant/updated-person-instant.mjs(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Publish TypeScript components
- GitHub Check: Verify TypeScript components
- GitHub Check: pnpm publish
- GitHub Check: Lint Code Base
🔇 Additional comments (18)
components/pipedrive/pipedrive.app.mjs (4)
358-361: LGTM! Note retrieval method correctly implemented.The
getNotesmethod follows the established pattern used by other API methods in this module and correctly instantiates the NotesApi.
432-437: LGTM! Note deletion method correctly implemented.The
deleteNotemethod follows the established pattern and correctly passes the note ID as expected by the Pipedrive API.
464-471: LGTM! Pagination utility correctly implemented.The
getPaginatedResourcesmethod correctly collects all results from the paginate generator and follows JavaScript async iteration patterns properly.
438-463:✅ Verification successful
Verify pagination structure with Pipedrive API documentation.
The pagination logic assumes the response structure
additional_data.pagination.more_items_in_collection, but this should be verified against the Pipedrive API documentation to ensure consistency across different endpoints.
🌐 Web query:
What is the standard pagination response structure for Pipedrive API v1 and v2?💡 Result:
Pipedrive's API employs two primary pagination methods across its endpoints: cursor-based pagination and offset-based pagination. The choice between these methods depends on the specific API version and endpoint.
1. Cursor-Based Pagination:
This method is utilized in Pipedrive API v2 and certain v1 endpoints, such as
/v1/activities/collection,/v1/deals/collection,/v1/organizations/collection, and/v1/persons/collection. It offers efficient and stable performance when traversing large datasets.
Request Parameters:
cursor(string): An opaque string representing the first item on the next page.limit(integer): The number of items to return per page. Defaults to 100 if not specified; maximum is 500.Response Structure:
success(boolean): Indicates the success of the request.data(array): Contains the list of items for the current page.additional_data(object): Includes pagination details.
next_cursor(string|null): The cursor for the next page. Ifnull, there are no more pages.Example Response:
{ "success": true, "data": [ { // Item data } ], "additional_data": { "next_cursor": "eyJhY3Rpdml0eSI6NDJ9" } }2. Offset-Based Pagination:
This method is employed in Pipedrive API v1 for most list and item collection endpoints.
Request Parameters:
start(integer): The offset to start fetching items. Defaults to 0 if not specified.limit(integer): The number of items to return per page. Defaults to 100 if not specified; maximum is 500.Response Structure:
success(boolean): Indicates the success of the request.data(array): Contains the list of items for the current page.additional_data(object): Includes pagination details.
pagination(object):
start(integer): The offset used in the request.limit(integer): The limit used in the request.more_items_in_collection(boolean): Indicates if more items are available.next_start(integer): The offset for the next page.Example Response:
{ "success": true, "data": [ { // Item data } ], "additional_data": { "pagination": { "start": 0, "limit": 100, "more_items_in_collection": true, "next_start": 100 } } }Understanding these pagination structures is crucial for effectively navigating and retrieving data from Pipedrive's API.
Citations:
- 1: https://pipedrive.readme.io/docs/core-api-concepts-pagination?utm_source=openai
- 2: https://pipedrive.readme.io/docs/core-api-concepts-pagination?utm_source=openai
Offset-based pagination logic aligns with Pipedrive v1 API
The current implementation uses
params.start/params.limit- checks
additional_data.pagination.more_items_in_collectionwhich matches Pipedrive’s offset-based pagination (v1) response structure. No changes are required here unless you plan to consume cursor-based (v2) endpoints—those would use
additional_data.next_cursorand a different request parameter (cursor) instead.components/pipedrive/package.json (1)
3-3: LGTM! Appropriate version bump for new features.The minor version increment from 0.5.1 to 0.6.0 correctly follows semantic versioning for the addition of new note management functionality.
components/pipedrive/actions/add-activity/add-activity.mjs (1)
10-10: LGTM! Coordinated version bump for release consistency.The patch version increment aligns with the overall package version update and maintains consistency across the Pipedrive integration components.
components/pipedrive/actions/add-person/add-person.mjs (1)
9-9: LGTM! Coordinated version bump for release consistency.The patch version increment maintains consistency across the Pipedrive integration components and aligns with the package version update.
components/pipedrive/actions/update-deal/update-deal.mjs (1)
8-8: Align version bump with coordinated release.
The patch version has been correctly incremented from0.1.11to0.1.12in sync with other Pipedrive actions. No further logic changes are required.components/pipedrive/actions/add-deal/add-deal.mjs (1)
8-8: Approve version increment.
The action’s version has been updated from0.1.9to0.1.10to align with the coordinated release. No code or behavior changes detected.components/pipedrive/actions/add-lead/add-lead.mjs (1)
9-9: Approve version bump.
Updated the version from0.0.3to0.0.4to match the overarching Pipedrive integration release. Implementation remains unchanged.components/pipedrive/sources/new-deal-instant/new-deal-instant.mjs (1)
9-9: Approve source version update.
Bumped the source version from0.0.5to0.0.6in line with other Pipedrive components. No functional modifications needed.components/pipedrive/actions/add-note/add-note.mjs (1)
8-8: Approve version synchronization.
The version of the “Add Note” action has been correctly incremented from0.0.7to0.0.8alongside other coordinated Pipedrive updates. Behavior is unchanged.components/pipedrive/sources/updated-deal-instant/updated-deal-instant.mjs (1)
9-9: Version bump approved
Updated to "0.0.6" to align with the coordinated Pipedrive integration release.components/pipedrive/sources/new-person-instant/new-person-instant.mjs (1)
9-9: Version bump approved
Bumped to "0.0.6" in sync with other instant source components.components/pipedrive/actions/search-persons/search-persons.mjs (1)
10-10: Version bump approved
Action version updated to "0.1.10" consistent with the broader action updates.components/pipedrive/actions/add-organization/add-organization.mjs (1)
8-8: Version bump approved
Version incremented to "0.1.10" matching the coordinated release across Pipedrive actions.components/pipedrive/actions/update-person/update-person.mjs (1)
9-9: Version bump approved
Updated to "0.0.2" to keep parity with the minor version rollout.components/pipedrive/sources/updated-person-instant/updated-person-instant.mjs (1)
9-9: LGTM! Standard version bump.Version increment from "0.0.5" to "0.0.6" is properly implemented with no functional changes.
components/pipedrive/actions/remove-duplicate-notes/remove-duplicate-notes.mjs
Show resolved
Hide resolved
components/pipedrive/actions/remove-duplicate-notes/remove-duplicate-notes.mjs
Show resolved
Hide resolved
components/pipedrive/actions/remove-duplicate-notes/remove-duplicate-notes.mjs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
components/pipedrive/actions/remove-duplicate-notes/remove-duplicate-notes.mjs (2)
67-71:⚠️ Potential issueValidate
add_timebefore sorting to avoidNaNcomparisons
new Date(<invalid>)yieldsInvalid Date; subtracting two invalid dates returnsNaN, producing an unstable sort that can throw in V8 (Array.prototype.sortexpects the callback to return a finite number). Use a safe fallback whenadd_timeis missing or malformed.- const sortedNotes = notes.sort((a, b) => { - const dateA = new Date(a.add_time); - const dateB = new Date(b.add_time); - return dateA - dateB; - }); + const sortedNotes = notes.sort((a, b) => { + const dateA = new Date(a.add_time || 0); + const dateB = new Date(b.add_time || 0); + if (isNaN(dateA) || isNaN(dateB)) { + return 0; // keep original order for invalid dates + } + return dateA - dateB; + });
125-127: 🛠️ Refactor suggestionAdd per-deletion error handling so one failure doesn’t abort the run
A single 4xx/5xx response will currently reject the entire action, leaving subsequent duplicates undeleted. Wrap each deletion in
try/catch, gather results, and report partial success.- for (const note of duplicates) { - await this.pipedriveApp.deleteNote(note.duplicate.id); - } + const deletionResults = []; + for (const { duplicate } of duplicates) { + try { + await this.pipedriveApp.deleteNote(duplicate.id); + deletionResults.push({ id: duplicate.id, success: true }); + } catch (err) { + console.error(`Failed to delete note ${duplicate.id}`, err); + deletionResults.push({ id: duplicate.id, success: false, error: err.message }); + } + } + + const successfulDeletions = deletionResults.filter(r => r.success).length; + const failedDeletions = deletionResults.length - successfulDeletions; + + // update summary / result accordinglyUpdate the returned
resultand$summaryto reflectsuccessfulDeletionsvsfailedDeletions.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
components/pipedrive/actions/remove-duplicate-notes/remove-duplicate-notes.mjs(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: pnpm publish
- GitHub Check: Lint Code Base
- GitHub Check: Verify TypeScript components
- GitHub Check: Publish TypeScript components
🔇 Additional comments (1)
components/pipedrive/actions/remove-duplicate-notes/remove-duplicate-notes.mjs (1)
112-114: Null-safe keyword filtering looks goodUsing optional chaining on
note.contentprevents crashes on null/undefined notes.
components/pipedrive/actions/remove-duplicate-notes/remove-duplicate-notes.mjs
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
components/pipedrive/actions/remove-duplicate-notes/remove-duplicate-notes.mjs (3)
68-72: 🛠️ Refactor suggestion
add_timevalidation still missing – potentialNaNsort ordering
new Date(a.add_time)returnsInvalid Datewhenadd_timeis null/invalid, leading the comparison callback to returnNaN→ undefined ordering and hard-to-trace bugs.
The same issue was raised on a previous commit and remains unaddressed.-const sortedNotes = notes.sort((a, b) => { - const dateA = new Date(a.add_time); - const dateB = new Date(b.add_time); - return dateA - dateB; -}); +const sortedNotes = notes.sort((a, b) => { + const dateA = new Date(a.add_time || 0); + const dateB = new Date(b.add_time || 0); + if (isNaN(dateA) || isNaN(dateB)) return 0; // keep relative order + return dateA - dateB; +});
107-117: 🛠️ Refactor suggestionUndefined params still forwarded to the API
Keys with
undefinedvalues are still passed togetPaginatedResources, risking"undefined"string-serialization by the HTTP client. The earlier refactor suggestion to build theparamsobject dynamically (and to throw if all IDs are missing) hasn’t been applied.- params: { - user_id: this.userId, - lead_id: this.leadId, - deal_id: this.dealId, - person_id: this.personId, - org_id: this.organizationId, - project_id: this.projectId, - }, + params: Object.fromEntries( + Object.entries({ + user_id: this.userId, + lead_id: this.leadId, + deal_id: this.dealId, + person_id: this.personId, + org_id: this.organizationId, + project_id: this.projectId, + }).filter(([, v]) => v != null), + ),Consider throwing early if no ID is supplied.
133-135: 🛠️ Refactor suggestionSequential deletes without error handling
A single failure in
deleteNoteaborts the entire loop, leaving other duplicates undeleted. Add per-notetry / catchand track successes/failures, or usePromise.allSettledwith a throttle.-for (const note of duplicates) { - await this.pipedriveApp.deleteNote(note.duplicate.id); -} +const deletionResults = []; +for (const note of duplicates) { + try { + await this.pipedriveApp.deleteNote(note.duplicate.id); + deletionResults.push({ id: note.duplicate.id, success: true }); + } catch (err) { + console.error(`Failed to delete note ${note.duplicate.id}:`, err); + deletionResults.push({ id: note.duplicate.id, success: false, error: err.message }); + } +}Update the summary to reflect partial failures.
🧹 Nitpick comments (1)
components/pipedrive/actions/remove-duplicate-notes/remove-duplicate-notes.mjs (1)
124-128: Dead interimresultobject
resultis initialised (lines 124-128) and then immediately overwritten at line 137. Remove the first assignment to reduce noise.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (2)
components/pipedrive/actions/remove-duplicate-notes/remove-duplicate-notes.mjs(1 hunks)components/pipedrive/package.json(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- components/pipedrive/package.json
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Publish TypeScript components
- GitHub Check: Verify TypeScript components
- GitHub Check: Lint Code Base
- GitHub Check: pnpm publish
components/pipedrive/actions/remove-duplicate-notes/remove-duplicate-notes.mjs
Outdated
Show resolved
Hide resolved
|
/approve |
Resolves #17009
Summary by CodeRabbit
New Features
Improvements
Chores