-
Notifications
You must be signed in to change notification settings - Fork 5
FIX: validate duplicate vaults #439
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
guimroque
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review - Summary
What was done
Implemented server-side validation to prevent duplicate vault/predicate names within a workspace. Added a validateUniqueName method that checks for existing predicates with the same name (case-insensitive) and throws a BadRequest error if duplicates are found.
Positive Points
- ✅ Addresses a real data integrity issue by moving validation from client to server
- ✅ Uses case-insensitive comparison for better UX
- ✅ Properly excludes soft-deleted records with
deletedAt IS NULL - ✅ Returns meaningful error messages with context
- ✅ Follows existing error handling patterns with GeneralError types
Issues Found
- 🔴 CRITICAL: Missing validation on update operations - only validates on create
- 🟡 IMPORTANT: Import statement formatting inconsistency
- 🟡 IMPORTANT: Console.log should be replaced with proper logging
- 🔵 SUGGESTION: Method could be made static for better reusability
Total comments: 4 (1 critical, 2 important, 1 suggestion)
guimroque
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! ✅
Previous issues have been fixed. Code approved.
Description
Enforces unique name validation for vaults/predicates in the API.
Previously, this check was only performed on the front-end, allowing edits to succeed with duplicate names in scenarios with slow internet or multiple quick clicks. Now, the API returns a
BadRequestwhen a duplicate name is detected, ensuring data integrity.Summary
BadRequestif the name already exists in the corresponding workspace.create() method inPredicateServiceto properly handle expected errors versus internal errors.Checklist