-
Notifications
You must be signed in to change notification settings - Fork 5.5k
[TRIGGER] LinkedIn Page / Profile Post #15799
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
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 3 Skipped Deployments
|
WalkthroughThis pull request updates multiple LinkedIn action modules by incrementing their version numbers without changing functionality. New constants and utility functions are introduced to streamline common operations. The LinkedIn app module is expanded with methods for listing posts and paginating resources. Additionally, a new polling module and a source component for new organization posts (along with a test event file) have been added. The package version and dependency on the platform have also been upgraded. Changes
Sequence Diagram(s)sequenceDiagram
participant Polling
participant DB as Database
participant App as LinkedIn App
Polling->>DB: Retrieve polling state (first run, lastDateAt)
Polling->>App: Call paginate() to fetch resources
App-->>Polling: Return fetched resources
Polling->>DB: Update lastDateAt if resources found
Polling->>App: Process each resource (reverse order)
sequenceDiagram
participant Source as New Org Post Source
participant API as LinkedIn API
participant App as LinkedIn App
participant Emitter
Source->>API: Deploy hook - search organization by vanity name
API-->>Source: Return organization info or error
Source->>App: Call listPosts/paginate() with organization ID
App-->>Source: Return posts data
Source->>Emitter: Emit event for new organization post
Suggested labels
Suggested reviewers
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
Scope: all 2 workspace projects ✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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: 2
🧹 Nitpick comments (3)
components/linkedin/linkedin.app.mjs (1)
298-298: Consider adding error handling for unexpected API response structures.The getNestedProperty utility is used to safely access nested properties, but consider adding explicit error handling for unexpected API response structures.
- const nextResources = utils.getNestedProperty(response, resourceName); + const nextResources = utils.getNestedProperty(response, resourceName); + if (nextResources === undefined) { + console.log(`Resource property '${resourceName}' not found in response`); + return; + }components/linkedin/common/utils.mjs (1)
33-38: Consider adding defaultEncodeFn to the exports.The defaultEncodeFn function is defined but not exported, which might be useful in some scenarios.
export default { iterate, getNestedProperty, encodeFn, + defaultEncodeFn, getParamsSerializer, };components/linkedin/sources/common/polling.mjs (1)
42-53: Template methods require implementation documentation.The methods that throw
ConfigurationErrorestablish a contract for child implementations but lack documentation about what each method should return.Consider adding JSDoc comments to these methods to document their purpose and expected return values. For example:
+ /** + * @returns {string} The name of the date field to use for incremental syncing + */ getDateField() { throw new ConfigurationError("getDateField is not implemented"); }, + /** + * @returns {string} The name of the resource being fetched (e.g., "posts") + */ getResourceName() { throw new ConfigurationError("getResourceName is not implemented"); },
📜 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 (24)
components/linkedin/actions/create-comment/create-comment.mjs(1 hunks)components/linkedin/actions/create-image-post-organization/create-image-post-organization.mjs(1 hunks)components/linkedin/actions/create-image-post-user/create-image-post-user.mjs(1 hunks)components/linkedin/actions/create-like-on-share/create-like-on-share.mjs(1 hunks)components/linkedin/actions/create-text-post-organization/create-text-post-organization.mjs(1 hunks)components/linkedin/actions/create-text-post-user/create-text-post-user.mjs(1 hunks)components/linkedin/actions/delete-post/delete-post.mjs(1 hunks)components/linkedin/actions/fetch-ad-account/fetch-ad-account.mjs(1 hunks)components/linkedin/actions/get-current-member-profile/get-current-member-profile.mjs(1 hunks)components/linkedin/actions/get-member-organization-access-control/get-member-organization-access-control.mjs(1 hunks)components/linkedin/actions/get-member-profile/get-member-profile.mjs(1 hunks)components/linkedin/actions/get-multiple-member-profiles/get-multiple-member-profiles.mjs(1 hunks)components/linkedin/actions/get-organization-access-control/get-organization-access-control.mjs(1 hunks)components/linkedin/actions/get-organization-administrators/get-organization-administrators.mjs(1 hunks)components/linkedin/actions/retrieve-comments-on-comments/retrieve-comments-on-comments.mjs(1 hunks)components/linkedin/actions/retrieve-comments-shares/retrieve-comments-shares.mjs(1 hunks)components/linkedin/actions/search-organization/search-organization.mjs(1 hunks)components/linkedin/common/constants.mjs(2 hunks)components/linkedin/common/utils.mjs(1 hunks)components/linkedin/linkedin.app.mjs(2 hunks)components/linkedin/package.json(1 hunks)components/linkedin/sources/common/polling.mjs(1 hunks)components/linkedin/sources/new-post-created/new-post-created.mjs(1 hunks)components/linkedin/sources/new-post-created/test-event.mjs(1 hunks)
✅ Files skipped from review due to trivial changes (19)
- components/linkedin/actions/delete-post/delete-post.mjs
- components/linkedin/package.json
- components/linkedin/actions/fetch-ad-account/fetch-ad-account.mjs
- components/linkedin/actions/create-comment/create-comment.mjs
- components/linkedin/actions/retrieve-comments-shares/retrieve-comments-shares.mjs
- components/linkedin/actions/get-organization-administrators/get-organization-administrators.mjs
- components/linkedin/actions/get-member-organization-access-control/get-member-organization-access-control.mjs
- components/linkedin/actions/get-current-member-profile/get-current-member-profile.mjs
- components/linkedin/actions/get-member-profile/get-member-profile.mjs
- components/linkedin/actions/create-like-on-share/create-like-on-share.mjs
- components/linkedin/actions/create-text-post-user/create-text-post-user.mjs
- components/linkedin/actions/create-text-post-organization/create-text-post-organization.mjs
- components/linkedin/actions/get-organization-access-control/get-organization-access-control.mjs
- components/linkedin/actions/create-image-post-user/create-image-post-user.mjs
- components/linkedin/actions/retrieve-comments-on-comments/retrieve-comments-on-comments.mjs
- components/linkedin/actions/create-image-post-organization/create-image-post-organization.mjs
- components/linkedin/actions/search-organization/search-organization.mjs
- components/linkedin/actions/get-multiple-member-profiles/get-multiple-member-profiles.mjs
- components/linkedin/sources/new-post-created/test-event.mjs
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: pnpm publish
- GitHub Check: Verify TypeScript components
- GitHub Check: Publish TypeScript components
🔇 Additional comments (27)
components/linkedin/common/constants.mjs (2)
78-81: Well-structured constants for pagination and state management.These new constants provide good configuration for pagination limits and state tracking. Using named constants instead of magic numbers improves code readability and maintainability.
93-96: Good job exporting all constants consistently.Adding the new constants to the exports object follows the established pattern in this file.
components/linkedin/sources/new-post-created/new-post-created.mjs (7)
1-3: Clear imports with good separation of concerns.Importing common functionality from a dedicated polling module demonstrates good modular design.
4-11: Good metadata structure with clear description.The component is well-defined with appropriate key, name, and descriptive text. Including the API documentation link in the description is excellent for developer reference.
12-20: Props are correctly defined and reuse common props.Good job reusing common props and properly defining the organizationId property using propDefinition.
21-32: Methods are well-structured and maintain single responsibility.Each method has a clear focus and follows good practices. The date field and resource name methods are simple and concise, while the getResourcesFn method properly refers to the app's listPosts method.
43-49: Metadata generation is concise and includes all required fields.The generateMeta method properly creates a metadata object with id, summary, and timestamp.
50-52: Version number correctly set for new component.Setting the initial version to 0.0.1 is appropriate for a new component.
33-42:❓ Verification inconclusive
Ensure query parameters are properly structured for the LinkedIn API.
The implementation looks good, but verify that the LinkedIn API properly accepts these exact parameter names and formats, especially for the
q: "author"parameter.
🌐 Web query:
LinkedIn API Posts query parameters documentation💡 Result:
To query LinkedIn posts via their API, several parameters and methods are available depending on the specific use case and resource type. Here’s a structured breakdown of key parameters and their applications:
Core Parameters for LinkedIn Posts API
Authentication and Base Requirements
- Access Token: Required for OAuth 2.0 authentication to authorize API requests[4].
- LinkedIn-Version Header: Specifies the API version (e.g.,
202304)[3].
Resource-Specific Parameters
1. Post Retrieval by URN
ugcPostUrnorshareUrn: Unique identifier for a post (e.g.,urn:li:ugcPost:12345)[3].viewContext: Determines the post version:
READER(default): Published, audience-facing version.AUTHOR: Includes unpublished drafts or edits[3].2. Batch Post Retrieval
ids: Comma-separated list of encoded URNs (e.g.,ids=List(urn%3Ali%3AugcPost%3A12345,urn%3Ali%3Ashare%3A67890))[3].X-RestLi-MethodHeader: Set toBATCH_GETfor batch requests[3].3. Post Search by Author
author: URN of the organization or user (e.g.,urn:li:organization:5515715)[3].sortBy: Sorts results byLAST_MODIFIED(default) orCREATED[3].- Pagination:
start: Index of the first result (default:0).count: Results per page (max:100)[3].
Search and Filter Parameters
General Search
keywords: Terms to search in posts or profiles[2][5].current_company/past_companies: Filter by company URNs[2].regions: Geo URNs to limit results by location[2].industries: Industry URNs for sector-specific filtering[2].Job Post Filters
job_type: Values likeF(full-time),C(contract)[2].experience: Levels like1(internship) to6(executive)[2].remote:1(onsite),2(remote),3(hybrid)[2].Profile Search
network_depths:F(1st-degree),S(2nd-degree),O(3rd+)[2].school-name: Filter by educational institutions (e.g.,school-name=Shermer%20High%20School)[1].
Content-Specific Parameters
Post Creation
author: Organization or user URN[3].commentary: Post text content[3].visibility:PUBLICor targeted distribution[3].content.article: Includessource,thumbnail,title, anddescriptionfor article posts[3].Engagement Metrics
fields: Request specific data likeengagementorreactionSummary[4].
Pagination and Limits
- Default Limits:
- LinkedIn returns 25 rows per API call for general searches[1].
- Maximum of 50 updates for picture posts via
type=PICT&count=50[1].- Adjustments: Use
startandcountto navigate large datasets[3].
Example Requests
Retrieve Posts by Author
GET https://api.linkedin.com/rest/posts?author=urn:li:organization:5515715&sortBy=CREATED&count=50 Headers: Authorization: Bearer {ACCESS_TOKEN} LinkedIn-Version: 202304Batch Fetch Posts
GET https://api.linkedin.com/rest/posts?ids=List(urn%3Ali%3AugcPost%3A12345,urn%3Ali%3Ashare%3A67890) Headers: X-RestLi-Method: BATCH_GET
By leveraging these parameters, developers can efficiently query LinkedIn posts while tailoring requests to specific needs such as filtering by author, content type, or engagement metrics[1][2][3][5].
Citations:
- 1: https://docs.informatica.com/data-integration/powerexchange-adapters-for-informatica/10-4-0/powerexchange-for-linkedin-user-guide/linkedin-data-objects/query-parameter-for-linkedin-resources.html
- 2: https://linkedin-api.readthedocs.io/en/latest/api.html
- 3: https://learn.microsoft.com/en-us/linkedin/marketing/community-management/shares/posts-api?view=li-lms-2025-02
- 4: https://www.unipile.com/how-to-get-linkedin-posts-using-api/
- 5: https://learn.microsoft.com/en-us/linkedin/shared/api-guide/concepts/methods
- 6: https://developer.unipile.com/docs/linkedin-search
Double-check the usage of the
qparameter.
The implementation correctly constructs theauthorandsortByfields. However, based on the LinkedIn Posts API documentation, sample requests for retrieving posts by author do not include aqparameter. Please verify whether the API actually requires or accepts a query parameter such asq: "author", and if not, consider removing it to align with the official specs.
- Verify if the LinkedIn API endpoint accepts
q: "author".- Ensure that the values for
author(formatted asurn:li:organization:<ID>) andsortBy: "CREATED"are correctly structured per the API documentation.components/linkedin/linkedin.app.mjs (5)
4-4: Good addition of utility module import.Importing the utility module makes the code more maintainable by centralizing common functions.
272-278: Well-implemented listPosts method with parameter serialization.The method properly uses the utility function for parameter serialization, which is essential for complex API parameters.
279-286: Well-structured method signature with good defaults.The getIterations method has a clear signature with appropriate default values using the constants.
287-327: Robust pagination implementation with appropriate logging.The pagination logic is well-implemented with proper handling of edge cases:
- Checks for empty resource arrays
- Properly filters resources based on date criteria
- Enforces maximum resource limits
- Increments page counters appropriately
- Includes helpful logging messages
329-331: Simple and effective pagination wrapper.The paginate method provides a clean interface by delegating to the iterate utility function.
components/linkedin/common/utils.mjs (4)
1-7: Well-implemented async iterator collector.The iterate function correctly collects items from an async iterator into an array, which is useful for converting paginated results into a single collection.
9-12: Safe nested property access with proper null handling.The getNestedProperty function safely accesses nested properties using optional chaining, which prevents errors when navigating through undefined properties.
14-20: Good separation between default and URL-safe encoding functions.Having separate functions for default and URL-safe encoding provides flexibility while maintaining a clear separation of concerns.
22-31: Flexible parameter serialization with customizable encoding.The getParamsSerializer function creates a parameter serializer with customizable encoding, which is a good abstraction for different API requirements.
components/linkedin/sources/common/polling.mjs (9)
1-7: LGTM! Clean import structure.The imports are well-organized, clearly separating platform components from app-specific modules.
8-20: Well-structured component with appropriate properties.The component follows Pipedream's best practices for defining source components with:
- App integration property
- Database for state management
- Timer configuration with sensible defaults and clear documentation
21-25: LGTM! Proper initialization in deploy hook.The deploy hook correctly initializes the component's state by setting the first run flag, which is essential for proper incremental polling behavior.
26-29: Template method pattern implemented correctly.The
generateMetamethod establishes a contract for subclasses to implement. This is an appropriate use of the template method pattern.
30-41: LGTM! Clean state management methods.The state management methods provide a clean API for storing and retrieving values from the database. They encapsulate the state logic well and make the code more maintainable.
54-57: LGTM! Well-implemented resource processing.The
processResourcemethod provides a good default implementation, generating metadata and emitting the resource.
102-105: LGTM! Resources are processed in chronological order.Processing resources in reverse order ensures they're handled chronologically if the API returns them in reverse chronological order (newest first), which is typical for social media APIs.
106-109: LGTM! First run flag is properly updated.The code correctly updates the first run flag after processing resources, which ensures subsequent runs use incremental fetching.
93-100:❓ Verification inconclusive
Verify lastDateAt is set to the most recent timestamp.
The code updates
lastDateAtusing the first resource from the array, but it's unclear if this represents the most recent timestamp, especially since resources are later processed in reverse order.
🏁 Script executed:
#!/bin/bash # Check how resources are ordered from the API # Look for any sort parameters in the methods that fetch resources echo "Checking for sort parameters in LinkedIn API calls..." rg -i "sort|order" components/linkedin/linkedin.app.mjs # Look for how resources are handled in other LinkedIn sources echo "Checking how other LinkedIn sources handle timestamps..." rg -i "lastDateAt|timestamp|sort|order" components/linkedin/sources/Length of output: 992
Please ensure that the timestamp value saved via
setLastDateAttruly represents the latest resource date.The current implementation extracts the first resource from the array and applies
setLastDateAt(firstResource[dateField]), but it isn’t clear if the API returns resources sorted so that the first element is the most recent. Notably, other parts of the code (e.g., innew-post-created/new-post-created.mjs) use a sort parameter (sortBy: "CREATED"), suggesting that proper ordering might be expected elsewhere. Please verify that the API or surrounding logic guarantees that the first resource has the most recent timestamp; otherwise, consider explicitly sorting or finding the resource with the maximum date to prevent potential issues with incremental updates.
b4922b9 to
c8697e9
Compare
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: 0
♻️ Duplicate comments (1)
components/linkedin/sources/common/polling.mjs (1)
71-122: 🛠️ Refactor suggestionAdd error handling when calling
app.paginate.
Atry/catchblock aroundapp.paginate(...)would provide a better user experience if the request fails or hits rate limits, preventing uncaught exceptions in the source.async run() { const { app, ... } = this; + try { const resources = await app.paginate({ resourcesFn: getResourcesFn(), resourcesFnArgs: getResourcesFnArgs(), resourceName: getResourceName(), ...otherArgs, }); // rest of the run logic ... + } catch (error) { + console.error("Error polling resources:", error); + throw error; + } }
🧹 Nitpick comments (2)
components/linkedin/linkedin.app.mjs (1)
279-328: Consider adding error handling for the pagination loop.
IfresourcesFnfails (e.g., network error, rate limit), the generator might break unexpectedly. Wrap the call with at least atry/catchor a check on the response to handle failures gracefully.async *getIterations({ resourcesFn, resourcesFnArgs, resourceName, lastDateAt, dateField, max = constants.DEFAULT_MAX, }) { let page = 0; let resourcesCount = 0; while (true) { + let response; + try { response = await resourcesFn({ ...resourcesFnArgs, params: { ...resourcesFnArgs?.params, count: constants.DEFAULT_LIMIT, start: page * constants.DEFAULT_LIMIT, }, }); + } catch (error) { + console.error("Failed to fetch next resources:", error); + return; + } // Remainder of method...components/linkedin/sources/new-organization-post-created/new-organization-post-created.mjs (1)
6-83: Minor spelling fix and handling multiple organization conflicts.
- There's a small spelling mistake in line 19: "Orgainization" → "Organization."
- If multiple organizations share the same vanity name, we currently pick the first match. Consider prompting the user or clarifying how you want to handle duplicates.
- description: "You can get the Orgainization Vanity Name ... + description: "You can get the Organization Vanity Name ...
📜 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 (26)
components/linkedin/actions/create-comment/create-comment.mjs(1 hunks)components/linkedin/actions/create-image-post-organization/create-image-post-organization.mjs(1 hunks)components/linkedin/actions/create-image-post-user/create-image-post-user.mjs(1 hunks)components/linkedin/actions/create-like-on-share/create-like-on-share.mjs(1 hunks)components/linkedin/actions/create-text-post-organization/create-text-post-organization.mjs(1 hunks)components/linkedin/actions/create-text-post-user/create-text-post-user.mjs(1 hunks)components/linkedin/actions/delete-post/delete-post.mjs(1 hunks)components/linkedin/actions/fetch-ad-account/fetch-ad-account.mjs(1 hunks)components/linkedin/actions/get-current-member-profile/get-current-member-profile.mjs(1 hunks)components/linkedin/actions/get-member-organization-access-control/get-member-organization-access-control.mjs(1 hunks)components/linkedin/actions/get-member-profile/get-member-profile.mjs(1 hunks)components/linkedin/actions/get-multiple-member-profiles/get-multiple-member-profiles.mjs(1 hunks)components/linkedin/actions/get-organization-access-control/get-organization-access-control.mjs(1 hunks)components/linkedin/actions/get-organization-administrators/get-organization-administrators.mjs(1 hunks)components/linkedin/actions/retrieve-comments-on-comments/retrieve-comments-on-comments.mjs(1 hunks)components/linkedin/actions/retrieve-comments-shares/retrieve-comments-shares.mjs(1 hunks)components/linkedin/actions/search-organization/search-organization.mjs(1 hunks)components/linkedin/common/constants.mjs(2 hunks)components/linkedin/common/utils.mjs(1 hunks)components/linkedin/linkedin.app.mjs(2 hunks)components/linkedin/package.json(2 hunks)components/linkedin/sources/common/polling.mjs(1 hunks)components/linkedin/sources/new-organization-post-created/new-organization-post-created.mjs(1 hunks)components/linkedin/sources/new-organization-post-created/test-event.mjs(1 hunks)components/linkedin/sources/new-post-created/new-post-created.mjs(1 hunks)components/linkedin/sources/new-post-created/test-event.mjs(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- components/linkedin/sources/new-organization-post-created/test-event.mjs
🚧 Files skipped from review as they are similar to previous changes (21)
- components/linkedin/actions/get-member-profile/get-member-profile.mjs
- components/linkedin/actions/get-current-member-profile/get-current-member-profile.mjs
- components/linkedin/actions/retrieve-comments-on-comments/retrieve-comments-on-comments.mjs
- components/linkedin/actions/get-multiple-member-profiles/get-multiple-member-profiles.mjs
- components/linkedin/actions/get-organization-access-control/get-organization-access-control.mjs
- components/linkedin/actions/delete-post/delete-post.mjs
- components/linkedin/actions/fetch-ad-account/fetch-ad-account.mjs
- components/linkedin/sources/new-post-created/test-event.mjs
- components/linkedin/actions/create-text-post-organization/create-text-post-organization.mjs
- components/linkedin/actions/create-comment/create-comment.mjs
- components/linkedin/actions/create-image-post-organization/create-image-post-organization.mjs
- components/linkedin/actions/get-organization-administrators/get-organization-administrators.mjs
- components/linkedin/actions/create-text-post-user/create-text-post-user.mjs
- components/linkedin/actions/create-like-on-share/create-like-on-share.mjs
- components/linkedin/actions/get-member-organization-access-control/get-member-organization-access-control.mjs
- components/linkedin/actions/retrieve-comments-shares/retrieve-comments-shares.mjs
- components/linkedin/actions/create-image-post-user/create-image-post-user.mjs
- components/linkedin/sources/new-post-created/new-post-created.mjs
- components/linkedin/actions/search-organization/search-organization.mjs
- components/linkedin/common/utils.mjs
- components/linkedin/common/constants.mjs
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: pnpm publish
- GitHub Check: Publish TypeScript components
- GitHub Check: Verify TypeScript components
🔇 Additional comments (7)
components/linkedin/package.json (2)
3-3: Version Update Acknowledged.
The version number has been correctly bumped from"0.1.1"to"0.2.0", aligning with the overall release strategy. Please ensure that any downstream documentation or changelogs are updated accordingly.
16-16: Dependency Update Verification Needed.
The dependency for@pipedream/platformhas been updated to"^3.0.3", which is a significant jump from the previous version. Please verify that this new version is fully compatible with the component’s API and does not introduce breaking changes in the integration with LinkedIn functionality.components/linkedin/linkedin.app.mjs (3)
4-4: Looks good importing the new utils module.
No issues identified.
272-278: Method implementation for listing posts appears correct.
TheparamsSerializerusage withencodeFnlooks appropriate._makeRequestpresumably handles errors.
329-331: Duplicate comment: Add error handling logic in thepaginatemethod.
As stated in a previous review, consider catching exceptions or surfacing failures fromgetIterationscalls, rather than relying on the parent context to handle them.components/linkedin/sources/new-organization-post-created/new-organization-post-created.mjs (1)
1-4: Module imports and base references look good.
No concerns with these import statements.components/linkedin/sources/common/polling.mjs (1)
1-70: Abstract methods throw explicit errors, promoting clarity.
Declaring placeholder methods that throwConfigurationErrorhelps ensure that subclasses must implement them. Well-structured for a base polling module.
c8697e9 to
dea3b0c
Compare
|
/approve |
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: 3
♻️ Duplicate comments (2)
components/linkedin/linkedin.app.mjs (1)
329-331: 🛠️ Refactor suggestionAdd error handling in the paginate method.
The
paginatemethod lacks error handling for API rate limits, timeouts, and other potential issues. The polling.mjs module depends on this method having robust error management.paginate(args = {}) { - return utils.iterate(this.getIterations(args)); + try { + return utils.iterate(this.getIterations(args)); + } catch (error) { + console.error("Error during pagination:", error); + throw error; + } },components/linkedin/sources/common/polling.mjs (1)
71-121: 🛠️ Refactor suggestionAdd error handling for required method calls in the run method.
The
runmethod destructures and calls several methods that could throwConfigurationErrorif not implemented by a child component, but there's no explicit error handling.async run() { const { app, getDateField, getLastDateAt, getResourcesFn, getResourcesFnArgs, getResourceName, getIsFirstRun, setIsFirstRun, setLastDateAt, processResource, } = this; + try { const isFirstRun = getIsFirstRun(); const dateField = getDateField(); const lastDateAt = getLastDateAt(); const otherArgs = isFirstRun ? { max: constants.DEFAULT_LIMIT, } : { dateField, lastDateAt, }; const resources = await app.paginate({ resourcesFn: getResourcesFn(), resourcesFnArgs: getResourcesFnArgs(), resourceName: getResourceName(), ...otherArgs, }); if (resources.length) { const [ firstResource, ] = Array.from(resources); if (firstResource) { setLastDateAt(firstResource[dateField]); } } Array.from(resources) .reverse() .forEach(processResource); if (isFirstRun) { setIsFirstRun(false); } + } catch (error) { + if (error instanceof ConfigurationError) { + throw new ConfigurationError(`Configuration error in ${this.constructor.name}: ${error.message}`); + } + throw error; + } },
🧹 Nitpick comments (1)
components/linkedin/sources/common/polling.mjs (1)
114-117: Consider handling large resource volumes more efficiently.The current implementation loads all resources into memory with
Array.from(resources), which could cause memory issues with large datasets. Sinceresourcesis likely an iterable from the paginate method, consider processing each item as it's received.- Array.from(resources) - .reverse() - .forEach(processResource); + // Store resources in memory only if we need to reverse them + const resourcesArray = []; + for (const resource of resources) { + resourcesArray.push(resource); + } + + // Process in reverse order + resourcesArray.reverse().forEach(processResource);Alternatively, if order doesn't matter:
- Array.from(resources) - .reverse() - .forEach(processResource); + // Process resources as they come + for (const resource of resources) { + processResource(resource); + }
📜 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 (24)
components/linkedin/actions/create-comment/create-comment.mjs(1 hunks)components/linkedin/actions/create-image-post-organization/create-image-post-organization.mjs(1 hunks)components/linkedin/actions/create-image-post-user/create-image-post-user.mjs(1 hunks)components/linkedin/actions/create-like-on-share/create-like-on-share.mjs(1 hunks)components/linkedin/actions/create-text-post-organization/create-text-post-organization.mjs(1 hunks)components/linkedin/actions/create-text-post-user/create-text-post-user.mjs(1 hunks)components/linkedin/actions/delete-post/delete-post.mjs(1 hunks)components/linkedin/actions/fetch-ad-account/fetch-ad-account.mjs(1 hunks)components/linkedin/actions/get-current-member-profile/get-current-member-profile.mjs(1 hunks)components/linkedin/actions/get-member-organization-access-control/get-member-organization-access-control.mjs(1 hunks)components/linkedin/actions/get-member-profile/get-member-profile.mjs(1 hunks)components/linkedin/actions/get-multiple-member-profiles/get-multiple-member-profiles.mjs(1 hunks)components/linkedin/actions/get-organization-access-control/get-organization-access-control.mjs(1 hunks)components/linkedin/actions/get-organization-administrators/get-organization-administrators.mjs(1 hunks)components/linkedin/actions/retrieve-comments-on-comments/retrieve-comments-on-comments.mjs(1 hunks)components/linkedin/actions/retrieve-comments-shares/retrieve-comments-shares.mjs(1 hunks)components/linkedin/actions/search-organization/search-organization.mjs(1 hunks)components/linkedin/common/constants.mjs(2 hunks)components/linkedin/common/utils.mjs(1 hunks)components/linkedin/linkedin.app.mjs(2 hunks)components/linkedin/package.json(2 hunks)components/linkedin/sources/common/polling.mjs(1 hunks)components/linkedin/sources/new-organization-post-created/new-organization-post-created.mjs(1 hunks)components/linkedin/sources/new-organization-post-created/test-event.mjs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (22)
- components/linkedin/actions/create-image-post-user/create-image-post-user.mjs
- components/linkedin/actions/fetch-ad-account/fetch-ad-account.mjs
- components/linkedin/actions/retrieve-comments-shares/retrieve-comments-shares.mjs
- components/linkedin/actions/create-like-on-share/create-like-on-share.mjs
- components/linkedin/actions/get-current-member-profile/get-current-member-profile.mjs
- components/linkedin/actions/retrieve-comments-on-comments/retrieve-comments-on-comments.mjs
- components/linkedin/actions/delete-post/delete-post.mjs
- components/linkedin/actions/get-organization-access-control/get-organization-access-control.mjs
- components/linkedin/actions/create-comment/create-comment.mjs
- components/linkedin/actions/get-multiple-member-profiles/get-multiple-member-profiles.mjs
- components/linkedin/actions/create-text-post-organization/create-text-post-organization.mjs
- components/linkedin/actions/create-text-post-user/create-text-post-user.mjs
- components/linkedin/actions/create-image-post-organization/create-image-post-organization.mjs
- components/linkedin/package.json
- components/linkedin/actions/get-member-organization-access-control/get-member-organization-access-control.mjs
- components/linkedin/actions/search-organization/search-organization.mjs
- components/linkedin/actions/get-organization-administrators/get-organization-administrators.mjs
- components/linkedin/actions/get-member-profile/get-member-profile.mjs
- components/linkedin/sources/new-organization-post-created/test-event.mjs
- components/linkedin/common/utils.mjs
- components/linkedin/common/constants.mjs
- components/linkedin/sources/new-organization-post-created/new-organization-post-created.mjs
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Verify TypeScript components
- GitHub Check: Publish TypeScript components
- GitHub Check: pnpm publish
- GitHub Check: Lint Code Base
🔇 Additional comments (1)
components/linkedin/linkedin.app.mjs (1)
272-278: LGTM: NewlistPostsmethod looks well-implemented.The implementation follows the pattern used by other API methods in this file and correctly leverages the utility function for parameter serialization.
| async *getIterations({ | ||
| resourcesFn, resourcesFnArgs, resourceName, | ||
| lastDateAt, dateField, | ||
| max = constants.DEFAULT_MAX, | ||
| }) { | ||
| let page = 0; | ||
| let resourcesCount = 0; | ||
|
|
||
| while (true) { | ||
| const response = | ||
| await resourcesFn({ | ||
| ...resourcesFnArgs, | ||
| params: { | ||
| ...resourcesFnArgs?.params, | ||
| count: constants.DEFAULT_LIMIT, | ||
| start: page * constants.DEFAULT_LIMIT, | ||
| }, | ||
| }); | ||
|
|
||
| const nextResources = utils.getNestedProperty(response, resourceName); | ||
|
|
||
| if (!nextResources?.length) { | ||
| console.log("No more resources found"); | ||
| return; | ||
| } | ||
|
|
||
| for (const resource of nextResources) { | ||
| const isDateGreater = | ||
| lastDateAt | ||
| && resource[dateField] > lastDateAt; | ||
|
|
||
| if (!lastDateAt || isDateGreater) { | ||
| yield resource; | ||
| resourcesCount += 1; | ||
| } | ||
|
|
||
| if (resourcesCount >= max) { | ||
| console.log("Reached max resources"); | ||
| return; | ||
| } | ||
| } | ||
|
|
||
| if (nextResources.length < constants.DEFAULT_LIMIT) { | ||
| console.log("No next page found"); | ||
| return; | ||
| } | ||
|
|
||
| page += 1; | ||
| } | ||
| }, |
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.
🛠️ Refactor suggestion
Add error handling to the getIterations method.
While the pagination logic is well-implemented with proper termination conditions, the method lacks error handling for API calls. If the resourcesFn call fails (e.g., network issues, rate limiting), the error will propagate without any specific handling.
async *getIterations({
resourcesFn, resourcesFnArgs, resourceName,
lastDateAt, dateField,
max = constants.DEFAULT_MAX,
}) {
let page = 0;
let resourcesCount = 0;
while (true) {
+ try {
const response =
await resourcesFn({
...resourcesFnArgs,
params: {
...resourcesFnArgs?.params,
count: constants.DEFAULT_LIMIT,
start: page * constants.DEFAULT_LIMIT,
},
});
const nextResources = utils.getNestedProperty(response, resourceName);
if (!nextResources?.length) {
console.log("No more resources found");
return;
}
for (const resource of nextResources) {
const isDateGreater =
lastDateAt
&& resource[dateField] > lastDateAt;
if (!lastDateAt || isDateGreater) {
yield resource;
resourcesCount += 1;
}
if (resourcesCount >= max) {
console.log("Reached max resources");
return;
}
}
if (nextResources.length < constants.DEFAULT_LIMIT) {
console.log("No next page found");
return;
}
page += 1;
+ } catch (error) {
+ console.error("Error fetching resources:", error);
+ throw error;
+ }
}
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| async *getIterations({ | |
| resourcesFn, resourcesFnArgs, resourceName, | |
| lastDateAt, dateField, | |
| max = constants.DEFAULT_MAX, | |
| }) { | |
| let page = 0; | |
| let resourcesCount = 0; | |
| while (true) { | |
| const response = | |
| await resourcesFn({ | |
| ...resourcesFnArgs, | |
| params: { | |
| ...resourcesFnArgs?.params, | |
| count: constants.DEFAULT_LIMIT, | |
| start: page * constants.DEFAULT_LIMIT, | |
| }, | |
| }); | |
| const nextResources = utils.getNestedProperty(response, resourceName); | |
| if (!nextResources?.length) { | |
| console.log("No more resources found"); | |
| return; | |
| } | |
| for (const resource of nextResources) { | |
| const isDateGreater = | |
| lastDateAt | |
| && resource[dateField] > lastDateAt; | |
| if (!lastDateAt || isDateGreater) { | |
| yield resource; | |
| resourcesCount += 1; | |
| } | |
| if (resourcesCount >= max) { | |
| console.log("Reached max resources"); | |
| return; | |
| } | |
| } | |
| if (nextResources.length < constants.DEFAULT_LIMIT) { | |
| console.log("No next page found"); | |
| return; | |
| } | |
| page += 1; | |
| } | |
| }, | |
| async *getIterations({ | |
| resourcesFn, resourcesFnArgs, resourceName, | |
| lastDateAt, dateField, | |
| max = constants.DEFAULT_MAX, | |
| }) { | |
| let page = 0; | |
| let resourcesCount = 0; | |
| while (true) { | |
| try { | |
| const response = | |
| await resourcesFn({ | |
| ...resourcesFnArgs, | |
| params: { | |
| ...resourcesFnArgs?.params, | |
| count: constants.DEFAULT_LIMIT, | |
| start: page * constants.DEFAULT_LIMIT, | |
| }, | |
| }); | |
| const nextResources = utils.getNestedProperty(response, resourceName); | |
| if (!nextResources?.length) { | |
| console.log("No more resources found"); | |
| return; | |
| } | |
| for (const resource of nextResources) { | |
| const isDateGreater = | |
| lastDateAt | |
| && resource[dateField] > lastDateAt; | |
| if (!lastDateAt || isDateGreater) { | |
| yield resource; | |
| resourcesCount += 1; | |
| } | |
| if (resourcesCount >= max) { | |
| console.log("Reached max resources"); | |
| return; | |
| } | |
| } | |
| if (nextResources.length < constants.DEFAULT_LIMIT) { | |
| console.log("No next page found"); | |
| return; | |
| } | |
| page += 1; | |
| } catch (error) { | |
| console.error("Error fetching resources:", error); | |
| throw error; | |
| } | |
| } | |
| }, |
| const resources = await app.paginate({ | ||
| resourcesFn: getResourcesFn(), | ||
| resourcesFnArgs: getResourcesFnArgs(), | ||
| resourceName: getResourceName(), | ||
| ...otherArgs, | ||
| }); |
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.
🛠️ Refactor suggestion
Handle API errors in the paginate call.
The call to app.paginate might fail due to API rate limits, timeouts, or other network issues, but there's no error handling for these scenarios.
- const resources = await app.paginate({
- resourcesFn: getResourcesFn(),
- resourcesFnArgs: getResourcesFnArgs(),
- resourceName: getResourceName(),
- ...otherArgs,
- });
+ let resources = [];
+ try {
+ resources = await app.paginate({
+ resourcesFn: getResourcesFn(),
+ resourcesFnArgs: getResourcesFnArgs(),
+ resourceName: getResourceName(),
+ ...otherArgs,
+ });
+ } catch (error) {
+ console.error("Error during pagination:", error);
+ // Either continue with empty resources or re-throw the error
+ // throw error;
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const resources = await app.paginate({ | |
| resourcesFn: getResourcesFn(), | |
| resourcesFnArgs: getResourcesFnArgs(), | |
| resourceName: getResourceName(), | |
| ...otherArgs, | |
| }); | |
| let resources = []; | |
| try { | |
| resources = await app.paginate({ | |
| resourcesFn: getResourcesFn(), | |
| resourcesFnArgs: getResourcesFnArgs(), | |
| resourceName: getResourceName(), | |
| ...otherArgs, | |
| }); | |
| } catch (error) { | |
| console.error("Error during pagination:", error); | |
| // Either continue with empty resources or re-throw the error | |
| // throw error; | |
| } |
| const meta = this.generateMeta(resource); | ||
| this.$emit(resource, meta); | ||
| }, |
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.
🛠️ Refactor suggestion
Add error handling in processResource method.
The processResource method calls generateMeta, which could throw a ConfigurationError if not implemented by a child component, but there's no error handling.
processResource(resource) {
- const meta = this.generateMeta(resource);
- this.$emit(resource, meta);
+ try {
+ const meta = this.generateMeta(resource);
+ this.$emit(resource, meta);
+ } catch (error) {
+ if (error instanceof ConfigurationError) {
+ console.error(`Error processing resource: ${error.message}`);
+ throw error;
+ }
+ throw error;
+ }
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const meta = this.generateMeta(resource); | |
| this.$emit(resource, meta); | |
| }, | |
| processResource(resource) { | |
| try { | |
| const meta = this.generateMeta(resource); | |
| this.$emit(resource, meta); | |
| } catch (error) { | |
| if (error instanceof ConfigurationError) { | |
| console.error(`Error processing resource: ${error.message}`); | |
| throw error; | |
| } | |
| throw error; | |
| } | |
| }, |
WHY
Resolves #15680
Summary by CodeRabbit
New Features
Chores