fix: handle hivemind website case #482
Conversation
WalkthroughCI workflow now points to a different reusable workflow file. In code, updateModule re-enables a conditional branch to handle the Hivemind + Website case via handleHivemindWebsiteCase, alongside existing Hivemind + MediaWiki handling. No public interfaces changed. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant C as Caller
participant S as module.service.updateModule
participant EP as existingPlatform
participant HPW as handleHivemindWebsiteCase
participant HPM as handleHivemindMediaWikiCase
C->>S: updateModule(module, newPlatform)
alt existing platform present
S->>EP: read existingPlatform metadata
alt module=Hivemind AND newPlatform=Website
S->>HPW: handleHivemindWebsiteCase(newPlatform)
HPW-->>S: updated platform state
else module=Hivemind AND newPlatform=MediaWiki
S->>HPM: handleHivemindMediaWikiCase(newPlatform)
HPM-->>S: updated platform state
end
S->>EP: update metadata as needed
else no existing platform
S-->>C: proceed with standard creation/update
end
S-->>C: return result
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks (2 passed, 1 warning)❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Poem
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
.github/workflows/ci.yml (1)
9-9: Pin reusable workflowci2.ymlto a specific commit SHA
Verifiedci2.ymlsupportson: workflow_calland includes theCC_TEST_REPORTER_IDsecret; use a fixed commit SHA instead of@mainfor reproducibility:- uses: TogetherCrew/operations/.github/workflows/ci2.yml@main + uses: TogetherCrew/operations/.github/workflows/ci2.yml@<pinned-commit-sha>src/services/module.service.ts (5)
97-99: Hivemind + Website path re-enabled—good; minor duplication with the next branchTiny cleanup to reduce repeated
module.name === ModuleNames.Hivemindchecks:- if (module.name === ModuleNames.Hivemind && newPlatform.name === PlatformNames.Website) { - await handleHivemindWebsiteCase(newPlatform); - } - if (module.name === ModuleNames.Hivemind && newPlatform.name === PlatformNames.MediaWiki) { - await handleHivemindMediaWikiCase(newPlatform); - } + if (module.name === ModuleNames.Hivemind) { + if (newPlatform.name === PlatformNames.Website) { + await handleHivemindWebsiteCase(newPlatform); + } else if (newPlatform.name === PlatformNames.MediaWiki) { + await handleHivemindMediaWikiCase(newPlatform); + } + }
115-139: Avoid orphan schedules on partial failure (create succeeds, save fails)If
createWebsiteSchedulesucceeds butplatformDoc.save()fails, the schedule is left running without a persistedscheduleId.Add compensation and rethrow:
if (isActivated === true) { if (!existingScheduleId) { - const scheduleId = await websiteService.coreService.createWebsiteSchedule(platform.platform); - platformDoc.set('metadata.scheduleId', scheduleId); - - await platformDoc.save(); + let scheduleId: string | null = null; + try { + scheduleId = await websiteService.coreService.createWebsiteSchedule(platform.platform); + platformDoc.set('metadata.scheduleId', scheduleId); + await platformDoc.save(); + } catch (err) { + if (scheduleId) { + await websiteService.coreService.deleteWebsiteSchedule(scheduleId).catch(() => {}); + } + throw err; + } } } else if (isActivated === false) {Per prior learning, upstream handles thrown errors from
updateModule, so this preserves that contract while preventing leaked schedules.
115-139: Add light observability around schedule changesLog schedule create/delete/terminate decisions with platform and community IDs for auditability and easier incident triage.
I can add structured logs (e.g., pino) with minimal noise if you want.
93-107: Tests for Website activation togglePlease add unit/integration tests covering:
- activate: creates schedule and persists
metadata.scheduleIdwhen absent- deactivate: deletes schedule, terminates workflow, nulls
metadata.scheduleId- idempotency: repeated activate/deactivate are no-ops
I can scaffold Jest tests with service mocks for
websiteServiceandplatformService.
9-10: No changes needed forwebsiteServiceshape
- The
src/services/websitemodule’s default export exposescoreService, which providescreateWebsiteSchedule,deleteWebsiteSchedule, andterminateWebsiteWorkflowas expected.- Optional: introduce a TypeScript interface for
coreServiceto enforce its API at compile time.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/ci.yml(1 hunks)src/services/module.service.ts(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-10-09T08:57:20.287Z
Learnt from: Behzad-rabiei
PR: TogetherCrew/api#350
File: src/services/module.service.ts:55-71
Timestamp: 2024-10-09T08:57:20.287Z
Learning: Error handling for the `updateModule` function in `src/services/module.service.ts` is managed by the parent function, as indicated by user Behzad-rabiei.
Applied to files:
src/services/module.service.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: ci / build-push / Build + Push Image
🔇 Additional comments (1)
src/services/module.service.ts (1)
3-5: Import reorg looks fineNo functional impact.
Summary by CodeRabbit
New Features
Chores