Skip to content

Conversation

@jcortes
Copy link
Collaborator

@jcortes jcortes commented Oct 21, 2025

WHY

Resolves #18686

Summary by CodeRabbit

  • New Features

    • Adds an informational alert in the report creation UI prompting users to select at least one facet.
    • Improved request header support for LinkedIn API compatibility and protocol handling.
  • Chores

    • Updated core platform dependency to a newer compatible release.
    • Incremented component and action versions across LinkedIn Ads sources and actions for consistency.

@jcortes jcortes self-assigned this Oct 21, 2025
@vercel
Copy link

vercel bot commented Oct 21, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

2 Skipped Deployments
Project Deployment Preview Comments Updated (UTC)
pipedream-docs Ignored Ignored Oct 21, 2025 9:42pm
pipedream-docs-redirect-do-not-edit Ignored Ignored Oct 21, 2025 9:42pm

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 21, 2025

Walkthrough

Bumped versions across LinkedIn Ads actions and sources, added a constants module exporting LinkedIn header values, introduced a new _getHeaders() method in the app to build API headers, and updated package metadata. One action also gained a new infoAlert prop.

Changes

Cohort / File(s) Summary
Action version bumps
components/linkedin_ads/actions/create-report-by-advertiser-account/create-report-by-advertiser-account.mjs, components/linkedin_ads/actions/create-report-by-campaign/create-report-by-campaign.mjs
version updated from "0.0.6" to "0.0.7".
Create report action (prop + version)
components/linkedin_ads/actions/create-report/create-report.mjs
version updated from "0.0.6" to "0.0.7"; added infoAlert prop (an informational alert prop defined before spreading common.props).
Send conversion event action
components/linkedin_ads/actions/send-conversion-event/send-conversion-event.mjs
version updated from "0.0.5" to "0.0.6".
Source version bumps
components/linkedin_ads/sources/new-event-registration-form-response/new-event-registration-form-response.mjs, components/linkedin_ads/sources/new-lead-gen-form-created/new-lead-gen-form-created.mjs
version updated from "0.0.4""0.0.5" and "0.0.3""0.0.4" respectively.
New constants module
components/linkedin_ads/common/constants.mjs
New module exporting default object with VERSION_HEADER: "202509" and RESTLI_PROTOCOL_VERSION: "2.0.0".
App header method and import
components/linkedin_ads/linkedin_ads.app.mjs
Added _getHeaders() method that returns headers (Authorization, Content-Type, Linkedin-Version, X-Restli-Protocol-Version); imports constants module to supply version values.
Package metadata
components/linkedin_ads/package.json
Package version bumped 0.3.20.3.3; dependency @pipedream/platform updated ^3.0.3^3.1.0.

Sequence Diagram(s)

sequenceDiagram
  participant App as LinkedIn Ads App
  participant Const as constants.mjs
  participant API as LinkedIn API

  Note over Const,App: New constants export
  App->>Const: import VERSION_HEADER, RESTLI_PROTOCOL_VERSION
  App->>App: _getHeaders(authToken)
  Note right of App: builds headers:\nAuthorization, Content-Type,\nLinkedin-Version, X-Restli-Protocol-Version
  App->>API: HTTP request with headers
  API-->>App: response
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐇 I hopped through code today, so spry,

