Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Contributor
WalkthroughAdds new SharePoint actions (Get Site, List Sites, Search Sites, Search Files), extends the SharePoint app API with getSite, searchQuery, and param-driven listAllSites, introduces select/orderBy/maxResults props across actions, updates value resolution in select-files, and bumps multiple component versions including package.json. Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@components/sharepoint/actions/list-files-in-folder/list-files-in-folder.mjs`:
- Around line 47-78: The params object in run() must use MS Graph query names
($select, $orderby, $filter) and support server-side filtering; add a new
filterType prop (e.g., supports values like "excludeFolders" or "foldersOnly"),
build params using keys "$select": this.select, "$orderby": this.orderBy and
"$filter": computedFilter (derived from filterType, e.g. "folder ne null" or
"folder eq null"), then call cleanObject(params) before passing to
sharepoint.listDriveItemsInFolder/listDriveItems so undefined values are
stripped; update any references to excludeFolders to use filterType and ensure
sharepoint calls receive the cleaned params.
In `@components/sharepoint/sharepoint.app.mjs`:
- Around line 11-24: In the async options function (options({ prevContext, query
})) stop adding args.params.search when following a paginated nextLink: only set
args.params = { search: query } when query is present AND prevContext?.nextLink
is falsy; when prevContext.nextLink exists, rely on that URL and do not append
params to avoid duplicate search query breaking pagination (adjust logic around
prevContext.nextLink, args, and args.params.search accordingly).
- Around line 269-281: Normalize Graph query params before the HTTP call by
updating _makeRequest to map incoming params keys: transform "select" →
"$select", "orderby" → "$orderby", and "filter" → "$filter" (preserving other
params) so axios receives the dollar-prefixed names; locate the params object
handling in _makeRequest and perform this key replacement (or a small mapping
pass) prior to passing params to axios, ensuring existing callers like
list-files-in-folder.mjs need no changes.
components/sharepoint/actions/list-files-in-folder/list-files-in-folder.mjs
Show resolved
Hide resolved
michelle0927
moved this to Ready for PR Review
in Component (Source and Action) Backlog
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/sharepoint/actions/list-files-in-folder/list-files-in-folder.mjs (1)
79-81: Client-side filtering fetches all items before filtering.The
excludeFoldersprop filters results after fetching all items from the API. For large folders, consider adding server-side filtering using MS Graph's$filter=file ne nullparameter to improve performance. This aligns with the PR objectives referencing$filtersupport from issue#19747.
🤖 Fix all issues with AI agents
In `@components/sharepoint/actions/get-file-by-id/get-file-by-id.mjs`:
- Around line 54-56: The params object passed to the Graph API call in
get-file-by-id.mjs uses the wrong OData parameter name; update the params entry
in the request (the object built where params: { select: this.select }) to use
the MS Graph OData key `$select` instead of `select` so the request uses
`$select: this.select` when calling the Graph API (locate the params
construction in the action that sends the request).
In `@components/sharepoint/actions/list-files-in-folder/list-files-in-folder.mjs`:
- Around line 53-59: The orderBy prop in list-files-in-folder (orderBy) is
defined inline while select reuses a shared propDefinition; add a reusable
orderBy propDefinition in sharepoint.app.mjs (e.g., export/orderByProp or
similar) that captures type, label, description, default, and optional settings,
then update the list-files-in-folder action to reference that shared
propDefinition instead of the inline orderBy to ensure consistency and easier
maintenance.
♻️ Duplicate comments (1)
components/sharepoint/actions/list-files-in-folder/list-files-in-folder.mjs (1)
62-65: MS Graph API query parameters require$prefix.The params object uses
selectandorderbykeys, but MS Graph API requires$selectand$orderby(with the$prefix) for OData query parameters to be recognized. Without this prefix, the API will silently ignore these parameters.♻️ Proposed fix
const params = { - select: this.select, - orderby: this.orderBy, + $select: this.select, + $orderby: this.orderBy, };
components/sharepoint/actions/list-files-in-folder/list-files-in-folder.mjs
Show resolved
Hide resolved
lcaresia
moved this from Ready for PR Review
to Ready for QA
in Component (Source and Action) Backlog
michelle0927
moved this from Ready for QA
to High Priority - Ready for QA
in Component (Source and Action) Backlog
vunguyenhung
moved this from High Priority - Ready for QA
to Changes Required
in Component (Source and Action) Backlog
Contributor
|
Hello everyone, I have tested this PR and there're some test cases failed or needed improvement. Please check test reports below for more information:
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@components/sharepoint/actions/search-files/search-files.mjs`:
- Around line 40-52: The optional boolean property descending currently has no
default, causing isDescending to be sent as undefined; either set a default
value (e.g., add default: false to the descending property alongside sortField)
or change the run method where sortProperties is built (look for sortProperties
and code that sets isDescending) to only include isDescending when descending
!== undefined (or explicitly coerce to Boolean) so the API receives a
deterministic true/false value; update whichever approach you choose and ensure
references to sortField, descending, and sortProperties are adjusted
accordingly.
In `@components/sharepoint/actions/select-files/select-files.mjs`:
- Around line 64-70: The resolveValue function's check uses truthiness
(prop?.value) which misclassifies legitimate falsy values (e.g., 0 or "") as
missing; update resolveValue to detect presence of the property instead: keep
the typeof prop === "object" guard and replace the truthy check with a
property-existence check (e.g., use 'value' in prop) so objects with a defined
but falsy value return that value correctly.
Contributor
|
Hi everyone, all test cases are passed! Ready for release! Test reports
|
Contributor
|
Hi everyone, all test cases are passed! Ready for release! Test reports
|
1 similar comment
Contributor
|
Hi everyone, all test cases are passed! Ready for release! Test reports
|
Collaborator
Author
|
/approve |
lcaresia
approved these changes
Jan 29, 2026
github-project-automation
bot
moved this from Ready for Release
to Done
in Component (Source and Action) Backlog
This was referenced Feb 26, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolves #19743
Resolves #19744
Resolves #19747
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.