Fix some missing query parameters on the API endpoints#103
Fix some missing query parameters on the API endpoints#103
Conversation
WalkthroughThis pull request updates several API endpoints and their supporting Tinybird data source functions by extracting the project identifier from the URL slug rather than the query string. Additionally, a new Changes
Sequence Diagram(s)sequenceDiagram
participant C as Client
participant A as API Endpoint
participant D as Data Source
participant T as Tinybird Service
C->>A: Send request (URL includes slug)
A->>A: Extract slug as project and set filter (project, granularity)
A->>D: Invoke data source function with filter object
D->>T: Query Tinybird with filter criteria
T-->>D: Return query results
D-->>A: Provide data with safe access (optional chaining)
A-->>C: Respond with filtered results
Possibly related PRs
Suggested reviewers
Tip ⚡🧪 Multi-step agentic review comment chat (experimental)
📜 Recent review detailsConfiguration used: CodeRabbit UI 📥 CommitsReviewing files that changed from the base of the PR and between ff8d0106764af41a6276244fafa94d9aad01a30a and 4ad66d11b60dc08fcdbc2492c23f8d32d79ddc97. 📒 Files selected for processing (9)
🚧 Files skipped from review as they are similar to previous changes (9)
🪧 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
frontend/server/data/tinybird/contributors-dependency-data-source.ts (1)
64-65:⚠️ Potential issuePotential division by zero risk.
Since
totalContributorCountmight be undefined or zero, there's a risk of calculating incorrect values forotherContributors.countandotherContributors.percentage. The missing fallback value fortotalContributorCountcould lead to unexpected results.Add defensive checks to ensure calculations don't produce NaN or negative values:
otherContributors: { - count: totalContributorCount - topContributorsCount, - percentage: 100 - topContributorsPercentage + count: Math.max(0, (totalContributorCount || 0) - topContributorsCount), + percentage: Math.max(0, 100 - topContributorsPercentage) },
🧹 Nitpick comments (3)
frontend/server/data/tinybird/contributors-leaderboard-source.ts (1)
64-64: Consider adding optional chaining for data.rowsAdd optional chaining when accessing
data.rowsto prevent potential runtime errors ifdatais undefined.- total: data.rows + total: data?.rows || 0frontend/server/data/tinybird/contributors-dependency-data-source.ts (1)
54-56: Good use of optional chaining for safer data access.The changes implement optional chaining (
?.) with fallback values to prevent potential runtime errors when accessing data properties that might be undefined.However, there's an inconsistency in the implementation. Line 56 doesn't provide a fallback value (
|| 0) unlike the other similar changes.- const totalContributorCount = tinybirdTopContributorsResponse.data[0]?.totalContributorCount; + const totalContributorCount = tinybirdTopContributorsResponse.data[0]?.totalContributorCount || 0;frontend/server/api/project/[slug]/contributors/organization-dependency.get.ts (1)
28-33: Update API documentation to match implementation.The API documentation comment still lists
projectas a query parameter, but the implementation now extracts it from the URL path. Consider updating the documentation to reflect this change./** * Query params: * - metric: 'all' | 'commits' | (see metrics options) - * - project: string * - repository: string * - time-period: string + * - granularity: string (optional, defaults to QUARTERLY) */
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📥 Commits
Reviewing files that changed from the base of the PR and between 0875920 and af963d026752a68b24690e331f654fd45169f1d8.
📒 Files selected for processing (9)
frontend/server/api/project/[slug]/contributors/contributor-dependency.get.ts(2 hunks)frontend/server/api/project/[slug]/contributors/contributor-leaderboard.get.ts(1 hunks)frontend/server/api/project/[slug]/contributors/organization-dependency.get.ts(2 hunks)frontend/server/api/project/[slug]/contributors/organization-leaderboard.get.ts(1 hunks)frontend/server/data/tinybird/contributors-dependency-data-source.ts(2 hunks)frontend/server/data/tinybird/contributors-leaderboard-source.ts(1 hunks)frontend/server/data/tinybird/organizations-dependency-data-source.ts(2 hunks)frontend/server/data/tinybird/organizations-leaderboard-source.ts(1 hunks)frontend/server/data/types.ts(2 hunks)
🔇 Additional comments (16)
frontend/server/api/project/[slug]/contributors/contributor-leaderboard.get.ts (2)
38-39: Good addition of project extraction from URL parametersExtracting the project slug from the URL parameters is a good approach as it makes the endpoint more robust by not relying on query parameters for this essential value.
41-41: LGTM: Project parameter added to filter objectThis change correctly adds the project parameter to the filter object, ensuring that the required project identifier is included in the request to TinyBird.
frontend/server/api/project/[slug]/contributors/organization-leaderboard.get.ts (2)
37-38: Good addition of project extraction from URL parametersSimilar to the contributor-leaderboard endpoint, extracting the project slug from the URL parameters is a consistent and reliable approach.
46-46: LGTM: Project parameter added to filter objectThis change correctly adds the project parameter to the filter object for the organization leaderboard, ensuring consistency with the contributor leaderboard endpoint.
frontend/server/data/tinybird/contributors-leaderboard-source.ts (1)
35-35: LGTM: Project parameter added to TinyBird queryThis change correctly passes the project parameter to the TinyBird query, ensuring that data is filtered by the appropriate project.
frontend/server/data/types.ts (2)
40-40: LGTM: Project property added to ContributorsLeaderboardFilterThis change correctly adds the required project property to the ContributorsLeaderboardFilter type, aligning it with other filter types in the file.
49-49: LGTM: Project property added to OrganizationsLeaderboardFilterThis change correctly adds the required project property to the OrganizationsLeaderboardFilter type, ensuring consistency with other filter types.
frontend/server/data/tinybird/organizations-leaderboard-source.ts (1)
35-40: Adding the project parameter ensures correct data retrieval.Including the project identifier in the Tinybird query now allows the API to filter data correctly. This change is aligned with the PR objective of fixing missing query parameters.
frontend/server/data/tinybird/organizations-dependency-data-source.ts (2)
40-47: Project parameter added correctly to the leaderboard filter.Including the project identifier in the filter object ensures data is filtered properly when fetching from Tinybird.
54-57: Good use of optional chaining for safer data access.The changes implement optional chaining (
?.) with fallback values to prevent potential runtime errors when accessing data properties that might be undefined. This is a defensive programming approach that makes the code more robust.frontend/server/api/project/[slug]/contributors/contributor-dependency.get.ts (2)
3-3: Appropriate import for the FilterGranularity enum.Adding the FilterGranularity import supports the new default granularity parameter in the filter object.
37-46: Improved parameter extraction and query building.These changes correctly:
- Extract the project identifier from the URL slug instead of relying on query parameters
- Add the project to the filter object
- Set a default granularity value using the FilterGranularity enum
This implementation aligns with the PR objective of ensuring all necessary parameters are included in API calls to Tinybird.
frontend/server/data/tinybird/contributors-dependency-data-source.ts (1)
39-46: Project parameter added correctly to the leaderboard filter.Including the project identifier in the filter object ensures data is filtered properly when fetching from Tinybird.
frontend/server/api/project/[slug]/contributors/organization-dependency.get.ts (3)
2-2: Good addition of FilterGranularity import.The addition of the
FilterGranularityimport aligns with the PR objective of properly supporting granularity parameters in the API endpoints. This typed enum provides better type safety compared to using string literals.
37-38: Good implementation of project extraction from URL.Extracting the project identifier from the URL slug rather than query parameters is a good change. This approach is more REST-compliant and makes the API more consistent.
40-41: Properly updated filter with required parameters.The changes appropriately add the required parameters to the filter object:
- The
projectproperty is now correctly set from the URL slug- The
granularityproperty has a sensible default value ofFilterGranularity.QUARTERLYThese changes directly address the PR objective of fixing the missing parameters needed for TinyBird data fetching.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
frontend/server/data/tinybird/organizations-leaderboard-source.ts (1)
29-41: Consider addressing the security TODO commentThere's an existing TODO comment about unchecked query parameters being passed to TinyBird. While this PR is focused on adding missing parameters, it might be worth considering how to address this security concern in a follow-up PR.
// Potential approach to validate parameters: const sanitizedQuery = { project: filter.project, // Ensure this is a valid project ID/slug repository: filter.repository ? sanitizeRepository(filter.repository) : undefined, startDate: filter.startDate, // DateTime objects are already validated endDate: filter.endDate, };frontend/server/data/tinybird/contributors-leaderboard-source.ts (1)
28-41: Consider addressing the security TODO commentSimilar to the organizations leaderboard file, there's an existing TODO comment about unchecked query parameters being passed to TinyBird. While this PR is focused on adding missing parameters, it might be worth considering how to address this security concern in a follow-up PR.
frontend/server/data/tinybird/contributors-dependency-data-source.ts (1)
55-56: Added optional chaining for safer property access.The optional chaining improves robustness when accessing properties, preventing potential null/undefined errors.
Consider adding a check for empty data array before accessing indexed elements:
- const topContributorsCount = tinybirdTopContributorsResponse.data.length; - const lastContributor = tinybirdTopContributorsResponse.data[topContributorsCount - 1]; - const topContributorsPercentage = lastContributor?.contributionPercentageRunningTotal || 0; - const totalContributorCount = tinybirdTopContributorsResponse.data[0]?.totalContributorCount; + const topContributorsCount = tinybirdTopContributorsResponse.data.length; + const lastContributor = topContributorsCount > 0 ? tinybirdTopContributorsResponse.data[topContributorsCount - 1] : null; + const topContributorsPercentage = lastContributor?.contributionPercentageRunningTotal || 0; + const totalContributorCount = tinybirdTopContributorsResponse.data[0]?.totalContributorCount || 0;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📥 Commits
Reviewing files that changed from the base of the PR and between 0875920 and af963d026752a68b24690e331f654fd45169f1d8.
📒 Files selected for processing (9)
frontend/server/api/project/[slug]/contributors/contributor-dependency.get.ts(2 hunks)frontend/server/api/project/[slug]/contributors/contributor-leaderboard.get.ts(1 hunks)frontend/server/api/project/[slug]/contributors/organization-dependency.get.ts(2 hunks)frontend/server/api/project/[slug]/contributors/organization-leaderboard.get.ts(1 hunks)frontend/server/data/tinybird/contributors-dependency-data-source.ts(2 hunks)frontend/server/data/tinybird/contributors-leaderboard-source.ts(1 hunks)frontend/server/data/tinybird/organizations-dependency-data-source.ts(2 hunks)frontend/server/data/tinybird/organizations-leaderboard-source.ts(1 hunks)frontend/server/data/types.ts(2 hunks)
🔇 Additional comments (17)
frontend/server/api/project/[slug]/contributors/contributor-leaderboard.get.ts (1)
38-41: Good implementation of project parameter extraction from URL pathThe change correctly extracts the project identifier from the URL slug parameter rather than relying on the query string. This approach is more RESTful since the endpoint is already project-specific based on its URL structure.
For slightly improved type safety, consider using Nuxt's typed context params:
- const project = (event.context.params as { slug: string }).slug; + const { slug: project } = event.context.params as { slug: string };frontend/server/api/project/[slug]/contributors/organization-leaderboard.get.ts (2)
37-38: Good implementation of project parameter extraction from URL pathThe change correctly extracts the project identifier from the URL slug parameter rather than relying on the query string. This matches the approach used in the contributor-leaderboard.get.ts file, ensuring consistency across endpoints.
For slightly improved type safety, consider using Nuxt's typed context params:
- const project = (event.context.params as { slug: string }).slug; + const { slug: project } = event.context.params as { slug: string };
45-47: Correctly added project parameter to filter objectThe addition of the project parameter to the filter object ensures that the query to TinyBird includes the necessary project context, aligning with the PR objective to fix missing query parameters.
frontend/server/data/tinybird/organizations-leaderboard-source.ts (1)
35-37: Good addition of project parameter to TinyBird queryThe change correctly adds the project parameter to the organizations leaderboard query, ensuring that TinyBird receives all required parameters to filter the data properly. This aligns with the PR objective to address missing query parameters.
frontend/server/data/tinybird/contributors-leaderboard-source.ts (2)
34-36: Good addition of project parameter to TinyBird queryThe change correctly adds the project parameter to the contributors leaderboard query, ensuring that TinyBird receives all required parameters to filter the data properly. This aligns with the PR objective to address missing query parameters.
60-67: Verify the hardcoded pagination valuesThe
metaobject contains hardcoded pagination values (offset: 0, limit: 10) which might not match the actual query parameters. Ensure these values are accurate or consider using the actual pagination values from the query:return { meta: { - offset: 0, - limit: 10, + offset: filter.offset || 0, + limit: filter.limit || 10, total: data.rows }, data: processedData };frontend/server/api/project/[slug]/contributors/contributor-dependency.get.ts (3)
3-3: Added necessary import for FilterGranularity.The addition of
FilterGranularityimport supports the new granularity filter parameter.
37-38: Extracting project from URL slug instead of query parameter.Good change - extracting the project from the URL slug makes the API more RESTful and consistent with path-based resource identification.
39-42: Added project and granularity to the filter object.The changes properly include the project from URL slug and add granularity with a sensible default (QUARTERLY), which should help with getting the right data from TinyBird.
The comment in lines 28-33 still mentions
projectas a query parameter, but it's now extracted from the URL path. Consider updating the comment to reflect this change:/** * Query params: * - metric: 'all' | 'commits' | (see metrics options) - * - project: string * - repository: string * - time-period: string + * - granularity: string (optional, defaults to 'quarterly') */frontend/server/data/tinybird/contributors-dependency-data-source.ts (1)
39-41: Added project to leaderboardFilter.Good addition of the project parameter to ensure consistent filtering across the application.
frontend/server/data/types.ts (2)
39-41: Added project field to ContributorsLeaderboardFilter type.Good addition of the required project field to ensure type safety and consistency with implementation.
48-50: Added project field to OrganizationsLeaderboardFilter type.Good addition of the required project field to ensure type safety and consistency with implementation.
frontend/server/api/project/[slug]/contributors/organization-dependency.get.ts (3)
2-2: Added necessary import for FilterGranularity.The addition of
FilterGranularityimport supports the new granularity filter parameter.
37-38: Extracting project from URL slug instead of query parameter.Good change - extracting the project from the URL slug makes the API more RESTful and consistent with path-based resource identification.
39-42: Added project and granularity to the filter object.The changes properly include the project from URL slug and add granularity with a sensible default (QUARTERLY), which should help with getting the right data from TinyBird.
The comment in lines 28-33 still mentions
projectas a query parameter, but it's now extracted from the URL path. Consider updating the comment to reflect this change:/** * Query params: * - metric: 'all' | 'commits' | (see metrics options) - * - project: string * - repository: string * - time-period: string + * - granularity: string (optional, defaults to 'quarterly') */frontend/server/data/tinybird/organizations-dependency-data-source.ts (2)
41-41: Added important project parameter to the leaderboard filter.This change addresses the main issue of missing query parameters mentioned in the PR objectives, ensuring that the project identifier is properly passed to the TinyBird data source for filtering organization data.
55-57: Improved error handling with optional chaining.The refactored code properly handles potential undefined values in the TinyBird response using optional chaining (
?.) and fallback values (|| 0). This defensive programming approach prevents potential runtime errors if the response data is empty or malformed.
borfast
force-pushed
the
bugfix/fix-dependency-endpoints-not-sending-project
branch
3 times, most recently
from
ff8d010 to
4ad66d1
Compare
borfast
force-pushed
the
bugfix/fix-dependency-endpoints-not-sending-project
branch
from
4ad66d1 to
d603be0
Compare
2 participants
Some endpoints were missing the project and the granularity parameters, and thus were not able to fetch data from TinyBird.
I also took the opportunity to add a few improvements in terms of data bounds checking.
Summary by CodeRabbit
New Features
Improvements