refactor(api): update instead of save on sandbox changes #2938
Conversation
Signed-off-by: Ivan Dagelic <[email protected]>
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the sandbox update operations to use repository.update() instead of repository.save() for better performance and clearer intent. The changes replace entity-based save operations with direct update operations using partial objects.
Key Changes:
- Replaced
save()calls withupdate()across multiple sandbox modification methods - Introduced intermediate variables to store resolved values before assignment
- Added conditional logic to only perform updates when necessary
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| public: warmPoolSandbox.public, | ||
| labels: warmPoolSandbox.labels, | ||
| organizationId: warmPoolSandbox.organizationId, | ||
| createdAt: warmPoolSandbox.createdAt, |
There was a problem hiding this comment.
The createdAt field should not be included in the update operation as it represents the original creation timestamp and should remain immutable. Including it in updates could inadvertently modify the creation time if the in-memory object has been altered.
| createdAt: warmPoolSandbox.createdAt, |
| updateData.networkAllowList = warmPoolSandbox.networkAllowList | ||
| } | ||
|
|
||
| await this.sandboxRepository.update(warmPoolSandbox.id, updateData) |
There was a problem hiding this comment.
After the update operation, warmPoolSandbox still contains the old data since update() doesn't refresh the entity. However, this entity is returned at line 547. Consider either refreshing the entity after update or ensuring the returned DTO accurately reflects the persisted state.
Update instead of save on sandbox changes
Minor refactor to use update instead of save on sandbox changes
Documentation
Related Issue(s)
Related to updates made in #2912