Refactor SQL query for improved date range logic #68
Conversation
- Updated the date range logic in the TARGET_UNITS_QUERY to calculate the start date of the quarter dynamically, ensuring accurate reporting for target units. - Adjusted the `composeWeeklyReportTitle` method to set the period start based on the current quarter, enhancing the clarity and accuracy of weekly financial reports. These changes improve the accuracy of financial data retrieval and reporting in the application.
📝 Walkthrough## Walkthrough
The changes update the date range logic in both a SQL query and a report title generator. Instead of using a weekly window, both now calculate the start date as the first day of the current quarter, extending the reporting and data aggregation period to cover the entire quarter up to the end of the previous week.
## Changes
| File(s) | Change Summary |
|-----------------------------------------------------------------------------------------|---------------------------------------------------------------------------------------------------------------|
| workers/main/src/services/TargetUnit/queries.ts | Modified SQL query to filter records from the first day of the current quarter to the end of the previous week.|
| workers/main/src/services/WeeklyFinancialReport/WeeklyFinancialReportRepository.ts | Updated report title date range logic to start from the current quarter's first day instead of previous week. |
## Sequence Diagram(s)
```mermaid
sequenceDiagram
participant User
participant WeeklyFinancialReportRepository
participant Database
User->>WeeklyFinancialReportRepository: Request weekly report
WeeklyFinancialReportRepository->>Database: Query data (from quarter start to last week)
Database-->>WeeklyFinancialReportRepository: Return filtered data
WeeklyFinancialReportRepository->>WeeklyFinancialReportRepository: Compose report title (quarter start to last week)
WeeklyFinancialReportRepository-->>User: Return report with updated date rangeSuggested reviewers
|
🔍 Vulnerabilities of
|
| digest | sha256:76b235793c50cfdfc586db67473e3b54fff0dc8dcd4f8121d268130d9cb7702d |
| 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
|
![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 (1)
workers/main/src/services/WeeklyFinancialReport/WeeklyFinancialReportRepository.ts (1)
153-171: Consider renaming the method to reflect the new quarterly scope.The method is named
composeWeeklyReportTitlebut now generates titles for quarterly periods. This could be misleading for future maintainers.Consider renaming to better reflect the actual functionality:
- private composeWeeklyReportTitle(currentDate: Date): string { + private composeQuarterlyReportTitle(currentDate: Date): string {And update the return string to reflect the quarterly nature:
- return `*Weekly Financial Summary for Target Units* (${periodStart} - ${periodEnd})`; + return `*Quarterly Financial Summary for Target Units* (${periodStart} - ${periodEnd})`;Also update the call site on line 39:
- const reportTitle = this.composeWeeklyReportTitle(currentDate); + const reportTitle = this.composeQuarterlyReportTitle(currentDate);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
workers/main/src/services/TargetUnit/queries.ts(1 hunks)workers/main/src/services/WeeklyFinancialReport/WeeklyFinancialReportRepository.ts(1 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.
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: Docker Security Scanning (temporal, Dockerfile.temporal, temporal-test:latest)
- GitHub Check: Docker Security Scanning (n8n, Dockerfile.n8n, n8n-test:latest)
- GitHub Check: SonarQube
- GitHub Check: Service Availability Check
- GitHub Check: Analyze (actions)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (2)
workers/main/src/services/TargetUnit/queries.ts (1)
28-36: Simplify the SQL quarter date calculation
Lines 28–36 in workers/main/src/services/TargetUnit/queries.tsThe current logic uses nested
DATE_SUBcalls with modulo arithmetic to compute the quarter start date. While mathematically correct, it’s hard to read and maintain. Please refactor to:
- Extract the week-end date into a single expression or CTE:
week_end := DATE_SUB(CURDATE(), INTERVAL WEEKDAY(CURDATE()) + 1 DAY)- Use
QUARTER(week_end)to get the quarter number.- Build the quarter-start via
CONCATandLPAD, e.g.:
CONCAT(YEAR(week_end), '-', LPAD((quarter_num - 1) * 3 + 1, 2, '0'), '-01')- Wrap these in a CTE or subquery to avoid repeating the same functions.
Example refactor:
WITH params AS ( SELECT DATE_SUB(CURDATE(), INTERVAL WEEKDAY(CURDATE()) + 1 DAY) AS week_end, QUARTER(DATE_SUB(CURDATE(), INTERVAL WEEKDAY(CURDATE()) + 1 DAY)) AS quarter_num ) ... WHERE te.spent_on BETWEEN CONCAT( YEAR(params.week_end), '-', LPAD((params.quarter_num - 1) * 3 + 1, 2, '0'), '-01' ) AND params.week_endThis approach is clearer, DRYs up repeated calculations, and makes future maintenance easier. Please verify that it produces the correct quarter boundaries for all edge cases.
workers/main/src/services/WeeklyFinancialReport/WeeklyFinancialReportRepository.ts (1)
154-158: Confirm SQL and TypeScript quarter logic consistencyI wasn’t able to locate a SQL query in the WeeklyFinancialReport service that calculates the quarter start date, so please manually verify that your database-side logic matches the new TypeScript implementation:
- Ensure the SQL used for quarterly reporting (e.g. via
date_trunc('quarter', …)or equivalentEXTRACT/QUARTERfunctions) produces the same first-of-quarter dates as:const quarter = Math.floor(currentDate.getMonth() / 3); const periodStart = new Date(currentDate.getFullYear(), quarter * 3, 1);- Confirm that this change from weekly to quarterly reporting scope aligns with your business requirements.
- Simplified the construction of the periodStart date in the `composeWeeklyReportTitle` method by removing unnecessary line breaks, enhancing code readability. - This change contributes to cleaner code while maintaining the existing functionality of generating weekly financial report titles. These improvements streamline the codebase and support better maintainability of the financial reporting features.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
workers/main/src/services/WeeklyFinancialReport/WeeklyFinancialReportRepository.ts(1 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.
⏰ 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 (actions)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (1)
workers/main/src/services/WeeklyFinancialReport/WeeklyFinancialReportRepository.ts (1)
154-155: Confirm SQL quarter logic in TargetUnit queriesI didn’t find any quarter‐calculation logic in
workers/main/src/services/TargetUnit/queries.ts. Please manually verify that the SQL there uses the same quarter‐calculation (Math.floor(getMonth()/3)) or update the query to align with the repository code.• File to check:
- workers/main/src/services/TargetUnit/queries.ts
workers/main/src/services/WeeklyFinancialReport/WeeklyFinancialReportRepository.ts
Show resolved
Hide resolved
|
composeWeeklyReportTitlemethod to set the period start based on the current quarter, enhancing the clarity and accuracy of weekly financial reports.These changes improve the accuracy of financial data retrieval and reporting in the application.