[8053] Remove deprecated getDescriptor API and update APIs in the js-sdk - #163
[8053] Remove deprecated getDescriptor API and update APIs in the js-sdk#163jvega190 wants to merge 5 commits into
Conversation
|
Warning Review limit reached
Next review available in: 13 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
WalkthroughRemoves the Changesv5.0.0 API Cleanup
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
packages/content/README.md (1)
148-220: 📐 Maintainability & Code Quality | 🔵 TrivialConsider documenting the
flattenoption in service signatures.
getItemandgetChildrennow passflattenas a query parameter derived fromCrafterConfig. The README currently documents these asgetItem(path: string, config?: CrafterConfig)without mentioning thatconfig.flattenaffects the response shape. Consider adding a note thatflattencontrols whether linked content items are recursively included.Add to the `getItem` and `getChildren` parameter tables or description: - `config.flatten` — boolean, controls whether to recursively include linked content items in the response.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/content/README.md` around lines 148 - 220, The `getItem` and `getChildren` docs do not mention the new `config.flatten` behavior, so update the README entries for these service signatures to document that `CrafterConfig.flatten` controls whether linked content items are recursively included in the response. Add this note near the parameter tables or example descriptions for `getItem(path, config?)` and `getChildren(path, config?)` so readers can find it alongside the existing `CrafterConfig` reference.packages/content/src/ContentStoreService.ts (1)
35-37: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the stale
GetDescriptorConfigexport.
getDescriptoris gone andCrafterConfigalready ownsflatten, so keeping this public type leaves deprecated descriptor API surface behind.Proposed cleanup
-export interface GetDescriptorConfig extends CrafterConfig { - flatten: boolean; -} -🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/content/src/ContentStoreService.ts` around lines 35 - 37, Remove the stale public GetDescriptorConfig export from ContentStoreService since getDescriptor is no longer used and CrafterConfig already provides flatten. Update any references in the ContentStoreService module and related exports so the deprecated descriptor-specific API surface is eliminated while keeping the remaining CrafterConfig-based types intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/content/src/utils.ts`:
- Around line 219-220: The `getItem` calls in `parseDescriptor` (and the
matching path also used around the later `getItem`/`map` flow) are allowing
`options.flatten === false` to override the required flattening behavior. Update
the option merge so `options` are spread first and `flatten: true` is enforced
last, keeping the new `getItem`-based parsing path working consistently.
In `@packages/redux/src/actions/search.ts`:
- Line 22: The search action type string in search should be corrected from the
misspelled constant to the properly spelled CRAFTERCMS_SEARCH, matching the
existing SEARCH_COMPLETE naming and avoiding consumer mismatches; update the
createAction<Query> call in search to use the corrected string while keeping the
surrounding action definitions unchanged.
---
Nitpick comments:
In `@packages/content/README.md`:
- Around line 148-220: The `getItem` and `getChildren` docs do not mention the
new `config.flatten` behavior, so update the README entries for these service
signatures to document that `CrafterConfig.flatten` controls whether linked
content items are recursively included in the response. Add this note near the
parameter tables or example descriptions for `getItem(path, config?)` and
`getChildren(path, config?)` so readers can find it alongside the existing
`CrafterConfig` reference.
In `@packages/content/src/ContentStoreService.ts`:
- Around line 35-37: Remove the stale public GetDescriptorConfig export from
ContentStoreService since getDescriptor is no longer used and CrafterConfig
already provides flatten. Update any references in the ContentStoreService
module and related exports so the deprecated descriptor-specific API surface is
eliminated while keeping the remaining CrafterConfig-based types intact.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e4632dff-929e-4865-8c9e-41f34d72dc7c
📒 Files selected for processing (13)
CHANGELOG.mdpackages/classes/src/config.tspackages/content/README.mdpackages/content/src/ContentStoreService.tspackages/content/src/utils.tspackages/content/test/index.spec.tspackages/models/src/CrafterConfig.tspackages/redux/src/actions/content.tspackages/redux/src/actions/search.tspackages/redux/src/epics/content.tspackages/redux/src/reducers/content.tspackages/redux/src/reducers/reducers.tspackages/redux/test/index.spec.ts
💤 Files with no reviewable changes (3)
- packages/redux/src/reducers/reducers.ts
- packages/content/test/index.spec.ts
- packages/redux/test/index.spec.ts
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/classes/package.json`:
- Line 3: The package version is out of sync with the changelog release header.
Update the version in package.json for the classes package, or adjust the
CHANGELOG.md header for `@craftercms/classes` so both reflect the same release
version; use the package metadata and changelog entry together to keep the
published package and release notes aligned.
In `@packages/redux/package.json`:
- Line 3: Remove the outdated getDescriptor documentation from the Redux README
so the published docs match the 5.0.0 API. Update packages/redux/README.md to
delete the action creator section, signature, and example that reference
getDescriptor, and make sure any surrounding navigation or headings still read
correctly after the removal.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 48e1f6b3-b4bb-442a-b8dd-79d2f79bbc2e
📒 Files selected for processing (8)
packages/classes/package.jsonpackages/content/package.jsonpackages/ice/package.jsonpackages/models/package.jsonpackages/redux/package.jsonpackages/redux/src/actions/search.tspackages/search/package.jsonpackages/utils/package.json
✅ Files skipped from review due to trivial changes (3)
- packages/models/package.json
- packages/ice/package.json
- packages/utils/package.json
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/redux/src/actions/search.ts
…ated getDescriptor API from README, and clean up related test imports.
|
@coderabbitai can you check if everything looks good? Please give it a final review before I hand over for human review |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
repo="craftercms/js-sdk"
pr="163"
echo "== PR summary =="
gh pr view "$pr" --repo "$repo" --json number,title,state,isDraft,reviewDecision,mergeStateStatus,headRefName,baseRefName,author,url \
--jq '{number,title,state,isDraft,reviewDecision,mergeStateStatus,headRefName,baseRefName,author: .author.login,url}'
echo
echo "== Reviews =="
gh pr view "$pr" --repo "$repo" --json reviews \
--jq '.reviews[] | {author: .author.login, state, submittedAt}'
echo
echo "== Review threads (unresolved first) =="
gh api graphql \
-F owner="craftercms" \
-F name="js-sdk" \
-F number="$pr" \
-f query='
query($owner: String!, $name: String!, $number: Int!) {
repository(owner: $owner, name: $name) {
pullRequest(number: $number) {
reviewThreads(first: 100) {
nodes {
isResolved
isOutdated
path
line
comments(first: 20) {
nodes {
author { login }
body
createdAt
}
}
}
}
}
}
}' --jq '
.data.repository.pullRequest.reviewThreads.nodes
| sort_by(.isResolved, .path, .line)
| .[]
| {
isResolved,
isOutdated,
path,
line,
comments: [.comments.nodes[] | {author: .author.login, body: (.body | gsub("\n"; " ") | .[0:220]), createdAt}]
}'
echo
echo "== Changed files =="
gh pr view "$pr" --repo "$repo" --json files --jq '.files[] | {path, additions, deletions}'Length of output: 5952
What I checked:
One note: GitHub still shows So yes — this looks ready to hand over for human review. ✅ Action performedReview finished.
|
|
@coderabbitai can you check if everything looks good? Please give it a final review before I hand over for human review |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
repo="craftercms/js-sdk"
pr="163"
echo "== PR summary =="
gh pr view "$pr" --repo "$repo" --json number,title,state,isDraft,reviewDecision,mergeStateStatus,headRefName,baseRefName,author,url,updatedAt \
--jq '{number,title,state,isDraft,reviewDecision,mergeStateStatus,headRefName,baseRefName,author: .author.login,url,updatedAt}'
echo
echo "== Latest commits =="
gh pr view "$pr" --repo "$repo" --json commits --jq '.commits[-5:] | .[] | {oid: .oid[0:7], messageHeadline}'
echo
echo "== Review threads (unresolved first) =="
gh api graphql \
-F owner="craftercms" \
-F name="js-sdk" \
-F number="$pr" \
-f query='
query($owner: String!, $name: String!, $number: Int!) {
repository(owner: $owner, name: $name) {
pullRequest(number: $number) {
reviewThreads(first: 100) {
nodes {
isResolved
isOutdated
path
line
comments(first: 20) {
nodes {
author { login }
body
createdAt
}
}
}
}
}
}
}' --jq '
.data.repository.pullRequest.reviewThreads.nodes
| sort_by(.isResolved, .path, .line)
| .[]
| {
isResolved,
isOutdated,
path,
line,
comments: [.comments.nodes[] | {author: .author.login, body: (.body | gsub("\n"; " ") | .[0:220]), createdAt}]
}'
echo
echo "== Changed files =="
gh pr view "$pr" --repo "$repo" --json files --jq '.files[] | {path, additions, deletions}'Length of output: 6096
One thing worth flagging: since the last review, a new commit (
These weren't part of the diff I last reviewed, so I haven't verified them yet. I'd recommend triggering a fresh full review to make sure this latest commit is covered before you hand off to human review. I'll kick that off now. The PR still shows ✅ Action performedFull review finished. |
|
@CodeRabbit can you do a fresh full review? |
|
✅ Action performedFull review finished. |
|
@coderabbitai can you check if everything looks good? Please give it a final review before I hand over for human review |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
repo="craftercms/js-sdk"
pr="163"
echo "== PR summary =="
gh pr view "$pr" --repo "$repo" --json number,title,state,isDraft,reviewDecision,mergeStateStatus,headRefName,baseRefName,updatedAt \
--jq '{number,title,state,isDraft,reviewDecision,mergeStateStatus,headRefName,baseRefName,updatedAt}'
echo
echo "== Latest commits =="
gh pr view "$pr" --repo "$repo" --json commits --jq '.commits[-5:] | .[] | {oid: .oid[0:7], messageHeadline}'
echo
echo "== Review threads (unresolved first) =="
gh api graphql \
-F owner="craftercms" \
-F name="js-sdk" \
-F number="$pr" \
-f query='
query($owner: String!, $name: String!, $number: Int!) {
repository(owner: $owner, name: $name) {
pullRequest(number: $number) {
reviewThreads(first: 100) {
nodes {
isResolved
isOutdated
path
line
comments(first: 20) {
nodes {
author { login }
body
createdAt
}
}
}
}
}
}
}' --jq '
.data.repository.pullRequest.reviewThreads.nodes
| sort_by(.isResolved, .path, .line)
| .[]
| {
isResolved,
isOutdated,
path,
line,
comments: [.comments.nodes[] | {author: .author.login, body: (.body | gsub("\n"; " ") | .[0:220]), createdAt}]
}'
echo
echo "== Changed files =="
gh pr view "$pr" --repo "$repo" --json files --jq '.files[] | {path, additions, deletions}'Length of output: 5985
The PR still shows This is ready to hand over for human review. 🐇✅ |
1 participant
craftersoftware/craftercms#8053
Summary by CodeRabbit
flattenoption for content retrieval and configuration, enabling flattened results from item/tree/children requests.{ url, config }.flatten.