Handle website workflows #451
Conversation
WalkthroughThis pull request introduces several code refactorings and structural improvements. The controller file now reformats a function call for improved readability. In the module service, the update logic for handling platforms has been restructured and a special case for the Hivemind website has been extracted into its own helper function. Additionally, the platform service now includes new methods for handling updated resources and cleaning up schedules associated with platform types like "Website" and "Discourse". Changes
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🧰 Additional context used🧬 Code Definitions (1)src/services/platform.service.ts (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
🔇 Additional comments (6)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/services/module.service.ts (1)
78-86: Clarify global option logic.
Relying onplatforms[0].name === undefinedto identify a “global option” can be misleading. Consider using a more explicit property or logic to distinguish it.src/services/platform.service.ts (1)
323-346: Centralized logic for platform cleanup.
This is a clean approach. Consider capturing potential errors from schedule deletion if robust error handling is needed.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/controllers/hivemind.controller.ts(1 hunks)src/services/module.service.ts(1 hunks)src/services/platform.service.ts(4 hunks)
🧰 Additional context used
🧬 Code Definitions (2)
src/services/module.service.ts (1)
src/services/index.ts (2)
platformService(30-30)websiteService(40-40)
src/services/platform.service.ts (1)
src/services/index.ts (3)
discourseService(37-37)websiteService(40-40)moduleService(33-33)
🔇 Additional comments (8)
src/controllers/hivemind.controller.ts (1)
12-17: Improve input validation for request body fields.
The new multi-line invocation is more readable, but consider adding checks for undefined or malformed request body fields to avoid runtime errors.src/services/module.service.ts (4)
66-68: Confirm early return logic.
Returning early if noplatformsis present might skip other updates inupdateBody. Confirm that this is desired behavior.
70-74: Ensuring a validmodule.options.platformsarray.
This approach looks good, as it prevents null references. No immediate concerns.
76-77: No concerns with assignment.
88-101: Check for duplicate platform naming.
Currently,.find((p) => p.name === newPlatform.name)will return only the first match. If multiple platforms share the same name, it could cause conflicts. Ensure this logic aligns with your data model.src/services/platform.service.ts (3)
5-5: Import references look correct.
These new imports align with the expanded functionalities in this file.Also applies to: 12-12, 15-15
127-131: Inline special-case handling for Website.
The approach is consistent with the rest of the code. No immediate concerns.
149-149: Good practice callinghandlePlatformCleanupbefore deletion.
This ensures schedules or resources are cleaned up properly.
| /** | ||
| * Handle special case for Hivemind module with Website platform | ||
| * @param {Object} platform - Platform object | ||
| */ | ||
| const handleHivemindWebsiteCase = async (platform: any) => { | ||
| const platformDoc = await platformService.getPlatformById(platform.platform); | ||
|
|
||
| if (!platformDoc) return; | ||
|
|
||
| const isActivated = platform.metadata?.activated; | ||
| const existingScheduleId = platformDoc.get('metadata.scheduleId'); | ||
|
|
||
| if (isActivated === true) { | ||
| if (!existingScheduleId) { | ||
| const scheduleId = await websiteService.coreService.createWebsiteSchedule(platform.platform); | ||
| platformDoc.set('metadata.scheduleId', scheduleId); | ||
| await platformDoc.save(); | ||
| } | ||
| } else if (isActivated === false) { | ||
| if (existingScheduleId) { | ||
| await websiteService.coreService.deleteWebsiteSchedule(existingScheduleId); | ||
| platformDoc.set('metadata.scheduleId', null); | ||
| await platformDoc.save(); | ||
| } | ||
| } | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use stronger typing for platform in handleHivemindWebsiteCase.
(platform: any) makes it easy to introduce type-related mistakes. Consider using a typed interface or type assertion to guarantee that critical fields like platform.platform and platform.metadata exist.
| /** | ||
| * Handle Website platform resource changes | ||
| * @param {HydratedDocument<IPlatform>} platform - Platform document | ||
| * @param {Partial<IPlatform>} updateBody - Update body | ||
| * @returns {Promise<void>} | ||
| */ | ||
| const handleWebsiteResourceChanges = async ( | ||
| platform: HydratedDocument<IPlatform>, | ||
| updateBody: Partial<IPlatform>, | ||
| ): Promise<void> => { | ||
| if (!updateBody.metadata?.resources || !platform.metadata?.resources) { | ||
| return; | ||
| } | ||
| const oldResources = JSON.stringify(platform.metadata.resources.sort()); | ||
| const newResources = JSON.stringify(updateBody.metadata.resources.sort()); | ||
|
|
||
| if (oldResources !== newResources) { | ||
| const existingScheduleId = platform.metadata.scheduleId; | ||
|
|
||
| if (existingScheduleId) { | ||
| await websiteService.coreService.deleteWebsiteSchedule(existingScheduleId); | ||
| updateBody.metadata.scheduleId = null; | ||
| } | ||
|
|
||
| const moduleFilter = { | ||
| name: ModuleNames.Hivemind, | ||
| 'options.platforms': { | ||
| $elemMatch: { | ||
| name: PlatformNames.Website, | ||
| platform: platform._id, | ||
| 'metadata.activated': true, | ||
| }, | ||
| }, | ||
| }; | ||
|
|
||
| const hivemindModule = await moduleService.getModuleByFilter(moduleFilter); | ||
|
|
||
| if (hivemindModule) { | ||
| const scheduleId = await websiteService.coreService.createWebsiteSchedule(platform._id); | ||
| updateBody.metadata.scheduleId = scheduleId; | ||
| } | ||
| } | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Potential pitfalls in resource comparison.
Sorting and stringifying arrays might not always produce accurate equality checks for nested objects or unsorted data. Consider a more robust deep equality check if resources can contain objects of arbitrary structure.
Summary by CodeRabbit
New Features
Refactor