feat: Add integration with: OneDrive #692
Conversation
…e-drive integration
|
Qovery can create a Preview Environment for this PR.
This comment has been generated from Qovery AI 🤖.
|
|
|
|
WalkthroughWalkthroughThis pull request introduces a comprehensive integration with OneDrive, enhancing the application's file storage capabilities. Key changes include the addition of environment variables for OneDrive authentication, modifications to the Prisma schema to support project pull frequency, and the implementation of various services and mappers to facilitate interactions with OneDrive's API for managing drives, files, folders, groups, users, permissions, and shared links. Changes
Possibly related issues
Possibly related PRs
Suggested labels
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 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: 25
Outside diff range comments (1)
packages/api/src/filestorage/sharedlink/sharedlink.module.ts (1)
Line range hint
1-40: Confirm imports and address duplicate entry.The imports for
OnedriveSharedLinkMapper,OnedriveService, andBoxServiceare correctly added to support the new functionalities. However, there is a duplicate entry forBoxSharedLinkMapperin the providers list which might be an oversight.Please consider removing the duplicate entry:
- BoxSharedLinkMapper, - BoxService, - OnedriveService, - BoxSharedLinkMapper, + BoxService, + OnedriveService, + BoxSharedLinkMapper, + OnedriveSharedLinkMapper,
Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Files ignored due to path filters (1)
packages/api/swagger/swagger-spec.yamlis excluded by!**/*.yaml
Files selected for processing (36)
- .env.example (1 hunks)
- packages/api/prisma/schema.prisma (2 hunks)
- packages/api/scripts/connectorUpdate.js (5 hunks)
- packages/api/src/@core/sync/sync.service.ts (1 hunks)
- packages/api/src/@core/utils/types/original/original.file-storage.ts (4 hunks)
- packages/api/src/filestorage/drive/drive.module.ts (2 hunks)
- packages/api/src/filestorage/drive/services/onedrive/index.ts (1 hunks)
- packages/api/src/filestorage/drive/services/onedrive/mappers.ts (1 hunks)
- packages/api/src/filestorage/drive/services/onedrive/types.ts (1 hunks)
- packages/api/src/filestorage/file/file.module.ts (2 hunks)
- packages/api/src/filestorage/file/services/onedrive/index.ts (1 hunks)
- packages/api/src/filestorage/file/services/onedrive/mappers.ts (1 hunks)
- packages/api/src/filestorage/file/services/onedrive/types.ts (1 hunks)
- packages/api/src/filestorage/folder/folder.module.ts (2 hunks)
- packages/api/src/filestorage/folder/services/onedrive/index.ts (1 hunks)
- packages/api/src/filestorage/folder/services/onedrive/mappers.ts (1 hunks)
- packages/api/src/filestorage/folder/services/onedrive/types.ts (1 hunks)
- packages/api/src/filestorage/group/group.module.ts (2 hunks)
- packages/api/src/filestorage/group/services/onedrive/index.ts (1 hunks)
- packages/api/src/filestorage/group/services/onedrive/mappers.ts (1 hunks)
- packages/api/src/filestorage/group/services/onedrive/types.ts (1 hunks)
- packages/api/src/filestorage/permission/permission.module.ts (2 hunks)
- packages/api/src/filestorage/permission/services/onedrive/index.ts (1 hunks)
- packages/api/src/filestorage/permission/services/onedrive/mappers.ts (1 hunks)
- packages/api/src/filestorage/permission/services/onedrive/types.ts (1 hunks)
- packages/api/src/filestorage/sharedlink/services/onedrive/index.ts (1 hunks)
- packages/api/src/filestorage/sharedlink/services/onedrive/mappers.ts (1 hunks)
- packages/api/src/filestorage/sharedlink/services/onedrive/types.ts (1 hunks)
- packages/api/src/filestorage/sharedlink/sharedlink.module.ts (2 hunks)
- packages/api/src/filestorage/user/services/onedrive/index.ts (1 hunks)
- packages/api/src/filestorage/user/services/onedrive/mappers.ts (1 hunks)
- packages/api/src/filestorage/user/services/onedrive/types.ts (1 hunks)
- packages/api/src/filestorage/user/user.module.ts (2 hunks)
- packages/shared/src/connectors/enum.ts (1 hunks)
- packages/shared/src/connectors/index.ts (1 hunks)
- packages/shared/src/connectors/metadata.ts (1 hunks)
Additional context used
Biome
packages/api/src/filestorage/group/services/onedrive/index.ts
[error] 22-22: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
[error] 54-54: The catch clause that only rethrows the original error is redundant.
These unnecessary catch clauses can be confusing. It is recommended to remove them.
(lint/complexity/noUselessCatch)
packages/api/src/filestorage/drive/services/onedrive/index.ts
[error] 67-67: The catch clause that only rethrows the original error is redundant.
These unnecessary catch clauses can be confusing. It is recommended to remove them.
(lint/complexity/noUselessCatch)
packages/api/src/filestorage/permission/services/onedrive/index.ts
[error] 22-22: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
[error] 77-77: The catch clause that only rethrows the original error is redundant.
These unnecessary catch clauses can be confusing. It is recommended to remove them.
(lint/complexity/noUselessCatch)
packages/api/src/filestorage/sharedlink/services/onedrive/index.ts
[error] 23-23: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
[error] 78-78: The catch clause that only rethrows the original error is redundant.
These unnecessary catch clauses can be confusing. It is recommended to remove them.
(lint/complexity/noUselessCatch)
packages/api/src/filestorage/file/services/onedrive/index.ts
[error] 89-89: The catch clause that only rethrows the original error is redundant.
These unnecessary catch clauses can be confusing. It is recommended to remove them.
(lint/complexity/noUselessCatch)
packages/api/src/filestorage/file/services/onedrive/mappers.ts
[error] 46-46: Use Number.parseInt instead of the equivalent global.
ES2015 moved some globals into the Number namespace for consistency.
Safe fix: Use Number.parseInt instead.(lint/style/useNumberNamespace)
packages/api/src/filestorage/folder/services/onedrive/index.ts
[error] 69-69: The catch clause that only rethrows the original error is redundant.
These unnecessary catch clauses can be confusing. It is recommended to remove them.
(lint/complexity/noUselessCatch)
[error] 86-88: Declare variables separately
Unsafe fix: Break out into multiple declarations
(lint/style/useSingleVarDeclarator)
[error] 160-160: The catch clause that only rethrows the original error is redundant.
These unnecessary catch clauses can be confusing. It is recommended to remove them.
(lint/complexity/noUselessCatch)
packages/api/scripts/connectorUpdate.js
[error] 268-271: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
[error] 420-420: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
Additional comments not posted (66)
packages/shared/src/connectors/index.ts (1)
7-7: Approved: Addition of 'onedrive' to FILESTORAGE_PROVIDERS.The addition of 'onedrive' to the
FILESTORAGE_PROVIDERSarray is consistent with the PR's objective to integrate OneDrive. Ensure that this integration is tested thoroughly.packages/shared/src/connectors/enum.ts (1)
30-30: Approved: Addition of 'ONEDRIVE' to FilestorageConnectors enum.The addition of 'ONEDRIVE' to the
FilestorageConnectorsenum is consistent with the PR's objective to integrate OneDrive. Ensure that this integration is tested thoroughly.packages/api/src/filestorage/drive/drive.module.ts (1)
Line range hint
1-23: Approved: Addition of OneDrive services and mappers.The addition of
OnedriveDriveMapperandOnedriveServiceto theDriveModuleis consistent with the PR's objective to integrate OneDrive. Ensure that these new components are properly registered and tested.Verification successful
Integration Verified: OnedriveDriveMapper and OnedriveService
The
OnedriveDriveMapperandOnedriveServiceare properly registered and used within theDriveModule, as well as across multiple other modules in the codebase. This confirms their successful integration and consistency with the application's architecture. No issues were found.
DriveModuleimports and registers bothOnedriveDriveMapperandOnedriveService.OnedriveServiceis implemented and used in several other modules, indicating its core role in the application.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that `OnedriveDriveMapper` and `OnedriveService` are properly registered and used within the `DriveModule`. # Test: Search for usage of `OnedriveDriveMapper` and `OnedriveService`. Expect: Proper registration and usage. rg --type typescript -A 5 $'OnedriveDriveMapper|OnedriveService'Length of output: 100
Script:
#!/bin/bash # Description: Verify that `OnedriveDriveMapper` and `OnedriveService` are properly registered and used within the `DriveModule`. # Test: Search for usage of `OnedriveDriveMapper` and `OnedriveService` in TypeScript files. Expect: Proper registration and usage. rg --glob '*.ts' -A 5 'OnedriveDriveMapper|OnedriveService'Length of output: 17243
packages/api/src/filestorage/file/file.module.ts (2)
1-2: Import statements are correct.The import paths for
OnedriveFileMapperandOnedriveServiceare correctly specified.
29-30: Provider additions are correct.
OnedriveServiceandOnedriveFileMapperhave been correctly added to the providers array. Ensure that integration tests cover their instantiation and usage to verify that they are properly integrated.packages/api/src/filestorage/user/user.module.ts (2)
1-2: Import statements are correct.The import paths for
OnedriveUserMapperandOnedriveServiceare correctly specified.
28-29: Provider additions are correct.
OnedriveServiceandOnedriveUserMapperhave been correctly added to the providers array. Ensure that integration tests cover their instantiation and usage to verify that they are properly integrated.packages/api/src/filestorage/group/group.module.ts (2)
1-2: Import statements are correct.The import paths for
OnedriveGroupMapperandOnedriveServiceare correctly specified.
29-30: Provider additions are correct.
OnedriveServiceandOnedriveGroupMapperhave been correctly added to the providers array. Ensure that integration tests cover their instantiation and usage to verify that they are properly integrated.packages/api/src/filestorage/folder/folder.module.ts (2)
1-2: Imports are correctly placed.The import statements for
OnedriveFolderMapperandOnedriveServiceare correctly structured and follow the project's organization.
29-30: Providers correctly added.
OnedriveServiceandOnedriveFolderMapperhave been correctly added to the providers list, enabling their use within the module. Ensure that these services are properly integrated and tested.Run the following script to verify the integration of these services:
Verification successful
Integration Verified: OnedriveService and OnedriveFolderMapper
The
OnedriveServiceandOnedriveFolderMapperare well-integrated across multiple modules in the codebase, indicating proper usage and functionality. Their presence in various modules suggests they are part of a cohesive system architecture. No further action is required.
OnedriveServiceis used in modules such asfile,user,permission,sharedlink,folder,group, anddrive.OnedriveFolderMapperis specifically used in thefoldermodule.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the integration of `OnedriveService` and `OnedriveFolderMapper`. # Test: Search for the usage of these services in the project. Expect: Proper integration and usage. rg --type typescript -A 5 $'OnedriveService|OnedriveFolderMapper'Length of output: 101
Script:
#!/bin/bash # Description: Verify the integration of `OnedriveService` and `OnedriveFolderMapper`. # Test: Search for the usage of these services in the project. Expect: Proper integration and usage. rg --type ts -A 5 $'OnedriveService|OnedriveFolderMapper'Length of output: 17251
packages/api/src/filestorage/permission/permission.module.ts (2)
1-2: Imports are correctly placed.The import statements for
OnedrivePermissionMapperandOnedriveServiceare correctly structured and follow the project's organization.
34-35: Providers correctly added.
OnedriveServiceandOnedrivePermissionMapperhave been correctly added to the providers list, enabling their use within the module. Ensure that these services are properly integrated and tested.Run the following script to verify the integration of these services:
Verification successful
Integration Verified: OnedriveService and OnedrivePermissionMapper
The
OnedriveServiceandOnedrivePermissionMapperare well-integrated across multiple modules in the codebase, confirming their proper addition to the providers list. This integration supports various OneDrive functionalities throughout the application.
OnedriveServiceis used in modules such asgroup,user,sharedlink,permission,folder,file, anddrive.OnedrivePermissionMapperis specifically utilized in thepermissionmodule.These findings confirm that the services are correctly integrated and utilized.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the integration of `OnedriveService` and `OnedrivePermissionMapper`. # Test: Search for the usage of these services in the project. Expect: Proper integration and usage. rg --type typescript -A 5 $'OnedriveService|OnedrivePermissionMapper'Length of output: 105
Script:
#!/bin/bash # Description: Verify the integration of `OnedriveService` and `OnedrivePermissionMapper`. # Test: Search for the usage of these services in the project. Expect: Proper integration and usage. rg --type ts -A 5 $'OnedriveService|OnedrivePermissionMapper'Length of output: 17308
packages/api/src/filestorage/sharedlink/services/onedrive/types.ts (1)
1-29: Type definitions are well-documented and correctly implemented.The
OnedriveSharedLinkInputinterface andOnedriveSharedLinkOutputtype are well-defined. The documentation is thorough, referencing the official Microsoft Graph API, which enhances maintainability and clarity.packages/api/src/filestorage/user/services/onedrive/index.ts (1)
13-62: Well-implemented user service for OneDrive.The
OnedriveServiceclass for user management is well-implemented with clear dependency injections and effective use of modern JavaScript practices. The detailed error handling and logging are commendable.packages/api/src/filestorage/user/services/onedrive/mappers.ts (2)
11-20: Well-structured service registration.The
OnedriveUserMapperclass is properly decorated with@Injectable()and correctly registers itself in the service registry. This is a good practice for maintaining modularity and ensuring that the service can be easily managed and injected where needed.
32-53: Efficient and flexible implementation ofunifymethod.The
unifymethod is well-implemented, handling both single and array inputs efficiently using asynchronous operations. This method ensures that the service can adapt to different scenarios where either individual user data or batches of user data need to be processed.packages/api/src/filestorage/drive/services/onedrive/index.ts (1)
15-27: Well-structured service registration.The
OnedriveServiceclass is properly decorated with@Injectable()and correctly registers itself in the service registry. This is a good practice for maintaining modularity and ensuring that the service can be easily managed and injected where needed.packages/api/src/filestorage/group/services/onedrive/mappers.ts (2)
11-20: Well-structured service registration.The
OnedriveGroupMapperclass is properly decorated with@Injectable()and correctly registers itself in the service registry. This is a good practice for maintaining modularity and ensuring that the service can be easily managed and injected where needed.
32-53: Efficient and flexible implementation ofunifymethod.The
unifymethod is well-implemented, handling both single and array inputs efficiently using asynchronous operations. This method ensures that the service can adapt to different scenarios where either individual group data or batches of group data need to be processed.packages/api/src/filestorage/drive/services/onedrive/mappers.ts (4)
12-25: Class setup and registration are correctly implemented.The
OnedriveDriveMapperclass is properly set up with dependency injection and is correctly registered in the service registry. This follows NestJS best practices for modularity and service availability.
27-35: Clarification needed on thedesunifymethod implementation.The
desunifymethod is currently not implemented. If this is intentional (e.g., a placeholder for future implementation), please add a comment to clarify. Otherwise, consider implementing the method or removing it if it's not needed.
37-58: Well-implementedunifymethod.The
unifymethod effectively handles both single and array inputs, using appropriate asynchronous patterns. The delegation tomapSingleDriveToUnifiedfor individual item processing is a clean and efficient approach.
60-85: Effective mapping inmapSingleDriveToUnified.The
mapSingleDriveToUnifiedmethod is well-implemented, effectively handling both standard and custom field mappings. The structured approach to building the result object ensures that all necessary data is correctly transformed.packages/api/src/filestorage/permission/services/onedrive/mappers.ts (2)
12-24: Class structure and registration logic are well-implemented.The
OnedrivePermissionMapperclass is correctly set up with dependency injection and registers itself in themappersRegistry. This setup ensures that it is properly integrated into the application's service architecture.
36-63: Efficient handling of single and array inputs inunify.The
unifymethod efficiently handles both single and array inputs ofOnedrivePermissionOutput. The use ofPromise.allfor array inputs is appropriate and ensures that all permissions are processed in parallel, which can improve performance.packages/api/src/filestorage/sharedlink/services/onedrive/mappers.ts (2)
14-27: Class structure and registration logic are well-implemented.The
OnedriveSharedLinkMapperclass is correctly set up with dependency injection and registers itself in themappersRegistry. This setup ensures that it is properly integrated into the application's service architecture.
39-66: Efficient handling of single and array inputs inunify.The
unifymethod efficiently handles both single and array inputs ofOnedriveSharedLinkOutput. The use ofPromise.allfor array inputs is appropriate and ensures that all shared links are processed in parallel, which can improve performance.packages/api/src/@core/utils/types/original/original.file-storage.ts (10)
60-60: Type modification forOriginalFileInputis appropriate.The inclusion of
OnedriveFileInputalongsideBoxFileInputin theOriginalFileInputtype is a sensible enhancement that broadens the system's compatibility with multiple cloud storage services.
63-63: Type modification forOriginalFolderInputis appropriate.The inclusion of
OnedriveFolderInputalongsideBoxFolderInputin theOriginalFolderInputtype is a sensible enhancement that broadens the system's compatibility with multiple cloud storage services.
66-66: Type modification forOriginalPermissionInputis appropriate.The inclusion of
OnedrivePermissionInputalongside the existing unspecified type in theOriginalPermissionInputtype is a sensible enhancement that broadens the system's compatibility with multiple cloud storage services.
72-72: Type modification forOriginalDriveInputis appropriate.The inclusion of
OnedriveDriveInputalongside the existing unspecified type in theOriginalDriveInputtype is a sensible enhancement that broadens the system's compatibility with multiple cloud storage services.
75-75: Type modification forOriginalGroupInputis appropriate.The inclusion of
OnedriveGroupInputalongsideBoxGroupInputin theOriginalGroupInputtype is a sensible enhancement that broadens the system's compatibility with multiple cloud storage services.
78-78: Type modification forOriginalUserInputis appropriate.The inclusion of
OnedriveUserInputalongsideBoxUserInputin theOriginalUserInputtype is a sensible enhancement that broadens the system's compatibility with multiple cloud storage services.
92-92: Type modification forOriginalFileOutputis appropriate.The inclusion of
OnedriveFileOutputalongsideBoxFileOutputin theOriginalFileOutputtype is a sensible enhancement that broadens the system's compatibility with multiple cloud storage services.
95-95: Type modification forOriginalFolderOutputis appropriate.The inclusion of
OnedriveFolderOutputalongsideBoxFolderOutputin theOriginalFolderOutputtype is a sensible enhancement that broadens the system's compatibility with multiple cloud storage services.
98-98: Type modification forOriginalPermissionOutputis appropriate.The inclusion of
OnedrivePermissionOutputalongside the existing unspecified type in theOriginalPermissionOutputtype is a sensible enhancement that broadens the system's compatibility with multiple cloud storage services.
104-104: Type modification forOriginalDriveOutputis appropriate.The inclusion of
OnedriveDriveOutputalongside the existing unspecified type in theOriginalDriveOutputtype is a sensible enhancement that broadens the system's compatibility with multiple cloud storage services.packages/api/src/filestorage/file/services/onedrive/index.ts (1)
13-25: Review: Constructor ImplementationThe constructor correctly injects required services and registers
OnedriveServicewith theServiceRegistry. This is a good use of dependency injection and service registration patterns in NestJS.packages/api/src/filestorage/file/services/onedrive/mappers.ts (1)
18-31: Review: Constructor ImplementationThe constructor correctly registers the
OnedriveFileMapperservice with theMappersRegistry. This is a good practice in service-oriented architecture, ensuring that the mapper is discoverable and can be reused across the application..env.example (2)
113-113: Review: Addition ofONEDRIVE_FILESTORAGE_CLOUD_CLIENT_IDThe addition of the
ONEDRIVE_FILESTORAGE_CLOUD_CLIENT_IDenvironment variable is correctly formatted and placed within the file storage section. This ensures consistency and ease of configuration for integrating with OneDrive.
114-114: Review: Addition ofONEDRIVE_FILESTORAGE_CLOUD_CLIENT_SECRETThe addition of the
ONEDRIVE_FILESTORAGE_CLOUD_CLIENT_SECRETenvironment variable is correctly formatted and placed within the file storage section. This ensures consistency and ease of configuration for integrating with OneDrive.packages/api/src/filestorage/folder/services/onedrive/mappers.ts (2)
16-29: Constructor implementation is appropriate.The constructor correctly handles dependency injection and service registration, which are good practices for modularity and testability.
31-55:desunifymethod implementation is correct.The method effectively handles the conversion from a unified model to a OneDrive-specific model, including custom field mappings.
packages/api/src/filestorage/drive/services/onedrive/types.ts (5)
1-32:OnedriveDriveOutputinterface is correctly defined.The interface accurately represents the expected structure from the OneDrive API, with appropriate use of readonly properties to ensure immutability.
34-59:IdentitySetinterface is comprehensive and flexible.The interface covers various identity aspects with appropriate optional markings, providing flexibility in handling different data availability scenarios.
61-74:Identityinterface is suitably defined.The interface is simple and flexible, correctly marking properties as optional to accommodate various data availability scenarios.
76-92:Quotainterface is accurately defined.The interface correctly represents drive quota information with readonly properties to ensure data integrity.
102-120:SharepointIdsinterface is well-defined and specific.The interface provides SharePoint-specific identifiers with readonly properties, ensuring that they are not modified after being set.
packages/api/src/filestorage/permission/services/onedrive/types.ts (4)
7-30:OnedrivePermissionOutputinterface is comprehensive and well-documented.The interface effectively represents the structure for permissions associated with OneDrive items, with appropriate use of optional properties to handle various data availability scenarios.
32-43:SharingInvitationinterface is correctly defined.The interface provides detailed structure for sharing invitations with appropriate use of readonly properties to maintain data integrity.
45-62:SharingLinkinterface is suitably defined.The interface is well-documented and flexible, correctly marking properties as optional to accommodate various data availability scenarios.
64-73:SharePointIdentitySetinterface is appropriately extended.The interface extends
IdentitySetto include SharePoint-specific details, effectively representing SharePoint identities associated with permissions.packages/api/src/filestorage/folder/services/onedrive/types.ts (4)
11-58: Well-defined TypeScript interface for OneDrive folder input.The
OnedriveFolderInputinterface is comprehensive and aligns well with the OneDrive API specifications. The use of optional and readonly properties is appropriate, ensuring flexibility and immutability where necessary.
63-68: Concise and effective interface for folder metadata.The
Folderinterface is succinct yet effective, capturing essential metadata about a folder with flexibility provided by the optionalviewproperty.
73-80: Appropriately defined interface for client file system information.The
FileSystemInfointerface correctly captures essential timestamps related to file operations on a client, with properties marked asreadonlyto prevent modifications, which is suitable for maintaining the integrity of file metadata.
85-99: Detailed and flexible interface for folder view settings.The
FolderViewinterface provides comprehensive options for customizing the display of folder contents, including sorting and view types. The use of enums for these properties ensures that the settings are both explicit and constrained to valid values.packages/api/src/filestorage/file/services/onedrive/types.ts (1)
1-210: Comprehensive and well-documented type definitions for OneDrive integration.The file provides a robust set of TypeScript interfaces and types that align well with the Microsoft Graph API for OneDrive. Each type is well-documented with links to the official documentation, which enhances understandability and maintainability. The use of
readonlyfor immutable fields is correctly applied, ensuring that these properties are not modified after their initial set up.Overall, the definitions are consistent and appear to be complete with respect to the described functionality in the PR summary. This should provide a solid foundation for the OneDrive integration within the application.
packages/api/src/filestorage/user/services/onedrive/types.ts (1)
1-203: Detailed and well-structured user type definitions for OneDrive integration.This file effectively outlines the necessary user-related data structures for the OneDrive integration, adhering closely to the specifications of the Microsoft Graph API. The interfaces are well-documented, and the use of TypeScript features like
readonlyand optional fields (?) is appropriately applied to match the expected behavior of these data entities.The thorough documentation and adherence to the external API's structure facilitate a clear understanding of how user data will be managed and interacted with within the system. This setup should support robust integration with OneDrive services, particularly in handling user information.
packages/api/prisma/schema.prisma (2)
1968-1980: Approve the addition ofprojects_pull_frequencyto theprojectsmodel.The addition of the
projects_pull_frequencyfield enhances the schema's capability to manage project-related pull frequency settings effectively. This change is well-aligned with the overall goal of improving data structure flexibility and functionality.
2133-2146: Approve the new modelprojects_pull_frequency.The introduction of the
projects_pull_frequencymodel is a significant enhancement, providing a structured way to manage pull frequencies for various systems. The model includes essential fields for different systems and timestamps, which are crucial for maintaining up-to-date information.packages/shared/src/connectors/metadata.ts (1)
2740-2740: Enable OneDrive ConnectorThe change to set the
activeproperty of theonedriveconnector totrueis crucial as it enables the integration with OneDrive. This change should be carefully tested to ensure that the integration works as expected and does not introduce any security or performance issues.Run the following script to verify the integration with OneDrive:
Verification successful
Enable OneDrive Connector
The
activeproperty for theonedriveconnector is correctly set totrue, enabling the integration. The configuration includes necessary properties such asscopes,urls, andauthStrategy, indicating readiness for integration. Ensure thorough testing to confirm functionality and address any potential issues.
- File:
packages/shared/src/connectors/metadata.ts- Line: 2740
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the integration with OneDrive by checking the active status and related configurations. # Test: Check if the active status is true and if all required properties are correctly set. rg --type json -A 5 $'onedrive'Length of output: 30
Script:
#!/bin/bash # Description: Verify the integration with OneDrive by checking the active status and related configurations in the TypeScript file. # Test: Check if the active status is true and if all required properties are correctly set in the TypeScript file. rg 'onedrive' -A 5 packages/shared/src/connectors/metadata.tsLength of output: 700
packages/api/src/filestorage/permission/services/onedrive/index.ts (1)
13-25: Review of Constructor in OnedriveServiceThe constructor is well-structured with dependency injection for services like
PrismaService,LoggerService,EncryptionService, andServiceRegistry. It also correctly sets the logger context and registers the service, which are good practices for maintainability and modularity.Tools
Biome
[error] 22-22: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
packages/api/src/filestorage/sharedlink/services/onedrive/index.ts (1)
14-26: Review of Constructor in OnedriveService for Shared LinksThe constructor effectively uses dependency injection and service registration. It sets the logger context appropriately, which aids in debugging and maintenance.
Tools
Biome
[error] 23-23: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
packages/api/src/filestorage/folder/services/onedrive/index.ts (1)
17-30: Review of Constructor in OnedriveService for FoldersThe constructor is correctly implemented with dependency injection for necessary services and effective service registration. It sets the logger context using template literals, which is a good practice.
packages/api/src/filestorage/group/services/onedrive/types.ts (1)
130-143: Base interface for directory objects is succinct and appropriate.The
DirectoryObjectinterface is simple and effective, providing just enough structure for identification and handling of deletion state. This minimalistic approach is suitable for the intended use case. However, consider adding more properties if the directory objects in your application context have more relevant data that could be encapsulated within this interface.
| @Injectable() | ||
| export class OnedriveService implements IGroupService { | ||
| constructor( | ||
| private prisma: PrismaService, | ||
| private logger: LoggerService, | ||
| private cryptoService: EncryptionService, | ||
| private registry: ServiceRegistry, | ||
| ) { | ||
| this.logger.setContext( | ||
| FileStorageObject.group.toUpperCase() + ':' + OnedriveService.name, | ||
| ); | ||
| this.registry.registerService('onedrive', this); | ||
| } | ||
|
|
||
| async sync(data: SyncParam): Promise<ApiResponse<OnedriveGroupOutput[]>> { | ||
| try { | ||
| const { linkedUserId } = data; | ||
| const connection = await this.prisma.connections.findFirst({ | ||
| where: { | ||
| id_linked_user: linkedUserId, | ||
| provider_slug: 'onedrive', | ||
| vertical: 'filestorage', | ||
| }, | ||
| }); | ||
| const resp = await axios.get(`${connection.account_url}/v1.0/groups`, { | ||
| headers: { | ||
| 'Content-Type': 'application/json', | ||
| Authorization: `Bearer ${this.cryptoService.decrypt( | ||
| connection.access_token, | ||
| )}`, | ||
| }, | ||
| }); | ||
|
|
||
| this.logger.log(`Synced onedrive groups !`); | ||
|
|
||
| return { | ||
| data: resp.data.value, | ||
| message: 'Onedrive groups retrieved', | ||
| statusCode: 200, | ||
| }; | ||
| } catch (error) { | ||
| throw error; | ||
| } | ||
| } |
There was a problem hiding this comment.
Improve code quality with template literals and simplify error handling.
- Template Literals: Replace string concatenation with template literals for better readability and consistency with modern JavaScript practices.
- Redundant Catch: The catch clause that only rethrows the error is redundant and can be removed to simplify the error handling.
- FileStorageObject.group.toUpperCase() + ':' + OnedriveService.name,
+ `${FileStorageObject.group.toUpperCase()}:${OnedriveService.name}`,
- } catch (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.
| @Injectable() | |
| export class OnedriveService implements IGroupService { | |
| constructor( | |
| private prisma: PrismaService, | |
| private logger: LoggerService, | |
| private cryptoService: EncryptionService, | |
| private registry: ServiceRegistry, | |
| ) { | |
| this.logger.setContext( | |
| FileStorageObject.group.toUpperCase() + ':' + OnedriveService.name, | |
| ); | |
| this.registry.registerService('onedrive', this); | |
| } | |
| async sync(data: SyncParam): Promise<ApiResponse<OnedriveGroupOutput[]>> { | |
| try { | |
| const { linkedUserId } = data; | |
| const connection = await this.prisma.connections.findFirst({ | |
| where: { | |
| id_linked_user: linkedUserId, | |
| provider_slug: 'onedrive', | |
| vertical: 'filestorage', | |
| }, | |
| }); | |
| const resp = await axios.get(`${connection.account_url}/v1.0/groups`, { | |
| headers: { | |
| 'Content-Type': 'application/json', | |
| Authorization: `Bearer ${this.cryptoService.decrypt( | |
| connection.access_token, | |
| )}`, | |
| }, | |
| }); | |
| this.logger.log(`Synced onedrive groups !`); | |
| return { | |
| data: resp.data.value, | |
| message: 'Onedrive groups retrieved', | |
| statusCode: 200, | |
| }; | |
| } catch (error) { | |
| throw error; | |
| } | |
| } | |
| @Injectable() | |
| export class OnedriveService implements IGroupService { | |
| constructor( | |
| private prisma: PrismaService, | |
| private logger: LoggerService, | |
| private cryptoService: EncryptionService, | |
| private registry: ServiceRegistry, | |
| ) { | |
| this.logger.setContext( | |
| `${FileStorageObject.group.toUpperCase()}:${OnedriveService.name}`, | |
| ); | |
| this.registry.registerService('onedrive', this); | |
| } | |
| async sync(data: SyncParam): Promise<ApiResponse<OnedriveGroupOutput[]>> { | |
| try { | |
| const { linkedUserId } = data; | |
| const connection = await this.prisma.connections.findFirst({ | |
| where: { | |
| id_linked_user: linkedUserId, | |
| provider_slug: 'onedrive', | |
| vertical: 'filestorage', | |
| }, | |
| }); | |
| const resp = await axios.get(`${connection.account_url}/v1.0/groups`, { | |
| headers: { | |
| 'Content-Type': 'application/json', | |
| Authorization: `Bearer ${this.cryptoService.decrypt( | |
| connection.access_token, | |
| )}`, | |
| }, | |
| }); | |
| this.logger.log(`Synced onedrive groups !`); | |
| return { | |
| data: resp.data.value, | |
| message: 'Onedrive groups retrieved', | |
| statusCode: 200, | |
| }; | |
| } | |
| } |
Tools
Biome
[error] 22-22: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
[error] 54-54: The catch clause that only rethrows the original error is redundant.
These unnecessary catch clauses can be confusing. It is recommended to remove them.
(lint/complexity/noUselessCatch)
| async desunify( | ||
| source: UnifiedFilestorageUserInput, | ||
| customFieldMappings?: { | ||
| slug: string; | ||
| remote_id: string; | ||
| }[], | ||
| ): Promise<OnedriveUserInput> { | ||
| return; | ||
| } |
There was a problem hiding this comment.
Incomplete implementation of desunify method.
The desunify method currently does not perform any operations and returns nothing. This seems like an incomplete implementation. Please ensure that the method correctly maps the UnifiedFilestorageUserInput to OnedriveUserInput based on the provided customFieldMappings.
| async addDrive( | ||
| driveData: DesunifyReturnType, | ||
| linkedUserId: string, | ||
| ): Promise<ApiResponse<OriginalDriveOutput>> { | ||
| // No API to add drive in onedrive | ||
| return; | ||
| } |
There was a problem hiding this comment.
Clarify or remove the addDrive method.
The addDrive method currently does nothing, with a comment indicating that there is no API to add a drive in OneDrive. If this functionality is not needed, consider removing the method. Otherwise, enhance the documentation to explain why this method is present and what its future intentions might be.
| async sync(data: SyncParam): Promise<ApiResponse<OnedriveDriveOutput[]>> { | ||
| try { | ||
| const { linkedUserId } = data; | ||
|
|
||
| const connection = await this.prisma.connections.findFirst({ | ||
| where: { | ||
| id_linked_user: linkedUserId, | ||
| provider_slug: 'onedrive', | ||
| vertical: 'filestorage', | ||
| }, | ||
| }); | ||
|
|
||
| const resp = await axios.get(`${connection.account_url}/v1.0/drives`, { | ||
| headers: { | ||
| 'Content-Type': 'application/json', | ||
| Authorization: `Bearer ${this.cryptoService.decrypt( | ||
| connection.access_token, | ||
| )}`, | ||
| }, | ||
| }); | ||
|
|
||
| const drives: OnedriveDriveOutput[] = resp.data.value; | ||
| this.logger.log(`Synced onedrive drives !`); | ||
|
|
||
| return { | ||
| data: drives, | ||
| message: 'Onedrive drives retrived', | ||
| statusCode: 200, | ||
| }; | ||
| } catch (error) { | ||
| throw error; | ||
| } | ||
| } |
There was a problem hiding this comment.
Optimize error handling in sync method.
The sync method is well-implemented for retrieving OneDrive drives. However, the catch block that simply rethrows the error is redundant and could be removed to clean up the code.
Apply this diff to remove the redundant catch block:
- } catch (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 sync(data: SyncParam): Promise<ApiResponse<OnedriveDriveOutput[]>> { | |
| try { | |
| const { linkedUserId } = data; | |
| const connection = await this.prisma.connections.findFirst({ | |
| where: { | |
| id_linked_user: linkedUserId, | |
| provider_slug: 'onedrive', | |
| vertical: 'filestorage', | |
| }, | |
| }); | |
| const resp = await axios.get(`${connection.account_url}/v1.0/drives`, { | |
| headers: { | |
| 'Content-Type': 'application/json', | |
| Authorization: `Bearer ${this.cryptoService.decrypt( | |
| connection.access_token, | |
| )}`, | |
| }, | |
| }); | |
| const drives: OnedriveDriveOutput[] = resp.data.value; | |
| this.logger.log(`Synced onedrive drives !`); | |
| return { | |
| data: drives, | |
| message: 'Onedrive drives retrived', | |
| statusCode: 200, | |
| }; | |
| } catch (error) { | |
| throw error; | |
| } | |
| } | |
| async sync(data: SyncParam): Promise<ApiResponse<OnedriveDriveOutput[]>> { | |
| const { linkedUserId } = data; | |
| const connection = await this.prisma.connections.findFirst({ | |
| where: { | |
| id_linked_user: linkedUserId, | |
| provider_slug: 'onedrive', | |
| vertical: 'filestorage', | |
| }, | |
| }); | |
| const resp = await axios.get(`${connection.account_url}/v1.0/drives`, { | |
| headers: { | |
| 'Content-Type': 'application/json', | |
| Authorization: `Bearer ${this.cryptoService.decrypt( | |
| connection.access_token, | |
| )}`, | |
| }, | |
| }); | |
| const drives: OnedriveDriveOutput[] = resp.data.value; | |
| this.logger.log(`Synced onedrive drives !`); | |
| return { | |
| data: drives, | |
| message: 'Onedrive drives retrived', | |
| statusCode: 200, | |
| }; | |
| } |
Tools
Biome
[error] 67-67: The catch clause that only rethrows the original error is redundant.
These unnecessary catch clauses can be confusing. It is recommended to remove them.
(lint/complexity/noUselessCatch)
| async desunify( | ||
| source: UnifiedFilestorageGroupInput, | ||
| customFieldMappings?: { | ||
| slug: string; | ||
| remote_id: string; | ||
| }[], | ||
| ): Promise<OnedriveGroupInput> { | ||
| return; | ||
| } |
There was a problem hiding this comment.
Incomplete implementation of desunify method.
The desunify method currently does not perform any operations and returns nothing. This seems like an incomplete implementation. Please ensure that the method correctly maps the UnifiedFilestorageGroupInput to OnedriveGroupInput based on the provided customFieldMappings.
| async sync( | ||
| data: SyncParam, | ||
| ): Promise<ApiResponse<OnedrivePermissionOutput[]>> { | ||
| try { | ||
| const { linkedUserId, extra } = data; | ||
| // TODO: where it comes from ?? extra?: { object_name: 'folder' | 'file'; value: string }, | ||
|
|
||
| const connection = await this.prisma.connections.findFirst({ | ||
| where: { | ||
| id_linked_user: linkedUserId, | ||
| provider_slug: 'onedrive', | ||
| vertical: 'filestorage', | ||
| }, | ||
| }); | ||
| let remote_id; | ||
| if (extra.object_name == 'folder') { | ||
| const a = await this.prisma.fs_folders.findUnique({ | ||
| where: { | ||
| id_fs_folder: extra.value, | ||
| }, | ||
| }); | ||
| remote_id = a.remote_id; | ||
| } | ||
| if (extra.object_name == 'file') { | ||
| const a = await this.prisma.fs_files.findUnique({ | ||
| where: { | ||
| id_fs_file: extra.value, | ||
| }, | ||
| }); | ||
|
|
||
| remote_id = a.remote_id; | ||
| } | ||
|
|
||
| const resp = await axios.get( | ||
| `${connection.account_url}/v1.0/drive/items/${remote_id}/permissions`, | ||
| { | ||
| headers: { | ||
| Authorization: `Bearer ${this.cryptoService.decrypt( | ||
| connection.access_token, | ||
| )}`, | ||
| }, | ||
| }, | ||
| ); | ||
|
|
||
| return { | ||
| data: resp.data.value as OnedrivePermissionOutput[], | ||
| message: 'Synced onedrive permissions !', | ||
| statusCode: 200, | ||
| }; | ||
| } catch (error) { | ||
| throw error; | ||
| } | ||
| } |
There was a problem hiding this comment.
Optimize Error Handling and String Usage in sync Method
- Use Template Literals: Replace string concatenation with template literals for better readability and performance, as suggested by static analysis.
- Redundant Catch Block: The catch block that simply rethrows the error can be removed to reduce unnecessary code complexity.
Apply these changes:
- this.logger.setContext(FileStorageObject.permission.toUpperCase() + ':' + OnedriveService.name);
+ this.logger.setContext(`${FileStorageObject.permission.toUpperCase()}:${OnedriveService.name}`);
- throw error;
+ // Removed redundant catch blockCommittable 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 sync( | |
| data: SyncParam, | |
| ): Promise<ApiResponse<OnedrivePermissionOutput[]>> { | |
| try { | |
| const { linkedUserId, extra } = data; | |
| // TODO: where it comes from ?? extra?: { object_name: 'folder' | 'file'; value: string }, | |
| const connection = await this.prisma.connections.findFirst({ | |
| where: { | |
| id_linked_user: linkedUserId, | |
| provider_slug: 'onedrive', | |
| vertical: 'filestorage', | |
| }, | |
| }); | |
| let remote_id; | |
| if (extra.object_name == 'folder') { | |
| const a = await this.prisma.fs_folders.findUnique({ | |
| where: { | |
| id_fs_folder: extra.value, | |
| }, | |
| }); | |
| remote_id = a.remote_id; | |
| } | |
| if (extra.object_name == 'file') { | |
| const a = await this.prisma.fs_files.findUnique({ | |
| where: { | |
| id_fs_file: extra.value, | |
| }, | |
| }); | |
| remote_id = a.remote_id; | |
| } | |
| const resp = await axios.get( | |
| `${connection.account_url}/v1.0/drive/items/${remote_id}/permissions`, | |
| { | |
| headers: { | |
| Authorization: `Bearer ${this.cryptoService.decrypt( | |
| connection.access_token, | |
| )}`, | |
| }, | |
| }, | |
| ); | |
| return { | |
| data: resp.data.value as OnedrivePermissionOutput[], | |
| message: 'Synced onedrive permissions !', | |
| statusCode: 200, | |
| }; | |
| } catch (error) { | |
| throw error; | |
| } | |
| } | |
| async sync( | |
| data: SyncParam, | |
| ): Promise<ApiResponse<OnedrivePermissionOutput[]>> { | |
| const { linkedUserId, extra } = data; | |
| // TODO: where it comes from ?? extra?: { object_name: 'folder' | 'file'; value: string }, | |
| const connection = await this.prisma.connections.findFirst({ | |
| where: { | |
| id_linked_user: linkedUserId, | |
| provider_slug: 'onedrive', | |
| vertical: 'filestorage', | |
| }, | |
| }); | |
| let remote_id; | |
| if (extra.object_name == 'folder') { | |
| const a = await this.prisma.fs_folders.findUnique({ | |
| where: { | |
| id_fs_folder: extra.value, | |
| }, | |
| }); | |
| remote_id = a.remote_id; | |
| } | |
| if (extra.object_name == 'file') { | |
| const a = await this.prisma.fs_files.findUnique({ | |
| where: { | |
| id_fs_file: extra.value, | |
| }, | |
| }); | |
| remote_id = a.remote_id; | |
| } | |
| const resp = await axios.get( | |
| `${connection.account_url}/v1.0/drive/items/${remote_id}/permissions`, | |
| { | |
| headers: { | |
| Authorization: `Bearer ${this.cryptoService.decrypt( | |
| connection.access_token, | |
| )}`, | |
| }, | |
| }, | |
| ); | |
| return { | |
| data: resp.data.value as OnedrivePermissionOutput[], | |
| message: 'Synced onedrive permissions !', | |
| statusCode: 200, | |
| }; | |
| } |
Tools
Biome
[error] 77-77: The catch clause that only rethrows the original error is redundant.
These unnecessary catch clauses can be confusing. It is recommended to remove them.
(lint/complexity/noUselessCatch)
| async sync( | ||
| data: SyncParam, | ||
| ): Promise<ApiResponse<OnedriveSharedLinkOutput[]>> { | ||
| try { | ||
| const { linkedUserId, extra } = data; | ||
| // TODO: where it comes from ?? extra?: { object_name: 'folder' | 'file'; value: string }, | ||
|
|
||
| const connection = await this.prisma.connections.findFirst({ | ||
| where: { | ||
| id_linked_user: linkedUserId, | ||
| provider_slug: 'onedrive', | ||
| vertical: 'filestorage', | ||
| }, | ||
| }); | ||
| let remote_id; | ||
| if (extra.object_name == 'folder') { | ||
| const a = await this.prisma.fs_folders.findUnique({ | ||
| where: { | ||
| id_fs_folder: extra.value, | ||
| }, | ||
| }); | ||
| remote_id = a.remote_id; | ||
| } | ||
| if (extra.object_name == 'file') { | ||
| const a = await this.prisma.fs_files.findUnique({ | ||
| where: { | ||
| id_fs_file: extra.value, | ||
| }, | ||
| }); | ||
| remote_id = a.remote_id; | ||
| } | ||
|
|
||
| const resp = await axios.get( | ||
| `${connection.account_url}/v1.0/drive/items/${remote_id}/permissions`, | ||
| // Same url as onedrive permissions | ||
| // We might use POST /drives/{driveId}/items/{itemId}/createLink later | ||
| { | ||
| headers: { | ||
| Authorization: `Bearer ${this.cryptoService.decrypt( | ||
| connection.access_token, | ||
| )}`, | ||
| }, | ||
| }, | ||
| ); | ||
| return { | ||
| data: resp.data, | ||
| message: 'Onedrive sharedlinks retrieved', | ||
| statusCode: resp.status, // 200 || 201 | ||
| }; | ||
| } catch (error) { | ||
| throw error; | ||
| } | ||
| } |
There was a problem hiding this comment.
Optimize Error Handling and String Usage in sync Method for Shared Links
- Use Template Literals: Replace string concatenation with template literals to enhance code readability and maintain consistency with modern JavaScript practices.
- Redundant Catch Block: Consider removing the catch block that only rethrows the error to simplify the error handling logic.
Apply these changes:
- this.logger.setContext(FileStorageObject.sharedlink.toUpperCase() + ':' + OnedriveService.name);
+ this.logger.setContext(`${FileStorageObject.sharedlink.toUpperCase()}:${OnedriveService.name}`);
- throw error;
+ // Removed redundant catch blockCommittable suggestion was skipped due to low confidence.
Tools
Biome
[error] 78-78: The catch clause that only rethrows the original error is redundant.
These unnecessary catch clauses can be confusing. It is recommended to remove them.
(lint/complexity/noUselessCatch)
| async addFolder( | ||
| folderData: OnedriveFolderInput, | ||
| linkedUserId: string, | ||
| ): Promise<ApiResponse<OnedriveFolderOutput>> { | ||
| try { | ||
| const connection = await this.prisma.connections.findFirst({ | ||
| where: { | ||
| id_linked_user: linkedUserId, | ||
| provider_slug: 'onedrive', | ||
| vertical: 'filestorage', | ||
| }, | ||
| }); | ||
|
|
||
| // Currently adding in root folder, might need to change | ||
| const resp = await axios.post( | ||
| `${connection.account_url}/v1.0/drive/root/children`, | ||
| JSON.stringify({ | ||
| name: folderData.name, | ||
| folder: {}, | ||
| '@microsoft.graph.conflictBehavior': 'rename', // 'rename' | 'fail' | 'replace' | ||
| }), | ||
| { | ||
| headers: { | ||
| 'Content-Type': 'application/json', | ||
| Authorization: `Bearer ${this.cryptoService.decrypt( | ||
| connection.access_token, | ||
| )}`, | ||
| }, | ||
| }, | ||
| ); | ||
|
|
||
| return { | ||
| data: resp.data, | ||
| message: 'Onedrive folder created', | ||
| statusCode: 201, | ||
| }; | ||
| } catch (error) { | ||
| throw error; | ||
| } | ||
| } |
There was a problem hiding this comment.
Optimize Error Handling in addFolder Method
The method is well-implemented for adding folders to OneDrive. However, the catch block that only rethrows the error can be removed to simplify the error handling logic.
Apply this change:
- throw error;
+ // Removed redundant catch blockCommittable 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 addFolder( | |
| folderData: OnedriveFolderInput, | |
| linkedUserId: string, | |
| ): Promise<ApiResponse<OnedriveFolderOutput>> { | |
| try { | |
| const connection = await this.prisma.connections.findFirst({ | |
| where: { | |
| id_linked_user: linkedUserId, | |
| provider_slug: 'onedrive', | |
| vertical: 'filestorage', | |
| }, | |
| }); | |
| // Currently adding in root folder, might need to change | |
| const resp = await axios.post( | |
| `${connection.account_url}/v1.0/drive/root/children`, | |
| JSON.stringify({ | |
| name: folderData.name, | |
| folder: {}, | |
| '@microsoft.graph.conflictBehavior': 'rename', // 'rename' | 'fail' | 'replace' | |
| }), | |
| { | |
| headers: { | |
| 'Content-Type': 'application/json', | |
| Authorization: `Bearer ${this.cryptoService.decrypt( | |
| connection.access_token, | |
| )}`, | |
| }, | |
| }, | |
| ); | |
| return { | |
| data: resp.data, | |
| message: 'Onedrive folder created', | |
| statusCode: 201, | |
| }; | |
| } catch (error) { | |
| throw error; | |
| } | |
| } | |
| async addFolder( | |
| folderData: OnedriveFolderInput, | |
| linkedUserId: string, | |
| ): Promise<ApiResponse<OnedriveFolderOutput>> { | |
| try { | |
| const connection = await this.prisma.connections.findFirst({ | |
| where: { | |
| id_linked_user: linkedUserId, | |
| provider_slug: 'onedrive', | |
| vertical: 'filestorage', | |
| }, | |
| }); | |
| // Currently adding in root folder, might need to change | |
| const resp = await axios.post( | |
| `${connection.account_url}/v1.0/drive/root/children`, | |
| JSON.stringify({ | |
| name: folderData.name, | |
| folder: {}, | |
| '@microsoft.graph.conflictBehavior': 'rename', // 'rename' | 'fail' | 'replace' | |
| }), | |
| { | |
| headers: { | |
| 'Content-Type': 'application/json', | |
| Authorization: `Bearer ${this.cryptoService.decrypt( | |
| connection.access_token, | |
| )}`, | |
| }, | |
| }, | |
| ); | |
| return { | |
| data: resp.data, | |
| message: 'Onedrive folder created', | |
| statusCode: 201, | |
| }; | |
| } catch (error) { | |
| // Removed redundant catch block | |
| } | |
| } |
Tools
Biome
[error] 69-69: The catch clause that only rethrows the original error is redundant.
These unnecessary catch clauses can be confusing. It is recommended to remove them.
(lint/complexity/noUselessCatch)
| async iterativeGetOnedriveFolders( | ||
| remote_folder_id: string, | ||
| linkedUserId: string, | ||
| ): Promise<OnedriveFolderOutput[]> { | ||
| try { | ||
| const connection = await this.prisma.connections.findFirst({ | ||
| where: { | ||
| id_linked_user: linkedUserId, | ||
| provider_slug: 'onedrive', | ||
| vertical: 'filestorage', | ||
| }, | ||
| }); | ||
|
|
||
| let result = [], | ||
| depth = 0, | ||
| batch = [remote_folder_id]; | ||
|
|
||
| while (batch.length > 0) { | ||
| if (depth > 5) { | ||
| // todo: handle this better | ||
| break; | ||
| } | ||
|
|
||
| const nestedFolders = await Promise.all( | ||
| batch.map(async (folder_id) => { | ||
| const resp = await axios.get( | ||
| `${connection.account_url}/v1.0/drive/items/${folder_id}/children`, | ||
| { | ||
| headers: { | ||
| 'Content-Type': 'application/json', | ||
| Authorization: `Bearer ${this.cryptoService.decrypt( | ||
| connection.access_token, | ||
| )}`, | ||
| }, | ||
| }, | ||
| ); | ||
|
|
||
| // Add permissions (shared link is also included in permissions in one-drive) | ||
| await Promise.all( | ||
| resp.data.value.map(async (driveItem) => { | ||
| const resp = await axios.get( | ||
| `${connection.account_url}/v1.0/drive/items/${driveItem.id}/permissions`, | ||
| { | ||
| headers: { | ||
| 'Content-Type': 'application/json', | ||
| Authorization: `Bearer ${this.cryptoService.decrypt( | ||
| connection.access_token, | ||
| )}`, | ||
| }, | ||
| }, | ||
| ); | ||
| driveItem.permissions = resp.data.value; | ||
| }), | ||
| ); | ||
|
|
||
| const folders = resp.data.value.filter( | ||
| (driveItem) => driveItem.folder, | ||
| ); | ||
|
|
||
| // const files = resp.data.value.filter( | ||
| // (driveItem) => !driveItem.folder, | ||
| // ); | ||
|
|
||
| // await this.ingestService.ingestData< | ||
| // UnifiedFilestorageFileOutput, | ||
| // OnedriveFileOutput | ||
| // >( | ||
| // files, | ||
| // 'onedrive', | ||
| // connection.id_connection, | ||
| // 'filestorage', | ||
| // FileStorageObject.file, | ||
| // ); | ||
|
|
||
| return folders; | ||
| }), | ||
| ); | ||
|
|
||
| // nestedFolders = [[subfolder1, subfolder2], [subfolder3, subfolder4]] | ||
| result = result.concat(nestedFolders.flat()); | ||
| batch = nestedFolders.flat().map((folder) => folder.id); | ||
| this.logger.log(`Batch size: ${batch.length} at depth ${depth}`); | ||
| depth++; | ||
| } | ||
|
|
||
| return result; | ||
| } catch (error) { | ||
| throw error; | ||
| } | ||
| } |
There was a problem hiding this comment.
Refactor iterativeGetOnedriveFolders Method for Clarity and Efficiency
- Variable Declarations: Separate the declarations of
result,depth, andbatchfor better readability and maintainability. - Redundant Catch Block: Remove the catch block that only rethrows the error to simplify the error handling logic.
Apply these changes:
- let result = [], depth = 0, batch = [remote_folder_id];
+ let result = [];
+ let depth = 0;
+ let batch = [remote_folder_id];
- throw error;
+ // Removed redundant catch blockCommittable 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 iterativeGetOnedriveFolders( | |
| remote_folder_id: string, | |
| linkedUserId: string, | |
| ): Promise<OnedriveFolderOutput[]> { | |
| try { | |
| const connection = await this.prisma.connections.findFirst({ | |
| where: { | |
| id_linked_user: linkedUserId, | |
| provider_slug: 'onedrive', | |
| vertical: 'filestorage', | |
| }, | |
| }); | |
| let result = [], | |
| depth = 0, | |
| batch = [remote_folder_id]; | |
| while (batch.length > 0) { | |
| if (depth > 5) { | |
| // todo: handle this better | |
| break; | |
| } | |
| const nestedFolders = await Promise.all( | |
| batch.map(async (folder_id) => { | |
| const resp = await axios.get( | |
| `${connection.account_url}/v1.0/drive/items/${folder_id}/children`, | |
| { | |
| headers: { | |
| 'Content-Type': 'application/json', | |
| Authorization: `Bearer ${this.cryptoService.decrypt( | |
| connection.access_token, | |
| )}`, | |
| }, | |
| }, | |
| ); | |
| // Add permissions (shared link is also included in permissions in one-drive) | |
| await Promise.all( | |
| resp.data.value.map(async (driveItem) => { | |
| const resp = await axios.get( | |
| `${connection.account_url}/v1.0/drive/items/${driveItem.id}/permissions`, | |
| { | |
| headers: { | |
| 'Content-Type': 'application/json', | |
| Authorization: `Bearer ${this.cryptoService.decrypt( | |
| connection.access_token, | |
| )}`, | |
| }, | |
| }, | |
| ); | |
| driveItem.permissions = resp.data.value; | |
| }), | |
| ); | |
| const folders = resp.data.value.filter( | |
| (driveItem) => driveItem.folder, | |
| ); | |
| // const files = resp.data.value.filter( | |
| // (driveItem) => !driveItem.folder, | |
| // ); | |
| // await this.ingestService.ingestData< | |
| // UnifiedFilestorageFileOutput, | |
| // OnedriveFileOutput | |
| // >( | |
| // files, | |
| // 'onedrive', | |
| // connection.id_connection, | |
| // 'filestorage', | |
| // FileStorageObject.file, | |
| // ); | |
| return folders; | |
| }), | |
| ); | |
| // nestedFolders = [[subfolder1, subfolder2], [subfolder3, subfolder4]] | |
| result = result.concat(nestedFolders.flat()); | |
| batch = nestedFolders.flat().map((folder) => folder.id); | |
| this.logger.log(`Batch size: ${batch.length} at depth ${depth}`); | |
| depth++; | |
| } | |
| return result; | |
| } catch (error) { | |
| throw error; | |
| } | |
| } | |
| async iterativeGetOnedriveFolders( | |
| remote_folder_id: string, | |
| linkedUserId: string, | |
| ): Promise<OnedriveFolderOutput[]> { | |
| const connection = await this.prisma.connections.findFirst({ | |
| where: { | |
| id_linked_user: linkedUserId, | |
| provider_slug: 'onedrive', | |
| vertical: 'filestorage', | |
| }, | |
| }); | |
| let result = []; | |
| let depth = 0; | |
| let batch = [remote_folder_id]; | |
| while (batch.length > 0) { | |
| if (depth > 5) { | |
| // todo: handle this better | |
| break; | |
| } | |
| const nestedFolders = await Promise.all( | |
| batch.map(async (folder_id) => { | |
| const resp = await axios.get( | |
| `${connection.account_url}/v1.0/drive/items/${folder_id}/children`, | |
| { | |
| headers: { | |
| 'Content-Type': 'application/json', | |
| Authorization: `Bearer ${this.cryptoService.decrypt( | |
| connection.access_token, | |
| )}`, | |
| }, | |
| }, | |
| ); | |
| // Add permissions (shared link is also included in permissions in one-drive) | |
| await Promise.all( | |
| resp.data.value.map(async (driveItem) => { | |
| const resp = await axios.get( | |
| `${connection.account_url}/v1.0/drive/items/${driveItem.id}/permissions`, | |
| { | |
| headers: { | |
| 'Content-Type': 'application/json', | |
| Authorization: `Bearer ${this.cryptoService.decrypt( | |
| connection.access_token, | |
| )}`, | |
| }, | |
| }, | |
| ); | |
| driveItem.permissions = resp.data.value; | |
| }), | |
| ); | |
| const folders = resp.data.value.filter( | |
| (driveItem) => driveItem.folder, | |
| ); | |
| // const files = resp.data.value.filter( | |
| // (driveItem) => !driveItem.folder, | |
| // ); | |
| // await this.ingestService.ingestData< | |
| // UnifiedFilestorageFileOutput, | |
| // OnedriveFileOutput | |
| // >( | |
| // files, | |
| // 'onedrive', | |
| // connection.id_connection, | |
| // 'filestorage', | |
| // FileStorageObject.file, | |
| // ); | |
| return folders; | |
| }), | |
| ); | |
| // nestedFolders = [[subfolder1, subfolder2], [subfolder3, subfolder4]] | |
| result = result.concat(nestedFolders.flat()); | |
| batch = nestedFolders.flat().map((folder) => folder.id); | |
| this.logger.log(`Batch size: ${batch.length} at depth ${depth}`); | |
| depth++; | |
| } | |
| return result; | |
| } |
Tools
Biome
[error] 86-88: Declare variables separately
Unsafe fix: Break out into multiple declarations
(lint/style/useSingleVarDeclarator)
[error] 160-160: The catch clause that only rethrows the original error is redundant.
These unnecessary catch clauses can be confusing. It is recommended to remove them.
(lint/complexity/noUselessCatch)
| * 2014-01-01T00:00:00Z. Returned by default. Supports $filter (eq, ne, not, ge, le, in). Read-only. | ||
| */ | ||
| expirationDateTime?: string; | ||
| /** | ||
| * Specifies the group type and its membership. If the collection contains Unified, the group is a Microsoft 365 group; | ||
| * otherwise, it's either a security group or a distribution group. For details, see groups overview.If the collection | ||
| * includes DynamicMembership, the group has dynamic membership; otherwise, membership is static. Returned by default. | ||
| * Supports $filter (eq, not). | ||
| */ | ||
| groupTypes?: string[]; | ||
| /** | ||
| * When a group is associated with a team, this property determines whether the team is in read-only mode.To read this | ||
| * property, use the /group/{groupId}/team endpoint or the Get team API. To update this property, use the archiveTeam and | ||
| * unarchiveTeam APIs. | ||
| */ | ||
| isArchived?: boolean; | ||
| /** | ||
| * The SMTP address for the group, for example, '[email protected]'. Returned by default. Read-only. Supports | ||
| * $filter (eq, ne, not, ge, le, in, startsWith, and eq on null values). | ||
| */ | ||
| mail?: string; | ||
| // Specifies whether the group is mail-enabled. Required. Returned by default. Supports $filter (eq, ne, not). | ||
| mailEnabled?: boolean; | ||
| /** | ||
| * The mail alias for the group, unique for Microsoft 365 groups in the organization. Maximum length is 64 characters. | ||
| * This property can contain only characters in the ASCII character set 0 - 127 except the following characters: @ () / [] | ||
| * ' ; : &lt;&gt; , SPACE. Required. Returned by default. Supports $filter (eq, ne, not, ge, le, in, startsWith, | ||
| * and eq on null values). | ||
| */ | ||
| mailNickname?: string; | ||
| /** | ||
| * The preferred data location for the Microsoft 365 group. By default, the group inherits the group creator's preferred | ||
| * data location. To set this property, the calling app must be granted the Directory.ReadWrite.All permission and the | ||
| * user be assigned at least one of the following Microsoft Entra roles: User Account Administrator Directory Writer | ||
| * Exchange Administrator SharePoint Administrator For more information about this property, see OneDrive Online | ||
| * Multi-Geo. Nullable. Returned by default. | ||
| */ | ||
| preferredDataLocation?: string; | ||
| /** | ||
| * The preferred language for a Microsoft 365 group. Should follow ISO 639-1 Code; for example, en-US. Returned by | ||
| * default. Supports $filter (eq, ne, not, ge, le, in, startsWith, and eq on null values). | ||
| */ | ||
| preferredLanguage?: string; | ||
| /** | ||
| * Email addresses for the group that direct to the same group mailbox. For example: ['SMTP: [email protected]', 'smtp: | ||
| * [email protected]']. The any operator is required to filter expressions on multi-valued properties. Returned by | ||
| * default. Read-only. Not nullable. Supports $filter (eq, not, ge, le, startsWith, endsWith, /$count eq 0, /$count ne 0). | ||
| */ | ||
| proxyAddresses?: string[]; | ||
| /** | ||
| * Timestamp of when the group was last renewed. This value can't be modified directly and is only updated via the renew | ||
| * service action. The Timestamp type represents date and time information using ISO 8601 format and is always in UTC. For | ||
| * example, midnight UTC on January 1, 2014 is 2014-01-01T00:00:00Z. Returned by default. Supports $filter (eq, ne, not, | ||
| * ge, le, in). Read-only. | ||
| */ | ||
|
|
||
| renewedDateTime?: string; | ||
| /** | ||
| * Specifies whether the group is a security group. Required. Returned by default. Supports $filter (eq, ne, not, in). | ||
| */ | ||
| securityEnabled?: boolean; | ||
| /** | ||
| * Security identifier of the group, used in Windows scenarios. Read-only. Returned by default. | ||
| */ | ||
| securityIdentifier?: string; | ||
| // The unique identifier that can be assigned to a group and used as an alternate key. Immutable. Read-only. | ||
| uniqueName?: string; | ||
| /** | ||
| * Specifies the group join policy and group content visibility for groups. Possible values are: Private, Public, or | ||
| * HiddenMembership. HiddenMembership can be set only for Microsoft 365 groups when the groups are created. It can't be | ||
| * updated later. Other values of visibility can be updated after group creation. If visibility value isn't specified | ||
| * during group creation on Microsoft Graph, a security group is created as Private by default, and the Microsoft 365 | ||
| * group is Public. Groups assignable to roles are always Private. To learn more, see group visibility options. Returned | ||
| * by default. Nullable. | ||
| */ | ||
| visibility?: string; | ||
| /** | ||
| * The user (or application) that created the group. NOTE: This property isn't set if the user is an administrator. | ||
| * Read-only. | ||
| */ | ||
| createdOnBehalfOf?: DirectoryObject; | ||
| /** | ||
| * Groups that this group is a member of. HTTP Methods: GET (supported for all groups). Read-only. Nullable. Supports | ||
| * $expand. | ||
| */ | ||
| memberOf?: DirectoryObject[]; | ||
| /** | ||
| * The members of this group, who can be users, devices, other groups, or service principals. Supports the List members, | ||
| * Add member, and Remove member operations. Nullable. Supports $expand including nested $select. For example, | ||
| * /groups?$filter=startsWith(displayName,'Role')&$select=id,displayName&$expand=members($select=id,userPrincipalName,displayName). | ||
| */ | ||
| members?: DirectoryObject[]; | ||
| /** | ||
| * The owners of the group. Limited to 100 owners. Nullable. If this property isn't specified when creating a Microsoft | ||
| * 365 group, the calling user is automatically assigned as the group owner. Supports $filter (/$count eq 0, /$count ne 0, | ||
| * /$count eq 1, /$count ne 1). Supports $expand including nested $select. For example, | ||
| * /groups?$filter=startsWith(displayName,'Role')&$select=id,displayName&$expand=owners($select=id,userPrincipalName,displayName). | ||
| */ | ||
| owners?: DirectoryObject[]; | ||
| } |
There was a problem hiding this comment.
Comprehensive and well-documented interface for OneDrive group integration.
The OnedriveGroupInput interface is well-documented with detailed comments explaining each property. This is crucial for maintainability and understanding the integration with OneDrive's API. The use of optional properties (?) is appropriate given the nature of API responses which may not always return all fields. The inclusion of TypeScript types such as string[] for groupTypes and boolean for flags like mailEnabled is correctly applied.
However, there are a few areas that could be improved:
- Consistency in Comment Formatting: Ensure that all comments are consistently formatted. Some comments start with a capital letter and end with a period, while others do not. Consistency in documentation style helps maintain readability.
- Validation of Properties: While the interface defines the properties, runtime validation might be necessary to ensure that the data conforms to the expected formats, especially for properties like
mailNicknamewhich have specific requirements. - Use of TypeScript Utility Types: For
OnedriveGroupOutput, usingPartial<OnedriveGroupInput>is a smart choice to allow partial updates. However, consider if other utility types likeReadonlyorPickcould be useful in certain contexts to enforce immutability or to restrict which properties can be updated.
Overall, the definitions are robust and provide a strong foundation for the OneDrive integration.
|
/claim #537 |
Add integration with one-drive #537
Objects
demo: https://drive.google.com/file/d/1gEDwP1lRquYH_yFzXTBna_OZjBY9tYv-/view?usp=drive_link
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Refactor