-
Notifications
You must be signed in to change notification settings - Fork 194
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Consider drive root to be a folder when syncing onedrive/sharepoint #703
Conversation
Qovery can create a Preview Environment for this PR.
This comment has been generated from Qovery AI 🤖.
|
|
WalkthroughWalkthroughThe changes involve enhancements to the Changes
Possibly related PRs
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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Files selected for processing (4)
- packages/api/src/filestorage/folder/services/onedrive/index.ts (1 hunks)
- packages/api/src/filestorage/folder/services/onedrive/types.ts (1 hunks)
- packages/api/src/filestorage/folder/services/sharepoint/index.ts (2 hunks)
- packages/api/src/filestorage/folder/services/sharepoint/types.ts (1 hunks)
Additional comments not posted (4)
packages/api/src/filestorage/folder/services/sharepoint/index.ts (2)
83-94
: LGTM!The code segment correctly retrieves the root folder data from SharePoint by making an HTTP GET request to the appropriate API endpoint. The request includes the necessary headers and securely handles the access token using the
cryptoService
.
120-120
: Documentation update looks good!The updated comment clarifies that shared link permissions are also included in SharePoint permissions. This aligns the documentation with the actual functionality and helps improve the understanding of the service's behavior.
packages/api/src/filestorage/folder/services/onedrive/index.ts (2)
86-97
: LGTM!The code segment correctly retrieves the root folder data from OneDrive using an asynchronous HTTP GET request. The request URL, headers, and response handling are implemented properly.
This addition enhances the functionality by providing the necessary context for subsequent operations related to folder management in OneDrive.
99-99
: Looks good!Initializing the
result
array with therootFolderData.data
is a logical addition that ensures the root folder's data is included in the result set. This change aligns well with the overall functionality of theiterativeGetOnedriveFolders
method.Tools
Biome
[error] 99-101: Declare variables separately
Unsafe fix: Break out into multiple declarations
(lint/style/useSingleVarDeclarator)
/** If this property is non-null, it indicates that the driveItem is the top-most driveItem in the drive. */ | ||
readonly root?: any; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Consider using a more specific type if possible.
The addition of the root
property enhances the OnedriveFolderInput
interface by providing additional context about the state of the driveItem
. The read-only nature of the property aligns with its purpose of indicating the top-most item in the drive hierarchy. The comment provides a clear explanation of the property's purpose, which is helpful for developers using this interface.
However, consider using a more specific type than any
for the root
property if possible, as it may provide better type safety and improve code maintainability.
/** If this property is non-null, it indicates that the driveItem is the top-most driveItem in the drive. */ | ||
readonly root?: any; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approve the addition of the root
property, but consider defining a specific type.
The new root
property enhances the SharepointFolderInput
interface by allowing it to indicate whether the associated driveItem
is the top-most item in the drive. This addition improves the interface's ability to represent the SharePoint folder structure accurately.
However, the any
type for the root
property might lead to inconsistencies or confusion. Consider defining a specific type, such as boolean
, to clearly convey the expected values and improve type safety.
Summary by CodeRabbit
New Features
root
property toOnedriveFolderInput
andSharepointFolderInput
interfaces for better handling of folder structures.Documentation