Feature/edit collection - #364
Merged
Merged
Conversation
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
Warning Review limit reached
Next review available in: 31 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: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds end-to-end collection schema editing with shared validation, a protected API endpoint, dashboard editor UI, users-schema safeguards, persistence, cache refresh, events, index synchronization, and tests. Also updates CORS preflight caching and documents dynamic connection handling. ChangesCollection schema editing
CORS preflight caching
Dynamic connection flow documentation
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant EditCollection
participant ProjectsRouter
participant updateCollection
participant Project
participant CacheHelpers
participant emitEvent
User->>EditCollection: Edit and submit collection schema
EditCollection->>ProjectsRouter: PUT project collection schema
ProjectsRouter->>updateCollection: Apply authorization middleware
updateCollection->>Project: Validate input and load project
updateCollection->>Project: Update model and save project
updateCollection->>CacheHelpers: Refresh project cache
updateCollection->>emitEvent: Emit collection_updated
updateCollection-->>EditCollection: Return sanitized project
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 14
🤖 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 `@apps/dashboard-api/src/controllers/project.controller.js`:
- Around line 962-993: The endpoint response paths around project collection
updates must use the mandated { success, data, message } envelope consistently.
Update the project/collection not-found, users-schema validation, success, and
catch responses in this controller method, replacing raw err.message exposure
with the established AppError and sanitized error-handling pattern while
preserving the existing status codes and payload meaning.
- Around line 950-976: Prevent editCollection from overwriting an existing
collection model when the optional schema is omitted. Update the assignment
after the users-schema validation to replace
project.collections[collectionIndex].model only when schema is defined,
preserving the stored model for requests without a schema.
- Around line 980-986: Update the project cache refresh logic around
deleteProjectById and setProjectById in updateCollection to also invalidate both
API-key caches using deleteProjectByApiKeyCache for the project’s publishableKey
and secretKey, matching createCollection behavior. Perform invalidation before
or alongside repopulating the project-by-id cache while preserving the existing
sanitized project object handling.
- Around line 976-978: Update updateCollection after assigning
project.collections[collectionIndex].model and before/around project.save to
recompile the edited collection’s model and invoke createUniqueIndexes, matching
createCollection’s behavior. Ensure unique indexes are created, changed, or
dropped to reflect additions, removals, and modifications to the collection
schema.
In `@apps/web-dashboard/src/pages/Auth.jsx`:
- Around line 553-564: The ConfirmationModal usage for
isEditUsersSchemaModalOpen shows a misleading hardcoded Delete action. Update
ConfirmationModal to accept configurable confirm-label and styling/variant
props, then use them in this editing-schema modal to display an appropriate
non-delete label and button style while preserving the existing confirmation and
cancellation behavior.
In `@apps/web-dashboard/src/pages/EditCollection.jsx`:
- Around line 576-588: The editable Name field conflicts with handleSubmit,
which only submits schema and retains the original collectionName. Either make
the Name input non-editable for all edit flows, or update handleSubmit and the
corresponding API handling to explicitly submit and apply the renamed name while
preserving the existing users restriction.
- Around line 479-486: Update the local mapFields function in the
existingCollection model-loading flow to recursively map both f.fields and
array-item sub-fields under f.items.fields, generating a fresh _id for every
nested field. Preserve the current object-nesting behavior and ensure the
transformed array item structure remains intact for FieldRow keys.
In `@dynamic_connection_sequence_diagram.md`:
- Line 216: Update the In-Memory Registry row’s idle-duration text from “20min”
to the standard spaced form “20 min” or “20 minutes,” preserving the rest of the
documentation unchanged.
- Around line 214-218: Update the Tier 3 row in the caching sequence table so
the “What’s Cached” column no longer describes raw encrypted configuration as
cached data; rename the column to “Source/operation” or describe Tier 3 as the
main MongoDB lookup and decryption source-of-truth fallback.
- Around line 238-253: Update the links in the “Key Files” table to use
repository-relative paths instead of machine-specific file:/// URLs. Preserve
each referenced file and its role while removing the local drive prefix.
- Around line 196-198: Update the PHASE 6 response example in the sequence
diagram to include the required message field alongside success and data, using
the documented empty-string message value so the canonical shape is { success,
data, message }.
- Around line 84-92: Update the POST Request (Write) authorization notes to
state that secret-key writes bypass authorization immediately, while valid
Bearer-token and ownership checks apply only to publishable-key writes. Clarify
that ownership validation and auto-injection use the configured ownerField
rather than always userId.
- Around line 54-64: The diagram’s cache lookup flow should distinguish key
types: show publishable-key verification trying the raw key first and falling
back to the hashed key, while secret-key verification uses the hashed key
directly. Update the Redis lookup notation and surrounding flow near “Check
project cache” without changing the documented database fallback behavior.
- Around line 229-232: Specify the fenced code block language for the database
naming examples by adding the `text` identifier to the existing fence, without
changing the example content.
🪄 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 Plus
Run ID: 1510a1f4-f7eb-4dff-a579-75f9f9fc0abc
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (13)
apps/dashboard-api/src/__tests__/project.controller.editCollection.test.jsapps/dashboard-api/src/__tests__/routes.projects.storage.test.jsapps/dashboard-api/src/app.jsapps/dashboard-api/src/controllers/project.controller.jsapps/dashboard-api/src/routes/projects.jsapps/web-dashboard/src/App.jsxapps/web-dashboard/src/components/Database/DatabaseHeader.jsxapps/web-dashboard/src/pages/Auth.jsxapps/web-dashboard/src/pages/Database.jsxapps/web-dashboard/src/pages/EditCollection.jsxdynamic_connection_sequence_diagram.mdpackages/common/src/index.jspackages/common/src/utils/input.validation.js
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/common/src/index.js (1)
95-95: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winExport
editCollectionSchema, notsyncSchemaPayload.The supplied validation module defines
editCollectionSchema, andproject.controller.jsinvokeseditCollectionSchema.parse(...). Re-exportingsyncSchemaPayloadleaves the controller’s expected schema unavailable and can break the collection update endpoint at runtime.Proposed fix
- syncSchemaPayload, + editCollectionSchema,Apply the same rename in both the destructuring import and
module.exports.Also applies to: 182-182
🤖 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/common/src/index.js` at line 95, Replace syncSchemaPayload with editCollectionSchema in both the validation-module destructuring import and module.exports in the common index module, so the controller’s editCollectionSchema.parse call receives the expected schema.
🧹 Nitpick comments (1)
apps/dashboard-api/src/controllers/project.controller.js (1)
979-997: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winConnection lookup / model recompile / index creation run even when
schemais omitted.
editCollectionSchema.schemais optional, and per Line 970 the model is only reassigned whenschema !== undefined. But lines 981-997 (getConnection,clearCompiledModel,getCompiledModel,createUniqueIndexes) execute unconditionally, and thecollection_updatedevent still fires at Line 1006 — so a request that omitsschemadoes needless connection/model work and emits a misleading "updated" event for a no-op.♻️ Suggested guard
- const connection = await getConnection(projectId); - const finalCollectionName = project.resources.db.isExternal - ? collectionName - : `${project._id}_${collectionName}`; - - clearCompiledModel(connection, finalCollectionName); - - const Model = getCompiledModel( - connection, - project.collections[collectionIndex], - projectId, - project.resources.db.isExternal, - ); - - if (project.collections[collectionIndex].model) { - await createUniqueIndexes(Model, project.collections[collectionIndex].model); - } + if (schema !== undefined) { + const connection = await getConnection(projectId); + const finalCollectionName = project.resources.db.isExternal + ? collectionName + : `${project._id}_${collectionName}`; + + clearCompiledModel(connection, finalCollectionName); + + const Model = getCompiledModel( + connection, + project.collections[collectionIndex], + projectId, + project.resources.db.isExternal, + ); + + await createUniqueIndexes(Model, project.collections[collectionIndex].model); + }🤖 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 `@apps/dashboard-api/src/controllers/project.controller.js` around lines 979 - 997, Guard the connection lookup, compiled-model clearing/recreation, and unique-index creation after project.save() with the same schema-present condition used when reassigning the model in editCollectionSchema. Also emit the collection_updated event only when schema !== undefined, so requests omitting schema perform no model work and do not report a misleading update.
🤖 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 `@apps/dashboard-api/src/controllers/project.controller.js`:
- Around line 995-997: Update the model synchronization flow around
createUniqueIndexes in the project collection handling to also remove stale
unique_${normalizedKey}_1 indexes that are no longer represented by the current
schema, including indexes created outside this path. Reuse or extend the
existing index-sync mechanism so MongoDB no longer enforces uniqueness after a
field loses unique: true, while preserving creation of current unique indexes.
---
Outside diff comments:
In `@packages/common/src/index.js`:
- Line 95: Replace syncSchemaPayload with editCollectionSchema in both the
validation-module destructuring import and module.exports in the common index
module, so the controller’s editCollectionSchema.parse call receives the
expected schema.
---
Nitpick comments:
In `@apps/dashboard-api/src/controllers/project.controller.js`:
- Around line 979-997: Guard the connection lookup, compiled-model
clearing/recreation, and unique-index creation after project.save() with the
same schema-present condition used when reassigning the model in
editCollectionSchema. Also emit the collection_updated event only when schema
!== undefined, so requests omitting schema perform no model work and do not
report a misleading update.
🪄 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 Plus
Run ID: c5b89cad-4024-4007-a4ed-d40df255cd2f
📒 Files selected for processing (11)
apps/dashboard-api/src/__tests__/project.controller.editCollection.test.jsapps/dashboard-api/src/app.jsapps/dashboard-api/src/controllers/project.controller.jsapps/dashboard-api/src/routes/projects.jsapps/web-dashboard/src/App.jsxapps/web-dashboard/src/pages/Auth.jsxapps/web-dashboard/src/pages/ConfirmationModal.jsxapps/web-dashboard/src/pages/EditCollection.jsxdynamic_connection_sequence_diagram.mdpackages/common/src/index.jspackages/common/src/utils/input.validation.js
🚧 Files skipped from review as they are similar to previous changes (8)
- apps/web-dashboard/src/App.jsx
- apps/dashboard-api/src/routes/projects.js
- packages/common/src/utils/input.validation.js
- apps/dashboard-api/src/app.js
- apps/web-dashboard/src/pages/Auth.jsx
- apps/dashboard-api/src/tests/project.controller.editCollection.test.js
- dynamic_connection_sequence_diagram.md
- apps/web-dashboard/src/pages/EditCollection.jsx
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/dashboard-api/src/controllers/project.controller.js (1)
1015-1020: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winCatch-all still forwards raw
err.message, risking MongoDB detail/PII leakage.Non-Zod errors (e.g., from
project.save(),getConnection,getCompiledModel,createUniqueIndexes) fall through tonew AppError(err.status || 400, err.message), surfacing raw driver/Mongoose messages (which can include field values such as duplicate-key data) directly to the client. This is the same class of concern raised previously on this endpoint; the response envelope and early-return paths are now compliant, but this fallback branch still leaks internal error text.🛡️ Proposed fix
} catch (err) { if (err instanceof z.ZodError) { return next(new AppError(400, err.issues?.[0]?.message || "Validation failed")); } - return next(new AppError(err.status || 400, err.message)); + if (err instanceof AppError) { + return next(err); + } + console.error("[updateCollection] Unexpected error:", err); + return next(new AppError(500, "Failed to update collection")); }As per coding guidelines, "use
AppErrorfor errors, never rawthrow, and never expose MongoDB errors to clients."🤖 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 `@apps/dashboard-api/src/controllers/project.controller.js` around lines 1015 - 1020, Update the non-Zod fallback in the controller’s catch block to stop passing raw err.message into AppError. Preserve the status when available, but replace the client-facing message with a generic safe error message so errors from project.save, getConnection, getCompiledModel, and createUniqueIndexes cannot expose MongoDB or Mongoose details.Source: Coding guidelines
🤖 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/common/src/utils/injectModel.js`:
- Around line 222-232: Move the stale-index removal loop around existingIndexes
so it runs only after duplicate validation and all index creation requirements
succeed. Ensure validation and createIndex failures leave existing unique_*
indexes untouched, while preserving the current drop criteria and createdIndexes
tracking.
- Around line 226-230: Update the dropIndex error handling in the index-refresh
flow around Model.collection.dropIndex so it ignores only errors verified to
mean the index is already absent. Re-throw permission, connectivity,
duplicate-index, and any other unexpected errors to ensure the unique-index
refresh fails instead of continuing.
- Around line 212-225: Update the unique-index reconciliation around
schemaUniqueKeys and existingIndexes to track each field’s desired
required/partial options, not only its index name. Compare existing unique-index
options with those desired options and drop incompatible indexes before
duplicate validation and createIndex. Ensure index-creation failures are caught
and propagated or reported as failures rather than allowing the operation to
report success.
---
Outside diff comments:
In `@apps/dashboard-api/src/controllers/project.controller.js`:
- Around line 1015-1020: Update the non-Zod fallback in the controller’s catch
block to stop passing raw err.message into AppError. Preserve the status when
available, but replace the client-facing message with a generic safe error
message so errors from project.save, getConnection, getCompiledModel, and
createUniqueIndexes cannot expose MongoDB or Mongoose details.
🪄 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 Plus
Run ID: 596e159d-6bd5-455c-97bf-d2e7e15f05f1
📒 Files selected for processing (2)
apps/dashboard-api/src/controllers/project.controller.jspackages/common/src/utils/injectModel.js
…ify duplicates as 422 validations
yash-pouranik
temporarily deployed
to
feature/edit-collection - urBackend-frankfrut PR #364
July 24, 2026 19:55 — with 
Render
Destroyed
Render
Destroyed
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
1 participant
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.
EDIT COLLECTION FEATURE
Summary by CodeRabbit