-
Notifications
You must be signed in to change notification settings - Fork 5.5k
[FIX] Lusha API Authentication Issue #18889
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 GitHub. 2 Skipped Deployments
|
WalkthroughThis pull request updates version metadata across multiple Lusha action modules, adjusts several actions to map single location inputs to Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant Paginate as paginate()
participant API as Lusha API
Note over Caller,Paginate: Old paginate signature
Caller->>Paginate: paginate({ fn, params={}, maxResults })
Paginate->>Paginate: payload.pages = params.pages
Paginate->>API: _makeRequest(payload)
API-->>Paginate: response
Paginate->>Paginate: iterate over response (data assumed in payload)
Paginate-->>Caller: accumulated results
Note over Caller,Paginate: New paginate signature
Caller->>Paginate: paginate({ fn, data={}, maxResults })
Paginate->>Paginate: payload.pages = data.pages
Paginate->>API: _makeRequest(payload)
API-->>Paginate: response
Paginate->>Paginate: results = response.data → iterate over results
Paginate-->>Caller: accumulated results
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (8)
🚧 Files skipped from review as they are similar to previous changes (5)
🧰 Additional context used🧠 Learnings (1)📚 Learning: 2024-09-25T16:13:11.505ZApplied to files:
🧬 Code graph analysis (2)components/lusha/actions/contact-search/contact-search.mjs (3)
components/lusha/lusha.app.mjs (4)
⏰ 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 (5)
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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
components/lusha/actions/search-and-enrich-contacts/search-and-enrich-contacts.mjs (1)
90-108: Critical: Pagination broken due to params vs data mismatch.The manual pagination loop passes pagination info via
params.pages(lines 95-100), but based on the refactoredpaginatemethod inlusha.app.mjs(line 272), pagination should now be indata.pages. The Lusha API likely expects pagination in the request body, not query parameters.Apply this diff to fix the pagination:
do { const { requestId, data = [], } = await this.lusha.searchContacts({ $, - params: { - pages: { - page, - size: 50, - }, - }, data: { + pages: { + page, + size: 50, + }, filters: { contacts: { include, }, }, }, });components/lusha/actions/search-and-enrich-companies/search-and-enrich-companies.mjs (1)
80-98: Critical: Pagination broken due to params vs data mismatch.The manual pagination loop passes pagination info via
params.pages(lines 85-90), but the refactoredpaginatemethod inlusha.app.mjs(line 272) now usesdata.pages. This inconsistency will cause pagination to fail.Apply this diff to fix the pagination:
do { const { requestId, data = [], } = await this.lusha.searchCompanies({ $, - params: { - pages: { - page, - size: 50, - }, - }, data: { + pages: { + page, + size: 50, + }, filters: { companies: { include, }, }, }, });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
components/lusha/actions/company-enrich/company-enrich.mjs(1 hunks)components/lusha/actions/company-search/company-search.mjs(1 hunks)components/lusha/actions/contact-enrich/contact-enrich.mjs(1 hunks)components/lusha/actions/contact-search/contact-search.mjs(1 hunks)components/lusha/actions/search-and-enrich-companies/search-and-enrich-companies.mjs(1 hunks)components/lusha/actions/search-and-enrich-contacts/search-and-enrich-contacts.mjs(1 hunks)components/lusha/lusha.app.mjs(2 hunks)components/lusha/package.json(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
components/lusha/lusha.app.mjs (4)
components/lusha/actions/company-enrich/company-enrich.mjs (1)
response(32-38)components/lusha/actions/company-search/company-search.mjs (1)
response(78-89)components/lusha/actions/contact-enrich/contact-enrich.mjs (1)
response(32-38)components/lusha/actions/contact-search/contact-search.mjs (1)
response(87-98)
⏰ 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: Publish TypeScript components
- GitHub Check: pnpm publish
- GitHub Check: Verify TypeScript components
- GitHub Check: Lint Code Base
🔇 Additional comments (1)
components/lusha/lusha.app.mjs (1)
182-182: Verify whether hardcoded debug mode is acceptable for production use and whether API key exposure is a known/acceptable trade-off.The Pipedream platform's debug feature intentionally exports the full request configuration (including the
Authorizationheader with the API key) tostep.debug_configwhen enabled. ThecloneSafe()function performs only a deep clone without filtering sensitive data.This hardcoding is atypical compared to how most components use the debug flag (primarily in source/webhook handlers). Since the original concern about API key exposure is technically valid, confirm whether this debug mode is:
- Required for troubleshooting in production
- An oversight that should be removed or made configurable
- An accepted security/observability trade-off for this integration
If debug is not essential for production use, consider removing it or gating it behind an environment variable or component setting.
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 @jcortes, LGTM! Ready for QA!
For Integration QA: |
|
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:
|
b0364a7 to
2d2a242
Compare
|
HI @vunguyenhung about: lusha - Company Search - FailedCompany search by name (Regression) - FailedImprovements neededReturn value should include the requestId and other metadata, for the user to use on other actions. AnswerThis is not a good idea because we are using pagination so we end up with a requestId per pagination lusha - Contact Search - FailedImprovement needed - Improve
|
WHY
Resolves #18861
Summary by CodeRabbit
Chores
Improvements