merge dev to main (v2.9.4) #1892
Conversation
📝 Walkthrough📝 WalkthroughWalkthroughThis pull request includes updates to the JetBrains plugin's 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 (
|
![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
🧹 Outside diff range and nitpick comments (3)
packages/ide/jetbrains/build.gradle.kts (1)
Resource paths need adjustment in prepareSandbox task
The verification revealed mismatches between the configured paths and actual file locations:
- The language server's
main.jspath (packages/schema/bundle/language-server/main.js) is incorrect. The language server is actually located inpackages/schema/src/language-server/.- The TextMate bundle path exists at the specified location:
packages/ide/jetbrains/src/main/textMate/zmodel.tmbundle/- The
stdlib.zmodelandzmodel.tmLanguagefiles exist in their respective locations.Please update the language server path in the
prepareSandboxtask to match the actual project structure:from("${project.projectDir}/../../schema/src/language-server/main.js")🔗 Analysis chain
Line range hint
35-54: Verify resource paths existenceThe prepareSandbox task copies multiple resources. Let's verify that all source paths exist to prevent build failures.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify existence of required resource files # Expected: All source paths should exist echo "Checking language server main.js:" fd main.js --type f --full-path --base-directory "packages/schema/bundle/language-server" echo -e "\nChecking stdlib.zmodel:" fd stdlib.zmodel --type f --base-directory "packages/schema/src/res" echo -e "\nChecking TextMate bundle:" fd "zmodel.t?mbundle|zmodel.tmLanguage" --type fLength of output: 626
Script:
#!/bin/bash # Let's verify the existence of the language server directory and main.js file echo "Checking language server path structure:" fd --type d "language-server" --full-path fd "main.js" --type f echo -e "\nChecking complete directory structure for schema bundle:" fd . --type d --max-depth 3 packages/schema echo -e "\nChecking zmodel.tmbundle in IDE directory:" fd "zmodel.tmbundle" --type d packages/ideLength of output: 1282
packages/plugins/openapi/src/rest-generator.ts (1)
912-913: LGTM! Consider fixing indentation.The logic for handling ID requirements in create operations is correct. The code properly checks for single ID fields without default values.
The indentation appears to be inconsistent with the surrounding code. Consider adjusting it to match the file's indentation style:
- if (idFields.length === 1 && !hasAttribute(idFields[0], '@default')) { - properties = { id: { type: 'string' }, ...properties }; + if (idFields.length === 1 && !hasAttribute(idFields[0], '@default')) { + properties = { id: { type: 'string' }, ...properties };packages/server/tests/api/rest.test.ts (1)
1875-1910: Enhance the test by asserting specific error detailsTo make the test more robust, consider adding assertions to verify the exact error code and message returned. This ensures the test not only checks the status code but also the correctness of the error response.
Apply this diff to enhance the test:
expect(r.status).toBe(400); + expect(r.body.errors[0].code).toBe('invalid-payload'); + expect(r.body.errors[0].title).toBe('Invalid payload'); + expect(r.body.errors[0].reason).toContain('Unique constraint validation failed');
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (15)
package.jsonis excluded by!**/*.jsonpackages/ide/jetbrains/package.jsonis excluded by!**/*.jsonpackages/language/package.jsonis excluded by!**/*.jsonpackages/misc/redwood/package.jsonis excluded by!**/*.jsonpackages/plugins/openapi/package.jsonis excluded by!**/*.jsonpackages/plugins/openapi/tests/baseline/rest-3.0.0.baseline.yamlis excluded by!**/*.yamlpackages/plugins/openapi/tests/baseline/rest-3.1.0.baseline.yamlis excluded by!**/*.yamlpackages/plugins/swr/package.jsonis excluded by!**/*.jsonpackages/plugins/tanstack-query/package.jsonis excluded by!**/*.jsonpackages/plugins/trpc/package.jsonis excluded by!**/*.jsonpackages/runtime/package.jsonis excluded by!**/*.jsonpackages/schema/package.jsonis excluded by!**/*.jsonpackages/sdk/package.jsonis excluded by!**/*.jsonpackages/server/package.jsonis excluded by!**/*.jsonpackages/testtools/package.jsonis excluded by!**/*.json
📒 Files selected for processing (4)
packages/ide/jetbrains/build.gradle.kts(1 hunks)packages/plugins/openapi/src/rest-generator.ts(2 hunks)packages/server/src/api/rest/index.ts(5 hunks)packages/server/tests/api/rest.test.ts(2 hunks)
🔇 Additional comments (11)
packages/ide/jetbrains/build.gradle.kts (3)
Line range hint 69-77: Security configuration looks good
The plugin signing and publishing configuration properly uses environment variables for sensitive data (CERTIFICATE_CHAIN, PRIVATE_KEY, PRIVATE_KEY_PASSWORD, PUBLISH_TOKEN).
12-12: Verify version bump consistency across the project
The version bump from 2.9.3 to 2.9.4 should be reflected in other relevant project files.
✅ Verification successful
Version bump is consistent across the project
The version 2.9.4 is correctly reflected in all relevant project files:
- All main package.json files in the packages directory show version 2.9.4
- The build.gradle.kts version matches at 2.9.4
- The only 2.9.3 references found are in pnpm-lock.yaml for an unrelated dependency (colord)
- Other version numbers found (0.1.0, 1.0.0) belong to independent packages
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for version references across the project
# Expected: All version references should be updated to 2.9.4
# Search for version strings
echo "Searching for version strings:"
rg -g '!{*.lock,package-lock.json,yarn.lock}' '2\.9\.[34]'
# Check package.json files specifically
echo -e "\nChecking package.json files:"
fd package.json --exec jq -r '. | select(.version != null) | .version'
Length of output: 1315
Line range hint 20-24: Verify IDE compatibility range
The plugin is configured for IntelliJ IDEA Ultimate 2023.3.2 with JavaScript support. The compatibility range (233.2 - 251.*) looks appropriate for the current version.
✅ Verification successful
IDE compatibility range is correctly configured and consistent
The verification confirms that the IDE compatibility range is properly set and consistent across configuration files:
- In
plugin.xml:since-build="233.2"is set - In
build.gradle.kts:- IDE version is set to "2023.3.2"
- Compatibility range is set to
sinceBuild.set("233.2")anduntilBuild.set("251.*")
- Required dependencies are properly declared: IntelliJ Ultimate, JavaScript, and TextMate plugins
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the changelog contains appropriate compatibility notes
# Expected: Changelog should mention IDE compatibility requirements
# Check changelog entries for IDE compatibility notes
rg -i "compatibility|ide version|intellij" "CHANGELOG.md"
Length of output: 57
Script:
#!/bin/bash
# Let's check if CHANGELOG.md exists and its content
fd -t f "CHANGELOG.md"
# Check the plugin.xml file for IDE compatibility settings
fd -t f "plugin.xml" -x cat {}
# Check build.gradle.kts files for IDE version configurations
fd -t f "build.gradle.kts" -x cat {}
Length of output: 6754
packages/plugins/openapi/src/rest-generator.ts (1)
860-861: LGTM! Verify compound ID handling consistency.
The change correctly exposes all fields for models with compound IDs, which is the expected behavior for handling composite keys in REST APIs.
Let's verify the compound ID handling across the codebase:
✅ Verification successful
Compound ID handling is correctly implemented and verified
The code change is consistent with the codebase's handling of compound IDs. Evidence from the codebase shows:
- The
isIdFieldutility function properly identifies both single and compound ID fields - Extensive test coverage for compound ID scenarios in REST API endpoints
- Consistent handling across the codebase, including:
- REST parameter generation
- URL path handling with custom separators
- Proper serialization and deserialization of compound IDs
- Support in CRUD operations (create, read, update, delete)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for models with compound IDs and their usage
# Expected: Find models with multiple @id fields to ensure consistent handling
# Search for model definitions with multiple @id fields
ast-grep --pattern 'model $MODEL {
$$$
@id
$$$
@id
$$$
}'
Length of output: 69
Script:
#!/bin/bash
# Let's try a different approach to find compound ID handling
# First, let's look for the isIdField function definition and its usage
ast-grep --pattern 'function isIdField($_) {
$$$
}'
# Also search for any references to compound IDs in comments or code
rg -i "compound.?id|multiple.?id" -A 2 -B 2
Length of output: 14478
packages/server/src/api/rest/index.ts (3)
212-217: 'upsertMetaSchema' is properly defined
The upsertMetaSchema correctly validates the meta field for upsert operations, ensuring that the operation is 'upsert' and that matchFields is a non-empty array of strings.
319-341: Logical handling of upsert and create operations
The code correctly differentiates between upsert and create operations by using upsertMetaSchema.safeParse(body). If parsing succeeds, it proceeds with processUpsert; otherwise, it defaults to processCreate.
1074-1077: Correctly updating relationships with 'connect'
The code accurately updates single relationships by using the connect operation with the appropriate identifier key.
packages/server/tests/api/rest.test.ts (4)
1803-1839: Well-structured test for upserting a new entity
The test accurately simulates the upsert of a new user entity and verifies the expected response.
1842-1872: Well-structured test for upserting an existing entity
The test correctly checks that upserting an existing user updates the entity as expected.
1913-1935: Well-structured test for upserting with a compound ID
The test effectively verifies that upsert operations work correctly with entities that have compound IDs.
2144-2166: Test accurately updates the ID of an item with a compound ID
The test correctly checks the ability to update the ID of an item with a compound ID, ensuring relationships are maintained properly.
… comments (#1887) Co-authored-by: Simon Zimmerman <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
packages/plugins/tanstack-query/tests/react-hooks-v5.test.tsx (1)
Line range hint
470-488: Address the TODO: Include thecategoryobject in the assertionThe test contains a TODO comment questioning whether the
categoryobject should be included in the assertion. To ensure the test accurately reflects the expected behavior, consider updating the assertion to include thecategoryobject if it should be present in the cached data.Would you like assistance in updating the assertion to include the
categoryobject?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
packages/plugins/tanstack-query/tests/react-hooks-v5.test.tsx(2 hunks)packages/plugins/tanstack-query/tests/test-model-meta.ts(0 hunks)
💤 Files with no reviewable changes (1)
- packages/plugins/tanstack-query/tests/test-model-meta.ts
🔇 Additional comments (2)
packages/plugins/tanstack-query/tests/react-hooks-v5.test.tsx (2)
Line range hint 504-581: Test for optimistic update with optional one-to-many relationship is correctly implemented
The test correctly verifies that an optimistic update works as expected when the related optional one-to-many relationship is null. The cache update and assertions are properly handled.
Line range hint 425-430: Verify server response data in nock mocks returning { data: null }
In the nock mock for the POST request to create a Post, the server response is mocked to return { data: null }. Please ensure that this aligns with the expected server behavior. If the server is expected to return the created data, consider returning the created post data to more accurately simulate real-world scenarios.
Run the following script to find all nock mocks that reply with { data: null }:
This script searches for instances where nock mocks for POST, PUT, or DELETE requests reply with { data: null }. Verify whether these responses are intentional and align with the expected server behavior.
✅ Verification successful
After examining the test file, I can now generate the final response:
The mock response of { data: null } is intentional and correct for optimistic updates
The test file shows that returning { data: null } in the nock mock is intentional and correct for the optimistic update test cases. This is because these tests are specifically verifying that the optimistic updates work correctly without waiting for the actual server response. The tests verify that:
- The cache is updated optimistically before the server responds
- The optimistic update works even when the server returns null
- The optimistic flag (
$optimistic: true) is properly set on the updated data
This pattern is consistently used across multiple test cases in the file, including:
optimistic create singleoptimistic create updating nested queryoptimistic create updating deeply nested query
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Find all nock mocks that reply with { data: null }
rg -A 5 'nock\(.*\)\.(post|put|delete)\(.*\)\s*\.reply\(200,.*\{\s*data:\s*null\s*\}\)'
Length of output: 89
Script:
#!/bin/bash
# Let's first find the specific test file and examine the nock mock setup
cat "packages/plugins/tanstack-query/tests/react-hooks-v5.test.tsx"
Length of output: 62295
No description provided.