Add sendReportToSlack activity #66
Conversation
- Introduced `formatCurrency` function to format numbers as USD currency, ensuring proper localization and rounding. - Added `getRateByDate` function to retrieve rates from a historical dataset based on a specified date, with handling for undefined inputs and edge cases. - Created unit tests for both functions to validate their functionality and accuracy, covering various scenarios for currency formatting and rate retrieval. These additions enhance the utility functions for financial calculations and improve test coverage for the application.
- Updated the `getRateByDate` function to sort the rate history dates chronologically using a custom sorting function. This ensures that the dates are processed in the correct order, improving the accuracy of rate retrieval based on date. These changes enhance the functionality of the rate retrieval process, ensuring more reliable data handling.
- Introduced `WeeklyFinancialReportRepository` class implementing `IWeeklyFinancialReportRepository` for generating weekly financial reports based on target units, employees, and projects. - Added `IWeeklyFinancialReportRepository` interface and `GenerateReportInput` type for structured input handling. - Created unit tests for the repository to validate report generation, ensuring correct summary and details output for various input scenarios, including handling of empty input arrays. These changes enhance the financial reporting capabilities of the application, providing structured and detailed insights into weekly performance.
- Introduced `sendReportToSlack` function to handle sending financial reports to Slack, integrating data retrieval and message posting. - Created unit tests for `sendReportToSlack` to validate successful report sending and error handling scenarios, ensuring robustness. - Updated `weeklyFinancialReports/index.ts` to export the new function. These changes enhance the application's ability to communicate financial reports via Slack, improving reporting capabilities and error management.
📝 WalkthroughWalkthroughA new function, Changes
Sequence Diagram(s)sequenceDiagram
participant Workflow
participant Activities
participant SlackService
participant ReportRepo
Workflow->>Activities: getTargetUnits()
Activities-->>Workflow: targetUnitsFileLink
Workflow->>Activities: fetchFinancialAppData()
Activities-->>Workflow: financialAppDataFileLink
Workflow->>Activities: sendReportToSlack(targetUnitsFileLink, financialAppDataFileLink)
Activities->>ReportRepo: generateWeeklyFinancialReport(data)
ReportRepo-->>Activities: {summary, details}
Activities->>SlackService: postMessage(summary)
SlackService-->>Activities: messageTs
Activities->>SlackService: postMessage(details, thread_ts=messageTs)
SlackService-->>Activities: ok
Activities-->>Workflow: "success"
Possibly related PRs
Suggested reviewers
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
🔍 Vulnerabilities of
|
| digest | sha256:44211e2b58d4fb885b3a37997c4380099a5d1bfda684e9f45379ceee5444e0f0 |
| vulnerabilities |     |
| platform | linux/amd64 |
| size | 243 MB |
| packages | 1628 |
📦 Base Image node:20-alpine
| also known as |
|
| digest | sha256:37a5a350292926f98d48de9af160b0a3f7fcb141566117ee452742739500a5bd |
| vulnerabilities |    |
Description
Description
| ||||||||||||||||
Description
| ||||||||||||||||
Description
| ||||||||||||||||
Description
| ||||||||||||||||
Description
| ||||||||||||||||
Description
| ||||||||||||||||
Description
| ||||||||||||||||
Description
|
…re/add-sendReportToSlack-activity
![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
🔭 Outside diff range comments (1)
workers/main/src/services/WeeklyFinancialReport/WeeklyFinancialReportRepository.ts (1)
29-135: Address ESLint violations by extracting helper methods.The
generateReportmethod exceeds ESLint limits:
- 107 lines (max: 50)
- 63 statements (max: 50)
- Complexity of 16 (max: 15)
While I understand the preference for simplicity during early development, the pipeline failures need to be addressed. Consider extracting at least these parts into private methods:
- Group processing logic (lines 46-101) →
processGroup()- Marginality categorization (lines 82-91) →
categorizeMarginalityLevel()- Report formatting sections →
formatGroupDetails()andformatSummarySection()Quick fix to bypass ESLint:
If you want to keep the current structure temporarily, you could disable the ESLint rules:+ // eslint-disable-next-line max-lines-per-function, max-statements, complexity async generateReport({However, I recommend the refactoring approach for better maintainability.
🧹 Nitpick comments (2)
workers/main/src/activities/weeklyFinancialReports/sendReportToSlack.test.ts (1)
43-51: Consider simplifying the mock reset logic.The runtime type checking seems overly defensive for a test environment. Since you're using Vitest mocks, you could simplify this:
- function tryMockReset(obj: unknown) { - if ( - typeof obj === 'function' && - 'mockReset' in obj && - typeof (obj as { mockReset: unknown }).mockReset === 'function' - ) { - (obj as { mockReset: () => void }).mockReset(); - } - } + function tryMockReset(obj: any) { + obj.mockReset?.(); + }However, the current implementation works and provides extra safety.
workers/main/src/services/WeeklyFinancialReport/WeeklyFinancialReportRepository.ts (1)
7-24: Date calculation logic is correct.The function correctly calculates the previous week's date range (Monday to Sunday). Consider adding a comment to explain the date arithmetic for future maintainability:
function composeWeeklyReportTitle(currentDate: Date): string { + // Calculate previous week's Monday const periodStart = new Date( currentDate.getFullYear(), currentDate.getMonth(), - currentDate.getDate() - ((currentDate.getDay() + 6) % 7) - 7, + currentDate.getDate() - ((currentDate.getDay() + 6) % 7) - 7, // Convert to Monday-based week and go back 1 week )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
workers/main/src/activities/weeklyFinancialReports/index.ts(1 hunks)workers/main/src/activities/weeklyFinancialReports/sendReportToSlack.test.ts(1 hunks)workers/main/src/activities/weeklyFinancialReports/sendReportToSlack.ts(1 hunks)workers/main/src/services/FinApp/types.ts(1 hunks)workers/main/src/services/WeeklyFinancialReport/WeeklyFinancialReportRepository.ts(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: anatolyshipitz
PR: speedandfunction/automatization#34
File: workers/main/src/workflows/financial/FinancialReportFormatter.ts:3-7
Timestamp: 2025-05-30T17:57:21.010Z
Learning: User anatolyshipitz prefers to keep code implementations simple during early development stages rather than adding comprehensive error handling and validation. They consider extensive type annotations and error handling "redundant" when focusing on core functionality and testing.
🧬 Code Graph Analysis (1)
workers/main/src/services/WeeklyFinancialReport/WeeklyFinancialReportRepository.ts (1)
workers/main/src/common/formatUtils.ts (2)
getRateByDate(5-27)formatCurrency(1-3)
🪛 GitHub Actions: Code Quality
workers/main/src/services/WeeklyFinancialReport/WeeklyFinancialReportRepository.ts
[error] 29-29: ESLint: Async method 'generateReport' has too many lines (107). Maximum allowed is 50. (max-lines-per-function)
[error] 29-29: ESLint: Async method 'generateReport' has too many statements (63). Maximum allowed is 50. (max-statements)
[error] 29-29: ESLint: Async method 'generateReport' has a complexity of 16. Maximum allowed is 15. (complexity)
🔇 Additional comments (4)
workers/main/src/services/FinApp/types.ts (1)
27-30: LGTM! Clean interface definition.The
FinancialsAppDatainterface is well-structured and follows the existing patterns in the file.workers/main/src/activities/weeklyFinancialReports/index.ts (1)
3-3: Export addition looks good.workers/main/src/activities/weeklyFinancialReports/sendReportToSlack.test.ts (1)
62-141: Excellent test coverage!The test suite comprehensively covers all success and error paths with appropriate assertions on both return values and mock interactions.
workers/main/src/activities/weeklyFinancialReports/sendReportToSlack.ts (1)
8-36: Clean implementation with good error handling.The function is well-structured with:
- Efficient concurrent file reading
- Proper error handling and wrapping
- Clear separation of concerns
- Thread support for organizing Slack messages
- Introduced a new method `processTargetUnit` to encapsulate the logic for processing each target unit, enhancing code readability and maintainability. - Added `pushGroupByMarginality` method to categorize groups based on their marginality level, streamlining the report generation process. - Refactored the report title generation into a private method `composeWeeklyReportTitle` for better organization. - Updated the report details and summary formatting using `WeeklyFinancialReportFormatter`, ensuring consistent output structure. These changes enhance the clarity and efficiency of the weekly financial report generation process, making it easier to manage and extend in the future.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
workers/main/src/workflows/weeklyFinancialReports/weeklyFinancialReports.workflow.test.ts (1)
78-96: Consider adding error propagation test for the new activity.While the existing error tests cover the first two activities, adding a test for error propagation from
sendReportToSlackwould provide complete coverage.Add this test case:
+ it('propagates error from sendReportToSlack', async () => { + getTargetUnitsMock.mockResolvedValueOnce({ fileLink: 'file.json' }); + fetchFinancialAppDataMock.mockResolvedValueOnce({ + fileLink: 'result.json', + }); + sendReportToSlackMock.mockRejectedValueOnce(new Error('slack error')); + await expect( + weeklyFinancialReportsWorkflow(GroupNameEnum.SD_REPORT), + ).rejects.toThrow('slack error'); + });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
workers/main/src/workflows/weeklyFinancialReports/weeklyFinancialReports.workflow.test.ts(2 hunks)workers/main/src/workflows/weeklyFinancialReports/weeklyFinancialReports.workflow.ts(2 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: anatolyshipitz
PR: speedandfunction/automatization#34
File: workers/main/src/workflows/financial/FinancialReportFormatter.ts:3-7
Timestamp: 2025-05-30T17:57:21.010Z
Learning: User anatolyshipitz prefers to keep code implementations simple during early development stages rather than adding comprehensive error handling and validation. They consider extensive type annotations and error handling "redundant" when focusing on core functionality and testing.
workers/main/src/workflows/weeklyFinancialReports/weeklyFinancialReports.workflow.ts (2)
Learnt from: CR
PR: speedandfunction/automatization#0
File: .cursor/rules/temporal-project-structure.mdc:0-0
Timestamp: 2025-06-24T12:29:48.641Z
Learning: When introducing new Temporal workers or workflows, it is essential to avoid duplicating logic already present in shared modules and to update documentation with every significant change to workflows or activities.
Learnt from: CR
PR: speedandfunction/automatization#0
File: .cursor/rules/temporal-project-structure.mdc:0-0
Timestamp: 2025-06-24T12:29:48.641Z
Learning: Workflow and activity logic in Temporal projects should be kept modular and well-tested, and all naming (workflows, activities, task queues) should be clear and descriptive to enhance code quality and maintainability.
workers/main/src/workflows/weeklyFinancialReports/weeklyFinancialReports.workflow.test.ts (2)
Learnt from: CR
PR: speedandfunction/automatization#0
File: .cursor/rules/temporal-project-structure.mdc:0-0
Timestamp: 2025-06-24T12:29:48.641Z
Learning: Workflow and activity logic in Temporal projects should be kept modular and well-tested, and all naming (workflows, activities, task queues) should be clear and descriptive to enhance code quality and maintainability.
Learnt from: CR
PR: speedandfunction/automatization#0
File: .cursor/rules/temporal-project-structure.mdc:0-0
Timestamp: 2025-06-24T12:29:48.641Z
Learning: When introducing new Temporal workers or workflows, it is essential to avoid duplicating logic already present in shared modules and to update documentation with every significant change to workflows or activities.
🧬 Code Graph Analysis (2)
workers/main/src/workflows/weeklyFinancialReports/weeklyFinancialReports.workflow.ts (1)
workers/main/src/activities/weeklyFinancialReports/sendReportToSlack.ts (1)
sendReportToSlack(8-36)
workers/main/src/workflows/weeklyFinancialReports/weeklyFinancialReports.workflow.test.ts (1)
workers/main/src/workflows/weeklyFinancialReports/weeklyFinancialReports.workflow.ts (1)
weeklyFinancialReportsWorkflow(13-26)
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: Docker Security Scanning (n8n, Dockerfile.n8n, n8n-test:latest)
- GitHub Check: Docker Security Scanning (temporal, Dockerfile.temporal, temporal-test:latest)
- GitHub Check: Service Availability Check
- GitHub Check: SonarQube
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (actions)
🔇 Additional comments (4)
workers/main/src/workflows/weeklyFinancialReports/weeklyFinancialReports.workflow.ts (2)
8-11: LGTM! Clean integration of the new activity.The
sendReportToSlackactivity is properly added to the proxy activities with consistent timeout configuration.
25-25: Excellent workflow orchestration.The sequential data flow is well-implemented: the workflow now properly orchestrates all three activities and returns the result from
sendReportToSlack, which aligns with the PR objective of sending reports to Slack.workers/main/src/workflows/weeklyFinancialReports/weeklyFinancialReports.workflow.test.ts (2)
11-11: LGTM! Comprehensive mock setup for the new activity.The mock setup follows the established pattern consistently and includes proper cleanup in
beforeEach.Also applies to: 17-17, 21-21, 29-29, 37-39, 44-44
83-94: Excellent test coverage for the workflow integration.The test properly verifies:
- The new activity is called with correct parameters (both file links)
- The workflow returns the expected result from
sendReportToSlack- The sequential execution flow is maintained
This ensures the workflow orchestration works correctly.
anatolyshipitz
force-pushed
the
feature/add-sendReportToSlack-activity
branch
from
6639af9 to
4cd5888
Compare
|
sendReportToSlackfunction to handle sending financial reports to Slack, integrating data retrieval and message posting.sendReportToSlackto validate successful report sending and error handling scenarios, ensuring robustness.weeklyFinancialReports/index.tsto export the new function.These changes enhance the application's ability to communicate financial reports via Slack, improving reporting capabilities and error management.