-
Notifications
You must be signed in to change notification settings - Fork 0
Enhance weekly financial reports workflow to include Slack reporting #70
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
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.
…re/add-sendReportToSlack-activity
- 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.
- Updated the `weeklyFinancialReportsWorkflow` to integrate the `sendReportToSlack` function, allowing for the sending of financial reports directly to Slack. - Modified the workflow to return the Slack link instead of the financial data file link, improving the reporting process. - Added necessary mocks for `sendReportToSlack` in the corresponding test file to ensure proper testing of the new functionality. These changes enhance the application's reporting capabilities by facilitating direct communication of financial reports via Slack, streamlining the workflow process.
📝 WalkthroughWalkthroughThe workflow for weekly financial reports was updated to call a new activity, Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant Workflow
participant getTargetUnits
participant fetchFinancialAppData
participant sendReportToSlack
Caller->>Workflow: weeklyFinancialReportsWorkflow(groupName)
Workflow->>getTargetUnits: getTargetUnits(groupName)
getTargetUnits-->>Workflow: fileLink
Workflow->>fetchFinancialAppData: fetchFinancialAppData(fileLink)
fetchFinancialAppData-->>Workflow: resultLink
Workflow->>sendReportToSlack: sendReportToSlack(fileLink, resultLink)
sendReportToSlack-->>Workflow: slackLink
Workflow-->>Caller: slackLink
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:da30eca7154c7c4cb2616e31f7411021ea9146d5b15c68adf2a57df4c52b164c |
vulnerabilities | |
platform | linux/amd64 |
size | 218 MB |
packages | 358 |
📦 Base Image alpine:3
also known as |
|
digest | sha256:1c4eef651f65e2f7daee7ee785882ac164b02b78fb74503052a26dc061c90474 |
vulnerabilities |
Description
| ||||||||||||
Description
| ||||||||||||
Description
| ||||||||||||
Description
| ||||||||||||
Description
| ||||||||||||
Description
| ||||||||||||
Description
| ||||||||||||
Description | ||||||||||||
Description
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
workers/main/src/workflows/weeklyFinancialReports/weeklyFinancialReports.workflow.test.ts (2)
78-78
: Update test description to reflect the new return value.The test description "returns fileLink on success" is now outdated since the workflow returns a Slack confirmation message instead of a file link.
- it('returns fileLink on success', async () => { + it('returns Slack confirmation on success', async () => {
96-96
: Consider adding error handling test for the new activity.The test suite covers error propagation from
getTargetUnits
andfetchFinancialAppData
, but there's no corresponding test for error propagation fromsendReportToSlack
. This would ensure comprehensive error handling coverage.Add this test after the existing error handling tests:
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: 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: 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 (5)
- GitHub Check: Docker Security Scanning (temporal, Dockerfile.temporal, temporal-test:latest)
- GitHub Check: Docker Security Scanning (n8n, Dockerfile.n8n, n8n-test:latest)
- GitHub Check: Service Availability Check
- GitHub Check: SonarQube
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (4)
workers/main/src/workflows/weeklyFinancialReports/weeklyFinancialReports.workflow.ts (2)
8-11
: LGTM: Clean integration of the new Slack reporting activity.The
sendReportToSlack
activity is properly integrated into the proxied activities with consistent timeout configuration. The implementation maintains the modular structure as recommended in the project guidelines.
25-25
: LGTM: Correct parameter passing to the new activity.The workflow correctly passes both file links (
targetUnits.fileLink
andfinData.fileLink
) to thesendReportToSlack
activity, matching the expected parameters from the activity implementation inworkers/main/src/activities/weeklyFinancialReports/sendReportToSlack.ts
.workers/main/src/workflows/weeklyFinancialReports/weeklyFinancialReports.workflow.test.ts (2)
11-11
: LGTM: Comprehensive mock setup for the new activity.The mock setup for
sendReportToSlack
follows the existing patterns and includes proper reset inbeforeEach
. The integration is consistent with the existing test structure.Also applies to: 17-17, 21-21, 29-29, 37-39, 44-44
83-83
: LGTM: Correct test updates for the new activity.The test properly verifies that:
sendReportToSlack
is called with the correct parameters ('file.json'
and'result.json'
)- The workflow returns the result from
sendReportToSlack
('slack-link.json'
)Also applies to: 88-88, 91-94
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good
weeklyFinancialReportsWorkflow
to integrate thesendReportToSlack
function, allowing for the sending of financial reports directly to Slack.sendReportToSlack
in the corresponding test file to ensure proper testing of the new functionality.These changes enhance the application's reporting capabilities by facilitating direct communication of financial reports via Slack, streamlining the workflow process.