fix: trim whitespace from instanceName on creation#2546
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdds trimming of leading/trailing whitespace to instance names at the start of instance creation so only normalized names are persisted and used downstream. Sequence diagram for instanceName trimming during instance creationsequenceDiagram
actor Client
participant InstanceController
participant ChannelController as channelController
Client->>InstanceController: createInstance(instanceData)
InstanceController->>InstanceController: instanceData.instanceName.trim()
InstanceController->>ChannelController: init(instanceData, configService, eventEmitter)
ChannelController-->>Client: createdInstanceResponse
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- Consider avoiding mutation of the
instanceDataDTO inside the controller by normalizinginstanceNameearlier in the request pipeline (e.g., via a validation/transform pipe or factory) so the controller only deals with already-sanitized input. - If there are other code paths that create or update instances (e.g., update endpoints, background jobs, or seeding scripts), it may be safer to centralize
instanceNamenormalization in a shared helper rather than only increateInstanceto keep behavior consistent.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider avoiding mutation of the `instanceData` DTO inside the controller by normalizing `instanceName` earlier in the request pipeline (e.g., via a validation/transform pipe or factory) so the controller only deals with already-sanitized input.
- If there are other code paths that create or update instances (e.g., update endpoints, background jobs, or seeding scripts), it may be safer to centralize `instanceName` normalization in a shared helper rather than only in `createInstance` to keep behavior consistent.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
Thanks for the review! After analyzing the full request pipeline, I believe the current placement is the right approach for this specific case. On moving normalization earlier in the pipeline: On centralizing in a shared helper: On JSONSchema validation: The single-line fix in |
|
Atualizei a base do PR de Pode rebasar em git fetch upstream
git rebase upstream/develop
git push --force-with-leaseDepois disso, o fix de 1 linha ( |
Prevents instances with trailing/leading whitespace in their names from becoming undeletable via the API or UI. Fixes evolution-foundation#2543.
b84e8e3 to
c8add7a
Compare
|
@brunovigo24 fiz um rebase na sua branch removendo commits de Force-push aplicado em Obrigado pela contribuição! |
DavidsonGomes
left a comment
There was a problem hiding this comment.
LGTM após rebase em develop. Fix de 1 linha (instanceData.instanceName?.trim()), seguro, com optional chaining defensivo.
Edge case não-bloqueante para follow-up: nome só com espaços vira string vazia e ainda passa pela validação JSONSchema. Considerar adicionar if (!instanceData.instanceName) throw new BadRequestException('instanceName cannot be empty after trim') em PR futuro.
📋 Description
Applies
.trim()toinstanceNameat the beginning ofcreateInstancein the instance controller. This ensures that any leading or trailing whitespace is removed before the instance name is persisted to the database.🔗 Related Issue
Closes #2543
🧪 Type of Change
🧪 Testing
Steps to reproduce and verify the fix:
"my-instance ")DELETE /instance/delete/my-instance— should return 200 instead of 404✅ Checklist
📝 Additional Notes
The root cause was that instance names with trailing spaces were stored as-is in the database, but URL path parameters used in delete/fetch operations naturally strip that whitespace — causing a name mismatch and a 404. The fix is applied at the entry point of creation so no invalid names ever reach the database.
Summary by Sourcery
Bug Fixes: