refactor: update default service and model in createAgentController (…#897
Conversation
…alkover-Web-Solution#888) * refactor: update default service and model in createAgentController * Update src/controllers/agentConfig.controller.js Co-authored-by: windsurf-bot[bot] <189301087+windsurf-bot[bot]@users.noreply.github.com> --------- Co-authored-by: Husain Baghwala <husainhackerrank@gmail.com> Co-authored-by: windsurf-bot[bot] <189301087+windsurf-bot[bot]@users.noreply.github.com>
|
Your card was not found on the dbdash with the branch final-schema-for-configuration-versions |
![windsurf-bot[bot]](https://avatars.githubusercontent.com/in/1066231?s=60&v=4){kind=small}
There was a problem hiding this comment.
Other comments (4)
- migrations/mongo/configurationAndVersions.js (140-202) The MongoDB operations in Step 4 (renaming fields) lack error handling. Consider wrapping these operations in try/catch blocks to ensure the script can continue or at least clean up properly if one operation fails.
- src/mongoModel/schemas/shared.js (56-59) There's an inconsistency in naming between schemas: fallBackSchema uses `is_enable` while guardrailsSchema uses `is_enabled`. Consider standardizing to either `is_enable` or `is_enabled` across all schemas for consistency.
- src/mongoModel/schemas/shared.js (2-2) There's a commented import for ModelConfigService. Is this import still needed? If not, consider removing the comment to keep the code clean. If it will be needed in the future, consider adding a TODO comment explaining why it's commented out.
- src/mongoModel/schemas/shared.js (106-106) The field `IsstarterQuestionEnable` doesn't follow the camelCase naming convention used for other fields in this file. Consider renaming to `isStarterQuestionEnabled` for consistency with JavaScript naming conventions.
💡 To request another review, post a new comment with "/windsurf-review".
| const copyImageSizeToSizeConfig = await configurations.updateMany( | ||
| { "configuration.image_size": { $exists: true }, "configuration.size": { $exists: false } }, | ||
| [{ $set: { "configuration.size": "$configuration.image_size" } }] | ||
| ); |
There was a problem hiding this comment.
The updateMany operations for copying image_size to size are using an incorrect syntax. When using an aggregation pipeline with $set, you should not wrap it in an array.
| const copyImageSizeToSizeConfig = await configurations.updateMany( | |
| { "configuration.image_size": { $exists: true }, "configuration.size": { $exists: false } }, | |
| [{ $set: { "configuration.size": "$configuration.image_size" } }] | |
| ); | |
| const copyImageSizeToSizeConfig = await configurations.updateMany( | |
| { "configuration.image_size": { $exists: true }, "configuration.size": { $exists: false } }, | |
| [{ $set: { "configuration.size": "$configuration.image_size" } }] |
Same issue exists in the versions collection update on line 183.
| const actionTypeSchema = new Schema( | ||
| { | ||
| description: { type: String }, | ||
| type: { type: String }, | ||
| variable: { type: String } | ||
| }, | ||
| { _id: false } |
There was a problem hiding this comment.
The actionTypeSchema doesn't specify default values for its fields, unlike other schemas in this file. Consider adding appropriate default values to maintain consistency and prevent potential issues with undefined values.
| const setDefaultSizeVersion = await versions.updateMany( | ||
| { "configuration.image_size": { $exists: false }, "configuration.size": { $exists: false } }, | ||
| { $set: { "configuration.size": "" } } | ||
| ); | ||
| console.log(` ✓ Set default size (where both missing): ${setDefaultSizeVersion.modifiedCount} versions`); |
There was a problem hiding this comment.
The script sets a default empty string for size when both image_size and size are missing in versions (lines 197-201), but doesn't do the same for configurations. This could lead to inconsistency between collections. Consider adding a similar operation for configurations.
|
Your card was not found on the dbdash with the branch final-schema-for-configuration-versions |
3 participants
final schema for configuration and version and its migration