Gitlab Connector #421
Gitlab Connector #421
Conversation
mit-27
commented
May 5, 2024
•
mit-27
commented
- Closes feat: Add Gitlab integration to ticketing vertical #367
- Objects
- Collections
- Comments
- Tags
- It is not compatible with Panora's Unified Model. The tag should be associated with the ticket (issue). However, for GitLab, tags are associated with individual projects, which makes it incompatible with Panora's Unified Model.
- Tickets
- Users
|
|
@mit-27 is attempting to deploy a commit to the Panora Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThe recent updates focus on integrating Gitlab into the ticketing system, enhancing functionalities across collections, comments, tags, tickets, and user data synchronization. These changes introduce new services, mappers, and types specific to Gitlab, improving the system's ability to interact effectively with Gitlab resources. Changes
Assessment against linked issues
Poem
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 Configration 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: 7
Out of diff range and nitpick comments (4)
packages/shared/src/authUrl.ts (1)
Line range hint
85-100: Ensure proper error handling for missingclientIdandscopes.- if (!clientId) throw new Error(`No client id for type ${type}`) + if (!clientId) { + throw new Error(`No client id for type ${type}`); + } - const scopes = data.SCOPE + const scopes = data.SCOPE; + if (!scopes) { + throw new Error(`No scopes provided for type ${type}`); + }packages/api/src/@core/connections/ticketing/services/gitlab/gitlab.service.ts (1)
Line range hint
198-198: Correct the logging to reflect the actual service being used (Gitlab instead of Jira).- this.logger.log('OAuth credentials updated : jira '); + this.logger.log('OAuth credentials updated : gitlab ');packages/shared/src/utils.ts (2)
17-23: Consider adding a comment explaining the purpose ofcustomPropertiesUrlin theProviderConfigtype. This helps maintainers understand its use case, especially since it appears in multiple places within the configuration.
2164-2172: The functiongetActiveProvidersForVerticalassumes that any provider not explicitly marked as inactive (active: false) is considered active. Consider adding a comment in the function to clarify this assumption for future maintainers.
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
packages/api/swagger/swagger-spec.jsonis excluded by!**/*.json
Files selected for processing (15)
- packages/api/src/@core/connections/ticketing/services/gitlab/gitlab.service.ts (3 hunks)
- packages/api/src/@core/utils/types/original/original.ticketing.ts (6 hunks)
- packages/api/src/ticketing/collection/collection.module.ts (2 hunks)
- packages/api/src/ticketing/collection/services/collection.service.ts (1 hunks)
- packages/api/src/ticketing/collection/services/gitlab/index.ts (1 hunks)
- packages/api/src/ticketing/collection/services/gitlab/mappers.ts (1 hunks)
- packages/api/src/ticketing/collection/services/gitlab/types.ts (1 hunks)
- packages/api/src/ticketing/collection/services/jira/index.ts (2 hunks)
- packages/api/src/ticketing/collection/sync/sync.service.ts (3 hunks)
- packages/api/src/ticketing/collection/types/index.ts (1 hunks)
- packages/api/src/ticketing/collection/types/mappingsTypes.ts (1 hunks)
- packages/shared/src/authUrl.ts (3 hunks)
- packages/shared/src/enum.ts (1 hunks)
- packages/shared/src/providers.ts (1 hunks)
- packages/shared/src/utils.ts (148 hunks)
Files not summarized due to errors (1)
- packages/shared/src/utils.ts: Error: Message exceeds token limit
Files skipped from review due to trivial changes (2)
- packages/shared/src/enum.ts
- packages/shared/src/providers.ts
Additional comments not posted (13)
packages/api/src/ticketing/collection/types/mappingsTypes.ts (3)
1-1: Ensure that theGitlabCollectionMapperandJiraCollectionMapperare correctly imported and utilized.
6-6: Initialization ofgitlabCollectionMapperis correctly done.
10-14: The mapping object forgitlabcorrectly binds theunifyanddesunifymethods to ensure the correct context is preserved when these methods are called.packages/api/src/ticketing/collection/types/index.ts (2)
6-6: Import statements are correctly organized and necessary types are included.
15-18: TheaddCollectionmethod in theICollectionServiceinterface is correctly defined with appropriate parameters and return type.packages/api/src/ticketing/collection/collection.module.ts (1)
36-36: TheGitlabServiceis correctly added to the providers array, ensuring it is available for dependency injection within the module.packages/api/src/ticketing/collection/services/gitlab/mappers.ts (1)
8-56: TheGitlabCollectionMapperclass correctly implements thedesunifyandunifymethods as per theICollectionMapperinterface. The methods handle custom field mappings and ensure proper transformation between unified and Gitlab-specific formats.packages/api/src/ticketing/collection/services/jira/index.ts (1)
66-73: TheaddCollectionmethod inJiraServiceis defined but returnsnull. This might be intentional (e.g., method not implemented yet), but it's important to ensure that this is either fully implemented or properly documented if it's a work in progress.packages/api/src/ticketing/collection/services/gitlab/types.ts (1)
1-98: TheGitlabCollectioninterface is comprehensive and includes a wide range of properties that cover various aspects of a Gitlab project. Ensure that all optional properties are correctly marked and that data types are appropriate for each property.packages/shared/src/authUrl.ts (1)
130-130: Clarify the use ofcode_challengeandcode_challenge_methodfor Gitlab OAuth.packages/api/src/@core/connections/ticketing/services/gitlab/gitlab.service.ts (1)
108-108: Validate the use ofaccount_urlfromprovidersConfigto ensure it is correctly populated and used.Verification successful
The search results confirm that
account_urlis widely used across various services within the codebase, including the specific file and line in question (packages/api/src/@core/connections/ticketing/services/gitlab/gitlab.service.ts). The usage pattern generally involves constructing API endpoints by appending specific paths toaccount_url, which is consistent with typical usage scenarios for such a configuration parameter.Given this context, the usage of
account_urlas seen in the GitlabConnectionService and across other services appears to be correct and consistent. The configuration is being properly retrieved (providersConfig['ticketing']['gitlab'].urls.apiUrl) and utilized in forming API requests, which aligns with the expected behavior described in the review comment.Conclusion:
The use ofaccount_urlin the GitlabConnectionService and throughout the codebase is verified to be correctly populated and used as per the standards observed in similar services. No discrepancies or issues were found regarding its implementation.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correct population and usage of `account_url` in the GitlabConnectionService. # Test: Search for the usage of `account_url`. Expect: Correct retrieval and usage in service methods. rg --type typescript 'account_url'Length of output: 69
Script:
#!/bin/bash # Correcting the script to search for the usage of `account_url` in TypeScript files. rg --type-add 'typescript:*.ts' -g '*.ts' 'account_url'Length of output: 21717
packages/api/src/@core/utils/types/original/original.ticketing.ts (1)
116-118: LGTM! The Gitlab type definitions are correctly integrated with existing ticketing types.packages/shared/src/utils.ts (1)
2164-2172: The implementation ofprovidersArrayis straightforward and correctly utilizesgetActiveProvidersForVertical. Good use of existing functionality.
| @@ -68,6 +68,7 @@ export class CollectionService { | |||
| remote_data?: boolean, | |||
| ): Promise<UnifiedCollectionOutput[]> { | |||
| try { | |||
| console.log("In collection service : ", integrationId) | |||
There was a problem hiding this comment.
The logging statement in getCollections method provides useful debugging information. Ensure that such logs are appropriately gated (e.g., only in debug mode) to avoid verbose logging in production.
- console.log("In collection service : ", integrationId)
+ if (process.env.DEBUG_MODE === 'true') {
+ console.log("In collection service : ", integrationId);
+ }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.
| console.log("In collection service : ", integrationId) | |
| if (process.env.DEBUG_MODE === 'true') { | |
| console.log("In collection service : ", integrationId); | |
| } |
| }, | ||
| }); | ||
|
|
||
| console.log("xzx : ", `${connection.account_url}/projects`) |
There was a problem hiding this comment.
The logging statement in syncCollections method of GitlabService is useful for debugging. Consider gating this log under a debug mode check to prevent verbose output in production environments.
- console.log("xzx : ", `${connection.account_url}/projects`)
+ if (process.env.DEBUG_MODE === 'true') {
+ console.log("xzx : ", `${connection.account_url}/projects`);
+ }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.
| console.log("xzx : ", `${connection.account_url}/projects`) | |
| if (process.env.DEBUG_MODE === 'true') { | |
| console.log("xzx : ", `${connection.account_url}/projects`); | |
| } |
| const BASE_URL: string = providerName === 'gorgias' ? `${apiUrl}${baseUrl}` : | ||
| data.SUBDOMAIN ? data.SUBDOMAIN + baseUrl : baseUrl; |
There was a problem hiding this comment.
Refactor the construction of BASE_URL to handle potential null values more gracefully.
- const BASE_URL: string = providerName === 'gorgias' ? `${apiUrl}${baseUrl}` :
- data.SUBDOMAIN ? data.SUBDOMAIN + baseUrl : baseUrl;
+ let baseDomain = baseUrl;
+ if (providerName === 'gorgias') {
+ baseDomain = `${apiUrl}${baseUrl}`;
+ } else if (data.SUBDOMAIN) {
+ baseDomain = `${data.SUBDOMAIN}${baseUrl}`;
+ }
+ const BASE_URL = baseDomain || throw new Error(`Base URL cannot be determined for provider: ${providerName}`);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.
| const BASE_URL: string = providerName === 'gorgias' ? `${apiUrl}${baseUrl}` : | |
| data.SUBDOMAIN ? data.SUBDOMAIN + baseUrl : baseUrl; | |
| let baseDomain = baseUrl; | |
| if (providerName === 'gorgias') { | |
| baseDomain = `${apiUrl}${baseUrl}`; | |
| } else if (data.SUBDOMAIN) { | |
| baseDomain = `${data.SUBDOMAIN}${baseUrl}`; | |
| } | |
| const BASE_URL = baseDomain || throw new Error(`Base URL cannot be determined for provider: ${providerName}`); |
| @@ -67,7 +67,7 @@ export class GitlabConnectionService implements ITicketingConnectionService { | |||
| grant_type: 'authorization_code', | |||
| }); | |||
| const res = await axios.post( | |||
| `https://api.gitlab.app/oauth/token`, | |||
| `https://gitlab.com/oauth/token`, | |||
There was a problem hiding this comment.
Ensure the URL used for the OAuth token exchange is configurable rather than hardcoded.
- `https://gitlab.com/oauth/token`,
+ `${providersConfig['ticketing']['gitlab'].urls.apiUrl}/oauth/token`,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.
| `https://gitlab.com/oauth/token`, | |
| `${providersConfig['ticketing']['gitlab'].urls.apiUrl}/oauth/token`, |
| @@ -59,7 +59,7 @@ export class SyncService implements OnModuleInit { | |||
| } | |||
| //function used by sync worker which populate our tcg_collections table | |||
| //its role is to fetch all collections from providers 3rd parties and save the info inside our db | |||
| //@Cron('*/2 * * * *') // every 2 minutes (for testing) | |||
| // @Cron('*/2 * * * *') // every 2 minutes (for testing) | |||
There was a problem hiding this comment.
Update the CRON schedule comment to reflect the actual schedule used (every 8 hours).
- // @Cron('*/2 * * * *') // every 2 minutes (for testing)
+ // @Cron('0 */8 * * *') // every 8 hoursCommittable 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.
| // @Cron('*/2 * * * *') // every 2 minutes (for testing) | |
| // @Cron('0 */8 * * *') // every 8 hours |
| @@ -195,6 +195,7 @@ | |||
| ): Promise<TicketingCollection[]> { | |||
| try { | |||
| let collections_results: TicketingCollection[] = []; | |||
| console.log(collections) | |||
There was a problem hiding this comment.
Remove unnecessary console logging to maintain clean and professional code.
- console.log(collections)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.
| console.log(collections) |
There was a problem hiding this comment.
Actionable comments posted: 0
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- packages/shared/src/webhookScopes.ts (1 hunks)
Files skipped from review due to trivial changes (1)
- packages/shared/src/webhookScopes.ts
There was a problem hiding this comment.
Actionable comments posted: 0
Out of diff range and nitpick comments (1)
packages/shared/src/utils.ts (1)
17-23: Consider adding a comment explaining the purpose ofcustomPropertiesUrlin theProviderConfigtype. This will help other developers understand its use case, especially since it appears in multiple places within the configuration object.
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (6)
- packages/api/src/ticketing/user/services/gitlab/index.ts (1 hunks)
- packages/api/src/ticketing/user/services/gitlab/mappers.ts (1 hunks)
- packages/api/src/ticketing/user/services/gitlab/types.ts (1 hunks)
- packages/api/src/ticketing/user/types/mappingsTypes.ts (1 hunks)
- packages/api/src/ticketing/user/user.module.ts (2 hunks)
- packages/shared/src/utils.ts (148 hunks)
Files not summarized due to errors (1)
- packages/shared/src/utils.ts: Error: Message exceeds token limit
Files skipped from review due to trivial changes (1)
- packages/api/src/ticketing/user/services/gitlab/types.ts
Additional comments not posted (9)
packages/api/src/ticketing/user/types/mappingsTypes.ts (3)
1-1: Ensure that theGitlabUserMapperimport is correctly utilized within the file.Verification successful
The verification process has confirmed that the
GitlabUserMapperis indeed utilized within the filemappingsTypes.ts. The import statement is followed by an instantiation ofGitlabUserMapper, which indicates that the import is actively used in the file.Conclusion:
- The import of
GitlabUserMapperis correctly utilized in the filemappingsTypes.ts.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that `GitlabUserMapper` is used in the file `mappingsTypes.ts`. # Test: Search for the usage of `GitlabUserMapper`. Expect: At least one occurrence. rg --type ts 'GitlabUserMapper' 'packages/api/src/ticketing/user/types/mappingsTypes.ts'Length of output: 198
10-10: Addition ofGitlabUserMapperinstance is consistent with the pattern used for other services.
22-25: The method bindings forgitlabinuserUnificationMappingare correctly implemented.packages/api/src/ticketing/user/user.module.ts (1)
42-42: Addition ofGitlabServiceto the providers list ensures it is available for dependency injection.packages/api/src/ticketing/user/services/gitlab/mappers.ts (1)
8-58: TheGitlabUserMapperclass is well-implemented with methods to handle both single and batch user data transformations.packages/api/src/ticketing/user/services/gitlab/index.ts (1)
13-63: TheGitlabServiceclass correctly implements user synchronization with Gitlab, including error handling and logging.packages/shared/src/utils.ts (3)
111-116: Ensure that theauthBaseUrlandapiUrlfor the 'accelo' provider are absolute URLs unless relative URLs are handled dynamically. Relative URLs could lead to requests being sent to incorrect addresses.
2164-2172: The functionprovidersArraycorrectly constructs an array of provider details. However, consider adding error handling or logging to manage cases where the provider or vertical does not exist, as previously suggested by another review. This would improve debugging and maintainability.
394-402: Thegitlabprovider configuration has been added with detailed scopes and URLs. Ensure that theauthBaseUrlandapiUrlare correct and accessible. This is crucial as incorrect URLs can lead to failed connections with the Gitlab API.
There was a problem hiding this comment.
Actionable comments posted: 2
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (12)
- packages/api/src/@core/connections/ticketing/services/gitlab/gitlab.service.ts (6 hunks)
- packages/api/src/ticketing/collection/services/gitlab/index.ts (1 hunks)
- packages/api/src/ticketing/collection/services/jira/index.ts (2 hunks)
- packages/api/src/ticketing/collection/types/index.ts (1 hunks)
- packages/api/src/ticketing/tag/services/gitlab/index.ts (1 hunks)
- packages/api/src/ticketing/tag/services/gitlab/mappers.ts (1 hunks)
- packages/api/src/ticketing/tag/services/gitlab/types.ts (1 hunks)
- packages/api/src/ticketing/tag/tag.module.ts (2 hunks)
- packages/api/src/ticketing/tag/types/mappingsTypes.ts (2 hunks)
- packages/api/src/ticketing/user/services/gitlab/index.ts (1 hunks)
- packages/api/src/ticketing/user/services/gitlab/mappers.ts (1 hunks)
- packages/api/src/ticketing/user/sync/sync.service.ts (3 hunks)
Files skipped from review due to trivial changes (1)
- packages/api/src/ticketing/collection/types/index.ts
Files skipped from review as they are similar to previous changes (5)
- packages/api/src/@core/connections/ticketing/services/gitlab/gitlab.service.ts
- packages/api/src/ticketing/collection/services/gitlab/index.ts
- packages/api/src/ticketing/collection/services/jira/index.ts
- packages/api/src/ticketing/user/services/gitlab/index.ts
- packages/api/src/ticketing/user/services/gitlab/mappers.ts
Additional comments not posted (7)
packages/api/src/ticketing/tag/services/gitlab/types.ts (2)
11-24: TheCommitinterface is well-defined and clear.
26-29: TheReleaseinterface is correctly defined.packages/api/src/ticketing/tag/types/mappingsTypes.ts (1)
11-29: The registration ofGitlabTagMapperis correctly implemented, and the use of.bindis appropriate for maintaining thethiscontext.packages/api/src/ticketing/tag/services/gitlab/mappers.ts (1)
19-47: Theunifymethod is implemented correctly, efficiently handling both single and array inputs.packages/api/src/ticketing/tag/tag.module.ts (1)
42-42: The addition ofGitlabServiceto the providers list is correctly implemented, ensuring availability for dependency injection.packages/api/src/ticketing/tag/services/gitlab/index.ts (1)
28-69: ThesyncTagsmethod is well-implemented, correctly handling API calls, using encrypted tokens for authorization, and logging the process.packages/api/src/ticketing/user/sync/sync.service.ts (1)
Line range hint
63-262: ThesyncUsersmethod is well-implemented, handling user synchronization across different providers, correctly managing errors, and logging the process.
| release: Release | ||
| name: string | ||
| target: string | ||
| message: any |
There was a problem hiding this comment.
Specify a more precise type for the message property instead of any to enhance type safety.
| desunify( | ||
| source: UnifiedTagInput, | ||
| customFieldMappings?: { | ||
| slug: string; | ||
| remote_id: string; | ||
| }[], | ||
| ): GitlabTagInput { | ||
| return; | ||
| } |
There was a problem hiding this comment.
Implement the desunify method or ensure it returns a meaningful value. Currently, it returns nothing, which is likely an oversight.
There was a problem hiding this comment.
Actionable comments posted: 0
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- packages/api/src/@core/utils/types/original/original.ticketing.ts (8 hunks)
- packages/api/src/ticketing/tag/services/gitlab/mappers.ts (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- packages/api/src/@core/utils/types/original/original.ticketing.ts
- packages/api/src/ticketing/tag/services/gitlab/mappers.ts
|
Sorry @Rutik7066, I am already on it.Feel free to pick any other issue. |
There was a problem hiding this comment.
Actionable comments posted: 12
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (18)
- packages/api/src/@core/utils/types/original/original.ticketing.ts (9 hunks)
- packages/api/src/ticketing/@lib/@utils/index.ts (1 hunks)
- packages/api/src/ticketing/comment/comment.module.ts (2 hunks)
- packages/api/src/ticketing/comment/services/gitlab/index.ts (1 hunks)
- packages/api/src/ticketing/comment/services/gitlab/mappers.ts (1 hunks)
- packages/api/src/ticketing/comment/services/gitlab/types.ts (1 hunks)
- packages/api/src/ticketing/comment/sync/sync.service.ts (1 hunks)
- packages/api/src/ticketing/comment/types/mappingsTypes.ts (2 hunks)
- packages/api/src/ticketing/tag/tag.module.ts (1 hunks)
- packages/api/src/ticketing/tag/types/mappingsTypes.ts (2 hunks)
- packages/api/src/ticketing/ticket/services/gitlab/index.ts (1 hunks)
- packages/api/src/ticketing/ticket/services/gitlab/mappers.ts (1 hunks)
- packages/api/src/ticketing/ticket/services/gitlab/types.ts (1 hunks)
- packages/api/src/ticketing/ticket/sync/sync.service.ts (1 hunks)
- packages/api/src/ticketing/ticket/ticket.module.ts (2 hunks)
- packages/api/src/ticketing/ticket/types/mappingsTypes.ts (2 hunks)
- packages/shared/src/enum.ts (1 hunks)
- packages/shared/src/providers.ts (1 hunks)
Files skipped from review as they are similar to previous changes (5)
- packages/api/src/@core/utils/types/original/original.ticketing.ts
- packages/api/src/ticketing/tag/tag.module.ts
- packages/api/src/ticketing/tag/types/mappingsTypes.ts
- packages/shared/src/enum.ts
- packages/shared/src/providers.ts
Additional comments not posted (21)
packages/api/src/ticketing/comment/services/gitlab/types.ts (3)
1-18: TheGitlabCommentinterface is well-defined with appropriate fields for GitLab comments. Ensure that theattachmenttype is appropriately handled in the system since it's currently set toany.
20-27: TheAuthorinterface is clearly defined. Consider adding type annotations for fields like
30-31: The type aliasesGitlabCommentInputandGitlabCommentOutputare correctly defined as partials ofGitlabComment, facilitating flexible API interactions.packages/api/src/ticketing/comment/types/mappingsTypes.ts (2)
1-2: Ensure that all imported mappers are utilized within the file. The import ofGitlabCommentMapperis correctly placed and used in the mapping object.
11-33: ThecommentUnificationMappingobject correctly binds methods from various comment mappers to handle unification and de-unification. This setup facilitates easy extension and maintenance of comment handling across different platforms.packages/api/src/ticketing/comment/comment.module.ts (2)
1-1: The import ofGitlabServiceis correctly placed for integration within the comment module. Ensure that this service is properly implemented to interact with GitLab's API.
42-42: The inclusion ofGitlabServicein the providers array of the@Moduledecorator ensures that it is available throughout the comment module. This is crucial for the integration of GitLab services.packages/api/src/ticketing/ticket/ticket.module.ts (2)
1-1: The import ofGitlabServiceis correctly placed for integration within the ticket module. Ensure that this service is properly implemented to interact with GitLab's API.
46-46: The inclusion ofGitlabServicein the providers array of the@Moduledecorator ensures that it is available throughout the ticket module. This is crucial for the integration of GitLab services.packages/api/src/ticketing/ticket/services/gitlab/types.ts (8)
1-36: TheGitlabTicketinterface is well-defined with appropriate fields for GitLab tickets. Ensure that the types for fields likeassignees,weight,epic,assignee,milestone,closed_by,due_date, anddescriptionare appropriately handled in the system since they're currently set toany.
38-44: TheEpicinterface is clearly defined with fields relevant to an epic in GitLab. Ensure that theurlfield is validated to be a proper URL format.
46-53: TheAuthorinterface for tickets is similar to that in comments but includes additional fields likeavatar_urlandweb_url. Ensure these fields are correctly handled and validated, especiallyweb_urlfor proper URL format.
55-59: TheReferencesinterface is well-structured to hold different types of references. Ensure that these references are correctly parsed and used within the system.
61-66: TheTimeStatsinterface captures time-related statistics for tickets. The use ofanyforhuman_time_estimateandhuman_total_time_spentshould be revisited to ensure they are parsed and displayed correctly in the UI.
68-74: TheLinksinterface provides URLs related to a ticket. Ensure all URLs are validated and securely handled to prevent injection attacks.
76-79: TheTaskCompletionStatusinterface is straightforward and effectively captures the completion status of tasks within a ticket.
81-82: The type aliasesGitlabTicketInputandGitlabTicketOutputare correctly defined as partials ofGitlabTicket, facilitating flexible API interactions.packages/api/src/ticketing/ticket/types/mappingsTypes.ts (2)
1-2: Ensure that all imported mappers are utilized within the file. The import ofGitlabTicketMapperis correctly placed and used in the mapping object.
15-44: TheticketUnificationMappingobject correctly binds methods from various ticket mappers to handle unification and de-unification. This setup facilitates easy extension and maintenance of ticket handling across different platforms.packages/api/src/ticketing/ticket/services/gitlab/index.ts (1)
14-105: TheGitlabServiceclass is well-structured and implements necessary methods to interact with GitLab's ticketing API. Ensure that error handling is robust and that all external calls are secured.packages/api/src/ticketing/comment/services/gitlab/mappers.ts (1)
13-123: TheGitlabCommentMapperclass effectively provides methods for transforming comments between GitLab-specific and unified formats. Ensure that the TODO items, especially regarding attachments, are addressed promptly to avoid incomplete features.
| async getCollectionUuidFromRemoteId(remote_id: string, remote_platform: string) { | ||
| try { | ||
| const res = await this.prisma.tcg_collections.findFirst({ | ||
| where: { | ||
| remote_id: remote_id, | ||
| remote_platform: remote_platform, | ||
| }, | ||
| }); | ||
| if (!res) return; | ||
| return res.id_tcg_collection; | ||
| } catch (error) { | ||
| throw new Error(error); | ||
| } | ||
| } |
There was a problem hiding this comment.
Consider handling the case where the result is null more gracefully.
- if (!res) return;
+ if (!res) throw new Error(`Collection not found for remote ID ${remote_id} and platform ${remote_platform}`);This change ensures that the function throws an informative error when no collection is found, rather than silently failing.
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.
| async getCollectionUuidFromRemoteId(remote_id: string, remote_platform: string) { | |
| try { | |
| const res = await this.prisma.tcg_collections.findFirst({ | |
| where: { | |
| remote_id: remote_id, | |
| remote_platform: remote_platform, | |
| }, | |
| }); | |
| if (!res) return; | |
| return res.id_tcg_collection; | |
| } catch (error) { | |
| throw new Error(error); | |
| } | |
| } | |
| async getCollectionUuidFromRemoteId(remote_id: string, remote_platform: string) { | |
| try { | |
| const res = await this.prisma.tcg_collections.findFirst({ | |
| where: { | |
| remote_id: remote_id, | |
| remote_platform: remote_platform, | |
| }, | |
| }); | |
| if (!res) throw new Error(`Collection not found for remote ID ${remote_id} and platform ${remote_platform}`); | |
| return res.id_tcg_collection; | |
| } catch (error) { | |
| throw new Error(error); | |
| } | |
| } |
| async getCollectionRemoteIdFromUuid(uuid: string) { | ||
| try { | ||
| const res = await this.prisma.tcg_collections.findFirst({ | ||
| where: { | ||
| id_tcg_collection: uuid, | ||
| }, | ||
| }); | ||
| if (!res) throw new Error(`tcg_contact not found for uuid ${uuid}`); | ||
| return res.remote_id; | ||
| } catch (error) { | ||
| throw new Error(error); | ||
| } | ||
| } |
There was a problem hiding this comment.
Ensure consistent error messages for better debugging and user feedback.
- if (!res) throw new Error(`tcg_contact not found for uuid ${uuid}`);
+ if (!res) throw new Error(`Collection not found for uuid ${uuid}`);This change corrects the error message to reflect the actual entity being queried, which is a collection, not a contact.
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.
| async getCollectionRemoteIdFromUuid(uuid: string) { | |
| try { | |
| const res = await this.prisma.tcg_collections.findFirst({ | |
| where: { | |
| id_tcg_collection: uuid, | |
| }, | |
| }); | |
| if (!res) throw new Error(`tcg_contact not found for uuid ${uuid}`); | |
| return res.remote_id; | |
| } catch (error) { | |
| throw new Error(error); | |
| } | |
| } | |
| async getCollectionRemoteIdFromUuid(uuid: string) { | |
| try { | |
| const res = await this.prisma.tcg_collections.findFirst({ | |
| where: { | |
| id_tcg_collection: uuid, | |
| }, | |
| }); | |
| if (!res) throw new Error(`Collection not found for uuid ${uuid}`); | |
| return res.remote_id; | |
| } catch (error) { | |
| throw new Error(error); | |
| } | |
| } |
| async getTicketUuidFromRemoteId(remote_id: string, remote_platform: string) { | ||
| try { | ||
| const res = await this.prisma.tcg_tickets.findFirst({ | ||
| where: { | ||
| remote_id: remote_id, | ||
| remote_platform: remote_platform, | ||
| }, | ||
| }); | ||
| if (!res) return; | ||
| return res.id_tcg_ticket; | ||
| } catch (error) { | ||
| throw new Error(error); | ||
| } | ||
| } |
There was a problem hiding this comment.
Consider handling the case where the result is null more gracefully.
- if (!res) return;
+ if (!res) throw new Error(`Ticket not found for remote ID ${remote_id} and platform ${remote_platform}`);This change ensures that the function throws an informative error when no ticket is found, rather than silently failing.
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.
| async getTicketUuidFromRemoteId(remote_id: string, remote_platform: string) { | |
| try { | |
| const res = await this.prisma.tcg_tickets.findFirst({ | |
| where: { | |
| remote_id: remote_id, | |
| remote_platform: remote_platform, | |
| }, | |
| }); | |
| if (!res) return; | |
| return res.id_tcg_ticket; | |
| } catch (error) { | |
| throw new Error(error); | |
| } | |
| } | |
| async getTicketUuidFromRemoteId(remote_id: string, remote_platform: string) { | |
| try { | |
| const res = await this.prisma.tcg_tickets.findFirst({ | |
| where: { | |
| remote_id: remote_id, | |
| remote_platform: remote_platform, | |
| }, | |
| }); | |
| if (!res) throw new Error(`Ticket not found for remote ID ${remote_id} and platform ${remote_platform}`); | |
| return res.id_tcg_ticket; | |
| } catch (error) { | |
| throw new Error(error); | |
| } | |
| } |
| async getTicketRemoteIdFromUuid(uuid: string) { | ||
| try { | ||
| const res = await this.prisma.tcg_tickets.findFirst({ | ||
| where: { | ||
| id_tcg_ticket: uuid, | ||
| }, | ||
| }); | ||
| if (!res) throw new Error(`tcg_contact not found for uuid ${uuid}`); | ||
| return res.remote_id; | ||
| } catch (error) { | ||
| throw new Error(error); | ||
| } | ||
| } |
There was a problem hiding this comment.
Ensure consistent error messages for better debugging and user feedback.
- if (!res) throw new Error(`tcg_contact not found for uuid ${uuid}`);
+ if (!res) throw new Error(`Ticket not found for uuid ${uuid}`);This change corrects the error message to reflect the actual entity being queried, which is a ticket, not a contact.
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.
| async getTicketRemoteIdFromUuid(uuid: string) { | |
| try { | |
| const res = await this.prisma.tcg_tickets.findFirst({ | |
| where: { | |
| id_tcg_ticket: uuid, | |
| }, | |
| }); | |
| if (!res) throw new Error(`tcg_contact not found for uuid ${uuid}`); | |
| return res.remote_id; | |
| } catch (error) { | |
| throw new Error(error); | |
| } | |
| } | |
| async getTicketRemoteIdFromUuid(uuid: string) { | |
| try { | |
| const res = await this.prisma.tcg_tickets.findFirst({ | |
| where: { | |
| id_tcg_ticket: uuid, | |
| }, | |
| }); | |
| if (!res) throw new Error(`Ticket not found for uuid ${uuid}`); | |
| return res.remote_id; | |
| } catch (error) { | |
| throw new Error(error); | |
| } | |
| } |
| async desunify( | ||
| source: UnifiedTicketInput, | ||
| customFieldMappings?: { | ||
| slug: string; | ||
| remote_id: string; | ||
| }[], | ||
| ): Promise<GitlabTicketInput> { | ||
|
|
||
| // TODO - Project_id should be mandatory field for gitlab provider | ||
|
|
||
| const remote_project_id = await this.utils.getCollectionRemoteIdFromUuid(source.project_id); | ||
|
|
||
|
|
||
|
|
||
|
|
||
| const result: GitlabTicketInput = { | ||
| title: source.name, | ||
| description: source.description ? source.description : '', | ||
| project_id: Number(remote_project_id) | ||
| }; | ||
|
|
||
| if (source.status) { | ||
| result.type = source.status === "OPEN" ? "opened" : "closed"; | ||
| } | ||
|
|
||
| if (source.assigned_to && source.assigned_to.length > 0) { | ||
| const data = await this.utils.getAsigneeRemoteIdFromUserUuid( | ||
| source.assigned_to[0], | ||
| ); | ||
| result.assignee = { | ||
| id: Number(data), | ||
| }; | ||
| } | ||
|
|
||
| if (source.tags) { | ||
| result.labels = source.tags ? source.tags : [] | ||
| } | ||
|
|
||
| // TODO - Custom fields mapping | ||
| // if (customFieldMappings && source.field_mappings) { | ||
| // result.meta = {}; // Ensure meta exists | ||
| // for (const [k, v] of Object.entries(source.field_mappings)) { | ||
| // const mapping = customFieldMappings.find( | ||
| // (mapping) => mapping.slug === k, | ||
| // ); | ||
| // if (mapping) { | ||
| // result.meta[mapping.remote_id] = v; | ||
| // } | ||
| // } | ||
| // } | ||
|
|
||
| return result; |
There was a problem hiding this comment.
Consider implementing the custom fields mapping logic.
The commented-out code for custom fields mapping (lines 54-65) suggests a planned feature that is not yet implemented. Would you like me to help implement this feature or should we track this as a task in your project management tool?
| async addComment( | ||
| commentData: GitlabCommentInput, | ||
| linkedUserId: string, | ||
| remoteIdTicket: string, | ||
| ): Promise<ApiResponse<GitlabCommentOutput>> { | ||
| try { | ||
| const connection = await this.prisma.connections.findFirst({ | ||
| where: { | ||
| id_linked_user: linkedUserId, | ||
| provider_slug: 'gitlab', | ||
| vertical: 'ticketing', | ||
| }, | ||
| }); | ||
|
|
||
| let uploads = []; | ||
| const uuids = commentData.attachment as any[]; | ||
| if (uuids && uuids.length > 0) { | ||
| const attachmentPromises = uuids.map(async (uuid) => { | ||
| const res = await this.prisma.tcg_attachments.findUnique({ | ||
| where: { | ||
| id_tcg_attachment: uuid.extra, | ||
| }, | ||
| }); | ||
| if (!res) { | ||
| throw new Error(`tcg_attachment not found for uuid ${uuid}`); | ||
| } | ||
| // Assuming you want to construct the right binary attachment here | ||
| // For now, we'll just return the URL | ||
| const stats = fs.statSync(res.file_url); | ||
| return { | ||
| url: res.file_url, | ||
| name: res.file_name, | ||
| size: stats.size, | ||
| content_type: 'application/pdf', //todo | ||
| }; | ||
| }); | ||
| uploads = await Promise.all(attachmentPromises); | ||
| } | ||
|
|
||
| // Assuming you want to modify the comment object here | ||
| // For now, we'll just add the uploads to the comment | ||
| const data = { | ||
| ...commentData, | ||
| attachments: uploads, | ||
| }; | ||
|
|
||
| const ticket = await this.prisma.tcg_tickets.findUnique({ | ||
| where: { | ||
| id_tcg_ticket: remoteIdTicket, | ||
| }, | ||
| select: { | ||
| collections: true | ||
| }, | ||
| }); | ||
|
|
||
| const remote_project_id = await this.utils.getCollectionRemoteIdFromUuid(ticket.collections[0]) | ||
|
|
||
|
|
||
|
|
||
|
|
||
|
|
||
|
|
||
| const resp = await axios.post( | ||
| `${connection.account_url}/projects/${remote_project_id}/issues/${remoteIdTicket}/notes`, | ||
| JSON.stringify(data), | ||
| { | ||
| headers: { | ||
| 'Content-Type': 'application/json', | ||
| Authorization: `Bearer ${this.cryptoService.decrypt( | ||
| connection.access_token, | ||
| )}`, | ||
| }, | ||
| }, | ||
| ); | ||
|
|
||
| return { | ||
| data: resp.data, | ||
| message: 'Gitlab comment created', | ||
| statusCode: 201, | ||
| }; | ||
| } catch (error) { | ||
| handleServiceError( | ||
| error, | ||
| this.logger, | ||
| 'Gitlab', | ||
| TicketingObject.comment, | ||
| ActionType.POST, | ||
| ); | ||
| } | ||
| } |
There was a problem hiding this comment.
Refactor to improve error handling and reduce code duplication.
The methods addComment and syncComments share similar structures and error handling strategies. Consider refactoring to a common method that handles the repetitive tasks such as fetching the connection and handling errors.
| let uploads = []; | ||
| const uuids = commentData.attachment as any[]; | ||
| if (uuids && uuids.length > 0) { | ||
| const attachmentPromises = uuids.map(async (uuid) => { | ||
| const res = await this.prisma.tcg_attachments.findUnique({ | ||
| where: { | ||
| id_tcg_attachment: uuid.extra, | ||
| }, | ||
| }); | ||
| if (!res) { | ||
| throw new Error(`tcg_attachment not found for uuid ${uuid}`); | ||
| } | ||
| // Assuming you want to construct the right binary attachment here | ||
| // For now, we'll just return the URL | ||
| const stats = fs.statSync(res.file_url); | ||
| return { | ||
| url: res.file_url, | ||
| name: res.file_name, | ||
| size: stats.size, | ||
| content_type: 'application/pdf', //todo | ||
| }; | ||
| }); | ||
| uploads = await Promise.all(attachmentPromises); | ||
| } |
There was a problem hiding this comment.
Handle potential errors in attachment processing more robustly.
- throw new Error(`tcg_attachment not found for uuid ${uuid}`);
+ this.logger.error(`Attachment not found for uuid ${uuid}. Skipping attachment.`);
+ return null;This change logs an error and skips the attachment if it is not found, rather than throwing an error, which could stop the processing of other attachments.
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.
| let uploads = []; | |
| const uuids = commentData.attachment as any[]; | |
| if (uuids && uuids.length > 0) { | |
| const attachmentPromises = uuids.map(async (uuid) => { | |
| const res = await this.prisma.tcg_attachments.findUnique({ | |
| where: { | |
| id_tcg_attachment: uuid.extra, | |
| }, | |
| }); | |
| if (!res) { | |
| throw new Error(`tcg_attachment not found for uuid ${uuid}`); | |
| } | |
| // Assuming you want to construct the right binary attachment here | |
| // For now, we'll just return the URL | |
| const stats = fs.statSync(res.file_url); | |
| return { | |
| url: res.file_url, | |
| name: res.file_name, | |
| size: stats.size, | |
| content_type: 'application/pdf', //todo | |
| }; | |
| }); | |
| uploads = await Promise.all(attachmentPromises); | |
| } | |
| let uploads = []; | |
| const uuids = commentData.attachment as any[]; | |
| if (uuids && uuids.length > 0) { | |
| const attachmentPromises = uuids.map(async (uuid) => { | |
| const res = await this.prisma.tcg_attachments.findUnique({ | |
| where: { | |
| id_tcg_attachment: uuid.extra, | |
| }, | |
| }); | |
| if (!res) { | |
| this.logger.error(`Attachment not found for uuid ${uuid}. Skipping attachment.`); | |
| return null; | |
| } | |
| // Assuming you want to construct the right binary attachment here | |
| // For now, we'll just return the URL | |
| const stats = fs.statSync(res.file_url); | |
| return { | |
| url: res.file_url, | |
| name: res.file_name, | |
| size: stats.size, | |
| content_type: 'application/pdf', //todo | |
| }; | |
| }); | |
| uploads = await Promise.all(attachmentPromises); | |
| } |
| url: res.file_url, | ||
| name: res.file_name, | ||
| size: stats.size, | ||
| content_type: 'application/pdf', //todo |
There was a problem hiding this comment.
Consider specifying the content type dynamically based on the file type.
- content_type: 'application/pdf', //todo
+ content_type: this.determineContentType(res.file_url),This change replaces the hardcoded content type with a dynamic method that determines the content type based on the file extension, improving flexibility and correctness.
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.
| content_type: 'application/pdf', //todo | |
| content_type: this.determineContentType(res.file_url), |
| if (ticket.project_id) { | ||
| data = { ...data, collections: [ticket.project_id] } | ||
| } |
There was a problem hiding this comment.
Ensure that the collections field is handled correctly when saving tickets.
- data = { ...data, collections: [ticket.project_id] }
+ data = { ...data, collections: ticket.project_id ? [ticket.project_id] : [] }This change adds a conditional check to ensure that the collections field is only populated if project_id is present, preventing potential issues with null values.
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.
| if (ticket.project_id) { | |
| data = { ...data, collections: [ticket.project_id] } | |
| } | |
| if (ticket.project_id) { | |
| data = { ...data, collections: ticket.project_id ? [ticket.project_id] : [] } | |
| } |
| @@ -298,6 +298,7 @@ export class SyncService implements OnModuleInit { | |||
| if (comment.creator_type) { | |||
| data = { ...data, creator_type: comment.creator_type }; | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
Ensure that the opts object is correctly handled when saving comments.
+ if (Object.keys(opts).length === 0) throw new Error('Creator type is invalid or not provided.');This change adds a check to ensure that the opts object, which depends on the creator_type, is not empty before proceeding to save the comment, preventing issues with invalid or missing creator types.
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.
| if (Object.keys(opts).length === 0) throw new Error('Creator type is invalid or not provided.'); |
There was a problem hiding this comment.
Actionable comments posted: 2
Out of diff range and nitpick comments (1)
packages/api/src/ticketing/ticket/services/ticket.service.ts (1)
Line range hint
71-71: Consider optimizing database queries withinaddTicketto reduce the number of calls, especially for checks that could be combined.
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (7)
- packages/api/src/ticketing/comment/services/gitlab/index.ts (1 hunks)
- packages/api/src/ticketing/comment/services/gitlab/mappers.ts (1 hunks)
- packages/api/src/ticketing/comment/sync/sync.service.ts (5 hunks)
- packages/api/src/ticketing/ticket/services/gitlab/index.ts (1 hunks)
- packages/api/src/ticketing/ticket/services/gitlab/mappers.ts (1 hunks)
- packages/api/src/ticketing/ticket/services/ticket.service.ts (1 hunks)
- packages/api/src/ticketing/ticket/sync/sync.service.ts (4 hunks)
Files skipped from review as they are similar to previous changes (6)
- packages/api/src/ticketing/comment/services/gitlab/index.ts
- packages/api/src/ticketing/comment/services/gitlab/mappers.ts
- packages/api/src/ticketing/comment/sync/sync.service.ts
- packages/api/src/ticketing/ticket/services/gitlab/index.ts
- packages/api/src/ticketing/ticket/services/gitlab/mappers.ts
- packages/api/src/ticketing/ticket/sync/sync.service.ts
Additional comments not posted (2)
packages/api/src/ticketing/ticket/services/ticket.service.ts (2)
Line range hint
71-71: Ensure that theremote_dataparameter is consistently used or documented if its usage is conditional.Verification successful
The usage of the
remote_dataparameter in theaddTicketmethod ofticket.service.tshas been verified. It is consistently used and appropriately documented as an optional parameter. The parameter is involved in conditional checks and database operations, aligning with its intended usage.
- Location:
ticket.service.ts, methodaddTicketScripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of `remote_data` parameter in `addTicket` method. # Test: Search for the parameter usage. Expect: Consistent usage or conditional documentation. rg --type typescript $'remote_data' packages/api/src/ticketing/ticket/services/ticket.service.tsLength of output: 130
Script:
#!/bin/bash # Description: Re-verify the usage of `remote_data` parameter in `addTicket` method by correcting the previous script error. # Test: Search for the parameter usage. Expect: Consistent usage or conditional documentation. rg 'remote_data' packages/api/src/ticketing/ticket/services/ticket.service.tsLength of output: 744
471-471: Validate the transformation logic ingetTicketsto ensure that the field mappings are correctly converted to the desired format.Verification successful
The search results confirm the presence and usage of
fieldMappingsMapwithin thegetTicketsmethod in theticket.service.tsfile. The code snippets show thatfieldMappingsMapis initialized as a newMap, and mappings are set based on theslugof an attribute and associated data. The map is then converted into an array of objects, which likely represents the transformed data structure.This indicates that the transformation logic for field mappings is implemented as expected, converting the mappings into a desired format using a map and array conversion. This aligns with the original review comment's concern about validating the transformation logic.
Given this information, the transformation logic appears to be correctly implemented, and the field mappings are being converted to the desired format as per the snippets provided.
The transformation logic in `getTickets` for handling field mappings appears to be correctly implemented. The usage of `fieldMappingsMap` to map and convert data is evident and aligns with expected practices for data transformation in TypeScript: - **Initialization and Mapping**: A new `Map` is created, and mappings are set based on attributes and data. - **Conversion to Array**: The map is converted into an array of objects, which likely represents the final transformed structure. This confirms that the field mappings are being correctly converted to the desired format.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the transformation logic for field mappings in `getTickets`. # Test: Check the transformation logic and ensure it matches the expected format. rg --type typescript $'fieldMappingsMap' packages/api/src/ticketing/ticket/services/ticket.service.tsLength of output: 135
Script:
#!/bin/bash # Description: Re-verify the transformation logic for field mappings in `getTickets`. # Corrected Test: Check the transformation logic and ensure it matches the expected format. rg 'fieldMappingsMap' packages/api/src/ticketing/ticket/services/ticket.service.tsLength of output: 411
| @@ -468,6 +468,7 @@ export class TicketService { | |||
| completed_at: ticket.completed_at || null, | |||
| priority: ticket.priority || '', | |||
| assigned_to: ticket.assigned_to || [], | |||
| collections: ticket.collections || [], | |||
There was a problem hiding this comment.
The method getTickets should handle cases where data is not present or is outdated. Consider adding explicit error handling or data validation steps.
+ if (!tickets) {
+ throw new Error('No tickets found for the given criteria');
+ }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.
| collections: ticket.collections || [], | |
| collections: ticket.collections || [], |
Complete the implementation of updateTicket method or remove it if it's not needed. Currently, it lacks functionality.
+ // Implementation needed for updating ticketsCommittable 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.
| collections: ticket.collections || [], | |
| collections: ticket.collections || [], | |
| // Implementation needed for updating tickets |
There was a problem hiding this comment.
Actionable comments posted: 3
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- packages/shared/src/utils.ts (148 hunks)
Files not summarized due to errors (1)
- packages/shared/src/utils.ts: Error: Message exceeds token limit
Additional comments not posted (1)
packages/shared/src/utils.ts (1)
394-402: Thegitlabprovider configuration has been added and marked as active. Ensure that thescopesandurlsare correctly configured and tested, as these are critical for the integration's functionality.
| }, | ||
| 'accelo': { | ||
| scopes: '', | ||
| urls: { | ||
| docsUrl: 'https://api.accelo.com/docs/#introduction', | ||
| authBaseUrl: '/oauth2/v0/authorize', | ||
| authBaseUrl: '/oauth2/v0/authorize', |
There was a problem hiding this comment.
Ensure that the authBaseUrl for the 'accelo' provider starts with a valid protocol (http or https). Relative URLs might lead to runtime errors when constructing full URLs.
| name: providerName, | ||
| urls: { | ||
| docsUrl: config.urls.docsUrl, | ||
| apiUrl: config.urls.apiUrl, | ||
| authBaseUrl: config.urls.authBaseUrl, | ||
| }, | ||
| scopes: config.scopes, | ||
| logoPath: config.logoPath, | ||
| description: config.description, |
There was a problem hiding this comment.
The function providersArray should include error handling or logging to manage cases where the provider or vertical does not exist. This would improve debugging and maintainability.
Would you like me to add error handling or logging to this function?
There was a problem hiding this comment.
Actionable comments posted: 1
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- apps/client-ts/src/components/Connection/columns.tsx (2 hunks)
- apps/client-ts/src/components/Events/columns.tsx (2 hunks)
- packages/shared/src/utils.ts (148 hunks)
Files not summarized due to errors (1)
- packages/shared/src/utils.ts: Error: Message exceeds token limit
Files skipped from review due to trivial changes (1)
- apps/client-ts/src/components/Events/columns.tsx
Additional comments not posted (5)
apps/client-ts/src/components/Connection/columns.tsx (3)
13-13: Import ofgetLogoURLis correctly placed and used in theappcolumn definition.
116-116: The use ofgetLogoURLto dynamically fetch logos based on the provider enhances the UI's adaptability and maintainability.
116-116: The dynamic rendering of the app logo and name in theappcolumn is well-implemented, enhancing the visual appeal and informational value of the table.packages/shared/src/utils.ts (2)
111-116: Ensure that theauthBaseUrlfor the 'accelo' provider starts with a valid protocol (http or https).This issue was previously flagged and remains valid. Relative URLs might lead to runtime errors when constructing full URLs.
2164-2172: Consider adding error handling or logging to manage cases where the provider or vertical does not exist.This would improve debugging and maintainability. Would you like me to add error handling or logging to this function?
| customPropertiesUrl?: string; | ||
| authStrategy?: AuthStrategy; | ||
| urls: { | ||
| docsUrl: string; | ||
| apiUrl: string; | ||
| authBaseUrl?: string; // url used to authorize an application on behalf of the user (only when authStrategy is oauth2) | ||
| customPropertiesUrl?: string; | ||
| customPropertiesUrl?: string; |
There was a problem hiding this comment.
Ensure consistent handling of customPropertiesUrl across different provider configurations.
It's important to ensure that the customPropertiesUrl is consistently handled across all provider configurations. Consider adding validation or a default value to manage cases where this property might be omitted.
|
@mit-27 I think you have missed the below issues.
|
|
Hi @rajeshj11,thank you for addressing the issues. However, we will handle pagination later, and we have also created an issue for it. If you would like to work on it, feel free to pick it up once it is merged. |
There was a problem hiding this comment.
Actionable comments posted: 0
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- packages/api/src/@core/connections/ticketing/services/gitlab/gitlab.service.ts (7 hunks)
Files skipped from review as they are similar to previous changes (1)
- packages/api/src/@core/connections/ticketing/services/gitlab/gitlab.service.ts