Bumped the versions, gave headers wings to fly.
A constant here, an alert that gleams,
Requests now wear neat, versioned beams.
Happy hops — deploy and watch them sky!

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Description Check ❓ Inconclusive The PR description follows the template structure by including the required "## WHY" section; however, the content is extremely minimal, consisting only of "Resolves #18686" without explaining the actual rationale behind the change. While the description is not off-topic or entirely blank, the minimal explanation lacks sufficient detail to fully convey the purpose of the change, leaving the description in a vague state that doesn't conclusively demonstrate adequate coverage of the template requirements.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The PR title "[FIX] Set restli protocol version header back to 2.0" directly addresses the main objective of this pull request as stated in the PR description. The raw summary confirms that the changeset includes the addition of a new constants module with RESTLI_PROTOCOL_VERSION set to "2.0.0" and a new _getHeaders() method in linkedin_ads.app.mjs that constructs request headers including X-Restli-Protocol-Version. While the changeset also includes secondary changes such as version bumps across multiple files and a new infoAlert prop, the title accurately captures the primary change and intent.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch linkedin-restli-protocol-ver-20

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 428842e and 76ffc39.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (9)
  • components/linkedin_ads/actions/create-report-by-advertiser-account/create-report-by-advertiser-account.mjs (1 hunks)
  • components/linkedin_ads/actions/create-report-by-campaign/create-report-by-campaign.mjs (1 hunks)
  • components/linkedin_ads/actions/create-report/create-report.mjs (1 hunks)
  • components/linkedin_ads/actions/send-conversion-event/send-conversion-event.mjs (1 hunks)
  • components/linkedin_ads/common/constants.mjs (1 hunks)
  • components/linkedin_ads/linkedin_ads.app.mjs (2 hunks)
  • components/linkedin_ads/package.json (2 hunks)
  • components/linkedin_ads/sources/new-event-registration-form-response/new-event-registration-form-response.mjs (1 hunks)
  • components/linkedin_ads/sources/new-lead-gen-form-created/new-lead-gen-form-created.mjs (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
  • components/linkedin_ads/common/constants.mjs
  • components/linkedin_ads/sources/new-lead-gen-form-created/new-lead-gen-form-created.mjs
  • components/linkedin_ads/actions/create-report-by-advertiser-account/create-report-by-advertiser-account.mjs
  • components/linkedin_ads/actions/create-report-by-campaign/create-report-by-campaign.mjs
⏰ 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: Publish TypeScript components
  • GitHub Check: pnpm publish
  • GitHub Check: Verify TypeScript components
🔇 Additional comments (8)
components/linkedin_ads/sources/new-event-registration-form-response/new-event-registration-form-response.mjs (1)

8-8: Version bump looks good.

The version increment aligns with the package release. No functional changes detected.

components/linkedin_ads/package.json (2)

3-3: Package version bump is appropriate.

The version increment from 0.3.2 to 0.3.3 aligns with the component updates across the PR.


17-17: Platform dependency upgrade looks reasonable.

The @pipedream/platform upgrade from ^3.0.3 to ^3.1.0 is a minor version bump within the caret range, which should maintain backward compatibility.

components/linkedin_ads/actions/send-conversion-event/send-conversion-event.mjs (1)

8-8: Version bump looks good.

The version increment aligns with the package release. No functional changes detected.

components/linkedin_ads/actions/create-report/create-report.mjs (2)

9-9: Version bump looks good.

The version increment from 0.0.6 to 0.0.7 aligns with the package release.


17-22: Helpful user guidance added.

The new infoAlert prop provides clear guidance to users about providing at least one facet. The eslint-disable comment is appropriate since alert-type props don't require label/description properties.

components/linkedin_ads/linkedin_ads.app.mjs (2)

3-3: Constants module verified and properly configured.

The constants module exists at components/linkedin_ads/common/constants.mjs and correctly exports both VERSION_HEADER ("202509") and RESTLI_PROTOCOL_VERSION ("2.0.0") values via default export. The import statement in line 3 is valid.


155-162: Verify parent class implementation in @pipedream/linkedin or confirm method override pattern with maintainers.

The review comment raises a valid concern that cannot be fully resolved through codebase inspection alone. Here's why:

  • The new _getHeaders() method is being added to linkedin_ads.app.mjs, which inherits from @pipedream/linkedin (an external npm package).
  • The parent implementation is not accessible in this repository, so we cannot directly verify whether its _makeRequest() method invokes this._getHeaders().
  • While the codebase shows many similar components (e.g., bloom_growth, zoom, zoho_meeting) that successfully override _getHeaders() and have their parent's _makeRequest() call it, this pattern alone doesn't guarantee the same behavior in @pipedream/linkedin.

To proceed, either:

  1. Check the @pipedream/linkedin package source code (available on npm or GitHub).
  2. Confirm with maintainers that @pipedream/linkedin's _makeRequest() invokes method overrides via this._getHeaders().
  3. Add a comment referencing the established pattern or runtime test.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@jcortes jcortes force-pushed the linkedin-restli-protocol-ver-20 branch from a54b844 to 428842e Compare October 21, 2025 21:29
Copy link
Collaborator

@michelle0927 michelle0927 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@vunguyenhung vunguyenhung merged commit bfecd93 into master Oct 23, 2025
10 checks passed
@vunguyenhung vunguyenhung deleted the linkedin-restli-protocol-ver-20 branch October 23, 2025 07:14
Lokeshchand33 pushed a commit to Lokeshchand33/pipedream that referenced this pull request Oct 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

LinkedIn Ads Integration

3 participants