Feat/fix cache testcase issues#86
Merged
Merged
Conversation
…rigin - Removed Access-Control-Allow-Origin from netlify.toml. - Added middleware to vary the Origin header in storage and verify functions. - Included a new integration test to validate CORS response for TradeTrust production website.
- Upgraded @trustvc/trustvc to version 2.12.2. - Updated @digitalbazaar/di-sd-primitives to version 3.3.0 and its dependencies. - Changed JSON schema version in document-sepolia.json to 3.0 and updated its structure to align with the new schema.
- Updated @types/node to version 22.19.15. - Added undici-types version 6.20.0 to the package-lock.json.
- Updated multiple dependencies to their latest versions, including @aws-sdk/client-kms and related packages. - Added resolved URLs and integrity hashes for several packages to ensure package integrity.
✅ Deploy Preview for tradetrust-functions ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe PR removes wildcard CORS headers and adds Origin-varying middleware to serverless functions while migrating integration tests to V3 Sepolia documents. The ChangesCORS and Origin-Varying Configuration with V3 Document Support
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 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🧪 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 |
- Modified the npm install command in the GitHub Actions workflow to use the --omit=optional flag, improving installation efficiency by excluding optional dependencies.
- Downgraded @trustvc/trustvc to version 2.0.7 and @types/node to version 22.0.0 in package-lock.json. - Removed resolved URLs and integrity hashes for several packages to streamline the lock file. - Updated the npm install command in the GitHub Actions workflow to include all dependencies.
- Changed the npm install command from 'npm ci' to 'npm i' in both manual_forked.yml and manual.yml workflows to streamline dependency installation.
- Removed optional dependencies from package-lock.json to streamline the file. - Added a new dependency for @netlify/blobs version 6.5.0 in package-lock.json. - Updated npm install command in manual_forked.yml and manual.yml workflows to include all dependencies.
…npm ci - Changed the npm install command from 'npm i' to 'npm ci' in both manual_forked.yml and manual.yml workflows for improved consistency and reliability in dependency installation.
- Changed the schema version in document-sepolia.json from 3.0 to 2.0. - Updated the structure of the document to align with the new schema, including changes to the data fields and template references. - Adjusted import paths in integration tests to reference the updated document-sepolia.json file.
- Removed import of document-sepolia version 2 and replaced it with version 3 in both document-storage.test.ts and document-verify.test.ts. - Updated postDataSepoliaV2 to use documentSepoliaV3 for consistency across tests.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
tests/integration/document-storage.test.ts (2)
22-22: ⚡ Quick winRename
postDataSepoliaV2to reflect that it now contains a V3 document.The variable
postDataSepoliaV2is assigneddocumentSepoliaV3, creating a misleading name. This inconsistency will confuse future maintainers about what document version is actually being tested.♻️ Suggested refactoring
Either rename the variable to match its content:
-const postDataSepoliaV2 = { document: documentSepoliaV3 }; +const postDataSepoliaV2_MigratedToV3 = { document: documentSepoliaV3 };Or if the intent is to test V3 documents with the V2 endpoint path, clarify this in the variable name and add a comment explaining why V3 documents are sent to a "V2" test case.
🤖 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 `@tests/integration/document-storage.test.ts` at line 22, The variable name postDataSepoliaV2 is misleading because it holds documentSepoliaV3; rename postDataSepoliaV2 to postDataSepoliaV3 (or postDataSepoliaV3ForV2Endpoint if the test intentionally posts a V3 document to a V2 endpoint) and update any references in tests to use the new identifier; if keeping the V2 name for a specific test purpose, add a one-line comment near the declaration explaining why a V3 document is being used with the V2-named variable.
46-59: ⚡ Quick winUpdate test description to reflect V3 document usage.
The test description says "should store encrypted v2 sepolia document" but the test now uses
postDataSepoliaV2which containsdocumentSepoliaV3. This mismatch between the description and actual test data is misleading.♻️ Suggested fix
Update the test description to accurately reflect what's being tested:
- it("should store encrypted v2 sepolia document", async () => { + it("should store encrypted v3 sepolia document (v2 variable migration)", async () => {Or if this test is intended to verify that the endpoint handles both V2 and V3 documents, clarify that in the description and add a comment explaining the test's purpose.
🤖 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 `@tests/integration/document-storage.test.ts` around lines 46 - 59, The test description is misleading: update the it(...) description for the test named "should store encrypted v2 sepolia document" (which sends postDataSepoliaV2 that contains documentSepoliaV3) so it accurately reflects the payload (e.g., "should store encrypted v3 sepolia document") or change the test data/variable name to match V2; ensure references to postDataSepoliaV2 and documentSepoliaV3 are consistent and, if the intent is to cover both versions, broaden the description to state that the endpoint handles V2 and V3 and add a short comment clarifying the purpose.tests/integration/document-verify.test.ts (2)
22-22: ⚡ Quick winRename
postDataSepoliaV2to reflect that it now contains a V3 document.The variable
postDataSepoliaV2is assigneddocumentSepoliaV3, creating the same misleading naming issue as indocument-storage.test.ts. This inconsistency makes it unclear what document version is being tested.♻️ Suggested refactoring
Rename the variable to match its content:
-const postDataSepoliaV2 = { document: documentSepoliaV3 }; +const postDataSepoliaV2_MigratedToV3 = { document: documentSepoliaV3 };Then update all references to this variable throughout the test file (lines 44, 53, 86, 119).
🤖 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 `@tests/integration/document-verify.test.ts` at line 22, The variable name postDataSepoliaV2 is misleading because it holds documentSepoliaV3; rename postDataSepoliaV2 to postDataSepoliaV3 (or another name that reflects V3) and update every reference in this test file to use the new identifier, including usages in assertions and request payloads so the variable name accurately matches documentSepoliaV3 and avoids confusion with V2 fixtures.
48-58: ⚡ Quick winUpdate test description to reflect V3 document usage.
The test description says "should verify a v2 sepolia document with sepolia network query" but uses
postDataSepoliaV2which now containsdocumentSepoliaV3.♻️ Suggested fix
- it("should verify a v2 sepolia document with sepolia network query", async () => { + it("should verify a v3 sepolia document with sepolia network query", async () => {🤖 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 `@tests/integration/document-verify.test.ts` around lines 48 - 58, Update the test's description string to correctly reflect the document version being used: change the it(...) description from "should verify a v2 sepolia document with sepolia network query" to something like "should verify a v3 sepolia document with sepolia network query" (the test block using the it(...) callback and the postDataSepoliaV2 variable which contains documentSepoliaV3 should have a matching description); alternatively, if you intended to test v2, swap the payload to a v2 fixture instead of postDataSepoliaV2 — make the change in the it(...) test declaration and ensure the symbol postDataSepoliaV2 or the payload name aligns with documentSepoliaV3.
🤖 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 `@tests/integration/headers.test.ts`:
- Around line 80-89: The test description is duplicated; update the it(...)
string for the test that sets Origin to "https://ref.tradetrust.io" so it is
unique and specific (e.g., mention the "ref.tradetrust.io" origin). Locate the
test in tests/integration/headers.test.ts (the it block that calls
request.post("/").set("Origin",
\"https://ref.tradetrust.io\").send(postData).expect(200)) and change its
description text accordingly.
---
Nitpick comments:
In `@tests/integration/document-storage.test.ts`:
- Line 22: The variable name postDataSepoliaV2 is misleading because it holds
documentSepoliaV3; rename postDataSepoliaV2 to postDataSepoliaV3 (or
postDataSepoliaV3ForV2Endpoint if the test intentionally posts a V3 document to
a V2 endpoint) and update any references in tests to use the new identifier; if
keeping the V2 name for a specific test purpose, add a one-line comment near the
declaration explaining why a V3 document is being used with the V2-named
variable.
- Around line 46-59: The test description is misleading: update the it(...)
description for the test named "should store encrypted v2 sepolia document"
(which sends postDataSepoliaV2 that contains documentSepoliaV3) so it accurately
reflects the payload (e.g., "should store encrypted v3 sepolia document") or
change the test data/variable name to match V2; ensure references to
postDataSepoliaV2 and documentSepoliaV3 are consistent and, if the intent is to
cover both versions, broaden the description to state that the endpoint handles
V2 and V3 and add a short comment clarifying the purpose.
In `@tests/integration/document-verify.test.ts`:
- Line 22: The variable name postDataSepoliaV2 is misleading because it holds
documentSepoliaV3; rename postDataSepoliaV2 to postDataSepoliaV3 (or another
name that reflects V3) and update every reference in this test file to use the
new identifier, including usages in assertions and request payloads so the
variable name accurately matches documentSepoliaV3 and avoids confusion with V2
fixtures.
- Around line 48-58: Update the test's description string to correctly reflect
the document version being used: change the it(...) description from "should
verify a v2 sepolia document with sepolia network query" to something like
"should verify a v3 sepolia document with sepolia network query" (the test block
using the it(...) callback and the postDataSepoliaV2 variable which contains
documentSepoliaV3 should have a matching description); alternatively, if you
intended to test v2, swap the payload to a v2 fixture instead of
postDataSepoliaV2 — make the change in the it(...) test declaration and ensure
the symbol postDataSepoliaV2 or the payload name aligns with documentSepoliaV3.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 341aa01a-f3df-4451-97a3-7aa59ebb8904
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (7)
netlify.tomlnetlify/functions/storage/index.tsnetlify/functions/verify/index.tspackage.jsontests/integration/document-storage.test.tstests/integration/document-verify.test.tstests/integration/headers.test.ts
💤 Files with no reviewable changes (1)
- netlify.toml
- Changed the test description to explicitly mention the origin URL as https://ref.tradetrust.io for clarity and accuracy in the integration tests.
rongquan1
approved these changes
May 14, 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
2 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.
Summary
What is the background of this pull request?
Changes
Issues
What are the related issues or stories?
Summary by CodeRabbit
Configuration
Tests