feat(api-service): enhance platform notification usage with clickhouse support#9849
feat(api-service): enhance platform notification usage with clickhouse support#9849djabarovgeorge merged 6 commits intonextfrom
Conversation
…eature flags for billing usage
✅ Deploy Preview for dashboard-v2-novu-staging canceled.
|
|
Hey there and thank you for opening this pull request! 👋 We require pull request titles to follow specific formatting rules and it looks like your proposed title needs to be adjusted. Your PR title is: Requirements:
Expected format: Details: PR title must end with 'fixes TICKET-ID' (e.g., 'fixes NOV-123') or include ticket ID in branch name |
WalkthroughSubproject commit reference updated from e38b53ee33bda663c1948ad3a8271c1f91c140b6 to 0f71416217e50b7a0343b980e458a96df149b93b. Added WorkflowRunRepository method getPlatformUsageByDateRange(startDate, endDate, organizationId?) that queries ClickHouse for counts per organization in a date range. Added feature-flag enum members IS_BILLING_USAGE_CLICKHOUSE_ENABLED and IS_BILLING_USAGE_CLICKHOUSE_SHADOW_ENABLED. Updated an E2E test to inject mocks for the new repository method and feature-flag service, adjusting the GetPlatformNotificationUsage constructor signature. 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
commit: |
…for workflow and feature flags
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/api/src/app/billing/e2e/get-platform-notification-usage.e2e-ee.ts (1)
49-149: Add test case for ClickHouse path when feature flag is enabled.The feature flag mock is hardcoded to
false(lines 25, 46), meaningworkflowRunRepository.getPlatformUsageByDateRangeis never invoked. Add at least one test that setsgetFlagtotrueto verify the ClickHouse-based behavior is exercised:it('should use ClickHouse when feature flag is enabled', async () => { mockFeatureFlagsService.getFlag.resolves(true); mockWorkflowRunRepository.getPlatformUsageByDateRange.resolves([ { _id: 'org-id', notificationsCount: 50 } ]); const useCase = createUseCase(); const result = await useCase.execute( GetPlatformNotificationUsageCommand.create({ startDate: new Date('2021-01-01'), endDate: new Date('2021-01-31'), }) ); expect(result).to.deep.equal([{ _id: 'org-id', notificationsCount: 50 }]); expect(mockWorkflowRunRepository.getPlatformUsageByDateRange.calledOnce).to.be.true; });
🧹 Nitpick comments (1)
apps/api/src/app/billing/e2e/get-platform-notification-usage.e2e-ee.ts (1)
20-26: Consider adding interface types for mock objects.The mock objects work correctly but lack TypeScript interfaces. Adding explicit types would help catch any drift between the mock and actual implementation, especially as
WorkflowRunRepositoryandFeatureFlagsServiceevolve.🔧 Suggested improvement
+ interface MockWorkflowRunRepository { + getPlatformUsageByDateRange: sinon.SinonStub; + } + + interface MockFeatureFlagsService { + getFlag: sinon.SinonStub; + } + - const mockWorkflowRunRepository = { + const mockWorkflowRunRepository: MockWorkflowRunRepository = { getPlatformUsageByDateRange: sinon.stub().resolves([]), }; - const mockFeatureFlagsService = { + const mockFeatureFlagsService: MockFeatureFlagsService = { getFlag: sinon.stub().resolves(false), };As per coding guidelines, prefer interfaces over types in backend code.
What changed? Why was the change needed?
EE-PR
Screenshots
Expand for optional sections
Related enterprise PR
Special notes for your reviewer