chore: Move more integration GraphQL types to SDL #10015
Conversation
There were some type resolver issues caused by #10000 and converting more types to SDL was the easiest way to fix the issues caused by not being able to resolve some interface types.
WalkthroughThe recent changes enhance the application by introducing new GraphQL mutations for managing team member integration authentication, specifically for adding and removing integrations. This includes updated type definitions for both Jira projects and the handling of integration credentials. The modifications streamline the authentication process and improve the GraphQL API's overall structure, facilitating better integration with various services and ensuring greater type safety. Changes
Recent review detailsConfiguration used: CodeRabbit UI Files selected for processing (1)
Additional comments not posted (2)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 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 as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
|
This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR will be delayed and might be rejected due to its size. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Outside diff range, codebase verification and nitpick comments (10)
packages/server/graphql/public/mutations/removeTeamMemberIntegrationAuth.ts (3)
1-6: Consider consolidating imports for better readability.Grouping similar imports together can enhance readability and maintainability.
import {IntegrationProviderServiceEnum} from '../../../postgres/queries/generated/removeTeamMemberIntegrationAuthQuery' import removeTeamMemberIntegrationAuthQuery from '../../../postgres/queries/removeTeamMemberIntegrationAuth' import {analytics} from '../../../utils/analytics/analytics' import {getUserId, isTeamMember} from '../../../utils/authorization' import standardError from '../../../utils/standardError' import updateRepoIntegrationsCacheByPerms from '../../queries/helpers/updateRepoIntegrationsCacheByPerms'
15-16: Enhance error message for clarity.The error message could be more descriptive to help with debugging and user feedback.
- return standardError(new Error('permission denied; must be team member')) + return standardError(new Error('Permission denied: User must be a team member to remove integration authentication.'))
30-31: Consider returning more detailed data.Returning more detailed data can be beneficial for debugging and client-side handling.
const data = {userId: viewerId, teamId, service} return datapackages/server/graphql/public/mutations/addTeamMemberIntegrationAuth.ts (5)
1-13: Consider consolidating imports for better readability.Grouping similar imports together can enhance readability and maintainability.
import IntegrationProviderId from '~/shared/gqlIds/IntegrationProviderId' import GcalOAuth2Manager from '../../../integrations/gcal/GcalOAuth2Manager' import GitLabOAuth2Manager from '../../../integrations/gitlab/GitLabOAuth2Manager' import JiraServerOAuth1Manager, {OAuth1Auth} from '../../../integrations/jiraServer/JiraServerOAuth1Manager' import {IntegrationProviderAzureDevOps} from '../../../postgres/queries/getIntegrationProvidersByIds' import upsertTeamMemberIntegrationAuth from '../../../postgres/queries/upsertTeamMemberIntegrationAuth' import AzureDevOpsServerManager from '../../../utils/AzureDevOpsServerManager' import {analytics} from '../../../utils/analytics/analytics' import {getUserId, isTeamMember} from '../../../utils/authorization' import standardError from '../../../utils/standardError' import updateRepoIntegrationsCacheByPerms from '../../queries/helpers/updateRepoIntegrationsCacheByPerms' import {MutationResolvers} from '../resolverTypes'
32-34: Enhance error message for clarity.The error message could be more descriptive to help with debugging and user feedback.
- return standardError(new Error('Attempted teamId spoof'), {userId: viewerId}) + return standardError(new Error('Permission denied: User is not a team member of the specified team.'), {userId: viewerId})
41-47: Ensure proper error handling for missing integration provider.Consider adding more context to the error message for better debugging.
return standardError( new Error(`Unable to find appropriate integration provider for providerId ${providerId}`), { userId: viewerId, providerId } )
127-131: Ensure proper error handling for token metadata.Consider adding more context to the error message for better debugging.
return standardError(tokenMetadata, { userId: viewerId, providerId })
145-146: Consider returning more detailed data.Returning more detailed data can be beneficial for debugging and client-side handling.
const data = {userId: viewerId, teamId, service} return datacodegen.json (2)
126-126: The path forRemoveTeamMemberIntegrationAuthSuccessis incorrect.The correct path should be
./types/RemoveTeamMemberIntegrationAuthSuccess#RemoveTeamMemberIntegrationAuthSuccessSource.
- Current path:
./types/RemoveTeamMemberIntegrationAuthPayload#RemoveTeamMemberIntegrationAuthSuccessSource- Correct path:
./types/RemoveTeamMemberIntegrationAuthSuccess#RemoveTeamMemberIntegrationAuthSuccessSourcePlease update the configuration accordingly.
Analysis chain
Verify the path and type mapping for
RemoveTeamMemberIntegrationAuthSuccess.Ensure that the path and type mapping are correct and the type exists at the specified path.
Verify the path and type mapping for
RemoveTeamMemberIntegrationAuthSuccess.Ensure that the file
RemoveTeamMemberIntegrationAuthPayload.tsexists and contains the type definitionRemoveTeamMemberIntegrationAuthSuccessSource.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the path and type mapping for `RemoveTeamMemberIntegrationAuthSuccess`. # Test: Search for the type definition. Expect: Correct path and type definition. rg --type ts 'RemoveTeamMemberIntegrationAuthSuccessSource' ./types/RemoveTeamMemberIntegrationAuthPayload.tsLength of output: 199
Script:
#!/bin/bash # Description: Verify the path and type mapping for `RemoveTeamMemberIntegrationAuthSuccess`. # Search for the file `RemoveTeamMemberIntegrationAuthPayload.ts` in the repository fd 'RemoveTeamMemberIntegrationAuthPayload.ts' # Search for the type definition `RemoveTeamMemberIntegrationAuthSuccessSource` across the repository rg 'RemoveTeamMemberIntegrationAuthSuccessSource' --type tsLength of output: 246
67-67: Incorrect path forAddTeamMemberIntegrationAuthSuccesstype mapping.The path specified for
AddTeamMemberIntegrationAuthSuccessis incorrect. It should be updated topackages/server/graphql/public/types/AddTeamMemberIntegrationAuthSuccess.ts#AddTeamMemberIntegrationAuthSuccessSource.
- Current path:
./types/AddTeamMemberIntegrationAuthPayload#AddTeamMemberIntegrationAuthSuccessSource- Correct path:
packages/server/graphql/public/types/AddTeamMemberIntegrationAuthSuccess.ts#AddTeamMemberIntegrationAuthSuccessSourceAnalysis chain
Verify the path and type mapping for
AddTeamMemberIntegrationAuthSuccess.Ensure that the path and type mapping are correct and the type exists at the specified path.
Verify the existence and correct path of
AddTeamMemberIntegrationAuthPayload.tsand the typeAddTeamMemberIntegrationAuthSuccessSource.Ensure that the file and type exist in the codebase.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the path and type mapping for `AddTeamMemberIntegrationAuthSuccess`. # Test: Search for the type definition. Expect: Correct path and type definition. rg --type ts 'AddTeamMemberIntegrationAuthSuccessSource' ./types/AddTeamMemberIntegrationAuthPayload.tsLength of output: 190
Script:
#!/bin/bash # Description: Verify the existence and correct path of `AddTeamMemberIntegrationAuthPayload.ts` and the type `AddTeamMemberIntegrationAuthSuccessSource`. # Search for the file `AddTeamMemberIntegrationAuthPayload.ts` in the entire codebase. fd AddTeamMemberIntegrationAuthPayload.ts # Search for the type definition `AddTeamMemberIntegrationAuthSuccessSource` in the entire codebase. rg 'AddTeamMemberIntegrationAuthSuccessSource'Length of output: 375
packages/server/graphql/public/mutations/removeTeamMemberIntegrationAuth.ts
Show resolved
Hide resolved
packages/server/graphql/public/mutations/addTeamMemberIntegrationAuth.ts
Show resolved
Hide resolved
packages/server/graphql/public/mutations/addTeamMemberIntegrationAuth.ts
Show resolved
Hide resolved
|
This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR will be delayed and might be rejected due to its size. |
There were some type resolver issues caused by #10000 and converting more types to SDL was the easiest way to fix the issues caused by not being able to resolve some interface types.
Description
Fixes/Partially Fixes #[issue number]
[Please include a summary of the changes and the related issue]
Demo
[If possible, please include a screenshot or gif/video, it'll make it easier for reviewers to understand the scope of the changes and how the change is supposed to work. If you're introducing something new or changing the existing patterns, please share a Loom and explain what decisions you've made and under what circumstances]
Testing scenarios
[Please list all the testing scenarios a reviewer has to check before approving the PR]
Scenario A
Scenario B
Final checklist
Summary by CodeRabbit
New Features
Bug Fixes
Documentation